Commit graph

12 commits

Author SHA1 Message Date
JJ Kasper
f0fd4962e9
Revert "Fix running server with Polyfilled fetch (#31935)" (#32100)
This reverts commit 1c199a5e4a.
2021-12-03 15:31:52 -06:00
Javi Velasco
1c199a5e4a
Fix running server with Polyfilled fetch (#31935)
This PR fixes #30398

By default Next will polyfill some fetch APIs (Request, Response, Header and fetch) only if fetch is not found in the global scope in certain entry points. If we have a custom server which is adding a global fetch (and only fetch) at the very top then the rest of APIs will not be polyfilled.

This PR adds a test on the custom server where we can add a custom polyfill for fetch with an env variable. This reproduces the issue since next-server.js will be required without having a polyfill for Response which makes it fail on requiring NextResponse. Then we remove the code that checks for subrequests to happen within the **sandbox** so that we don't need to polyfill `next-server` anymore.

The we also introduce an improvement on how we handle relative requests. Since #31858 introduced a `port` and `hostname` options for the server, we can always pass absolute URLs to the Middleware so we can always use the original `nextUrl` to pass it to fetch. This brings a lot of simplification for `NextURL` since we don't have to consider relative URLs no more.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
2021-12-03 16:35:28 +00:00
Javi Velasco
2e5218c778
Fix hydration middleware effects (#31800)
Whenever we trigger a route change in the client we check if there route we are navigating to is affected by a middleware. When this is the case we run a preflight and in case there is an effect that tells us that the middleware is responding with content we force a _refresh_. This is fine for navigation in general but it is not ok when the change is triggered for hydration. For example, in cases where the rendered page is a dynamic page this triggers an infinite reload.

In this PR we add a test where we add a `_middleware` that proxies to a dynamic path. When making a request to `/interface/root-subrequest` there will be a middleware that simply performs a fetch against localhost for the same `/interface/root-subrequest`. The new request will skip the middleware to avoid loops and then render the dynamic page. Then client will force a change for hydration resulting in a preflight request that tells that the client must refresh because for that path there is a middleware writing content.

Then we add a fix which simply consist of checking the internal option that tells when a change is triggered for hydration and skip the preflight in such scenario.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
2021-11-26 12:46:57 +00:00
Javi Velasco
5a3d558c9f
Fix HMR for middleware #30791 (#31548)
Fixes #30791

The issue is that with Middleware we are introducing client compilation on a new layer. When a middleware changes (or is dropped after some time), Webpack reorganizes non-user modules as they are duplicated across different layer. This is currently triggering a full reload.

This also brings tests for HMR:
- Refresh on a Middleware change
- HMR works after a middleware compilation change.

Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
2021-11-18 18:23:52 +00:00
Matt Kane
9952cc79db
Don't proxy middleware if host is the same (#31180)
This changes the check for whether a rewrite is internal or if it should be proxied. Currently it checks if `protocol` is unset, which is only the case for relative URLs or localhost. This means that requests where there is a hostname set, or if localhost is specified in another way such as `127.0.0.1`, then it will be proxied, which potentially causes a proxy loop or ssl error. This PR changes the test so that it also checks if the hosts match, and only proxies if they are different.

Fixes #31179 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
2021-11-13 00:20:13 +00:00
Hiroshi Ogawa
4a22059b11
fix(30724): clear "x-middleware-next" header when chaining middlewares (#30866)
* fix(30724): clear "x-middleware-next" header when chaining middlewares

* refactor: do not accumulate "x-middleware-next" header

* test: fix grammar of test case title

* Update packages/next/server/next-server.ts

Co-authored-by: Javi Velasco <javier.velasco86@gmail.com>
2021-11-12 17:44:17 -06:00
Filip Skokan
f796ea3e7d
fix(middleware): fetch resource may be a URL instance (or any stringifiable value) (#31260)
The `resource` argument[^1] in fetch may also be an instance of URL (or any other stringifiable value) but the sandbox variant of middlewares doesn't support that.

```js
export async function middleware(req, ev) {
  await fetch(new URL('https://www.googleapis.com/oauth2/v3/certs'), {
    redirect: 'manual',
    method: 'GET',
  })

  return new Response(JSON.stringify({}), { status: 200 });
}
```

This is fixing the use of e.g. URL instance in `fetch`.

```
TypeError: initurl.startsWith is not a function
  at getFetchURL (/my-next-app/node_modules/next/dist/server/web/sandbox/sandbox.js:246:17)
  at fetch (/my-next-app/node_modules/next/dist/server/web/sandbox/sandbox.js:77:29)
  at Object.middleware [as handler] (webpack-internal:///./pages/_middleware.js:86:15)
  at async adapter (webpack-internal:///./node_modules/next/dist/server/web/adapter.js:30:22)
  at async DevServer.runMiddleware (/my-next-app/node_modules/next/dist/server/next-server.js:430:26)
  at async DevServer.runMiddleware (/my-next-app/node_modules/next/dist/server/dev/next-dev-server.js:394:28)
  at async Object.fn (/my-next-app/node_modules/next/dist/server/next-server.js:807:34)
  at async Router.execute (/my-next-app/node_modules/next/dist/server/router.js:211:32)
  at async DevServer.run (/my-next-app/node_modules/next/dist/server/next-server.js:1115:29)
  at async DevServer.run (/my-next-app/node_modules/next/dist/server/dev/next-dev-server.js:440:20)
```

[^1]: https://developer.mozilla.org/en-US/docs/Web/API/fetch#parameters
2021-11-12 13:22:27 +00:00
Tobias Koppers
9c4c7123b6
run middleware parser handler only for middleware modules (#31219)
## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
2021-11-10 15:31:46 +00:00
Filip Skokan
0985b0b472
share collections in middleware vm context (#31043)
When libraries are required outside of the middleware function context and they do checks such as `a instanceof Uint8Array` since the constructors are different between the two contexts they'll always yield false.

This is a problem for libraries validating user input as well as the WebCryptoAPI polyfill used outside of Edge Functions.

- Fixes #30477
- Fixes #30911

This is only a problem for the sandbox runtime, not when ran inside an Edge Function.
2021-11-09 19:57:19 +00:00
Hiroshi Ogawa
764e29c170
fix(31013): add base path to preflight request url (#31101)
Fixes https://github.com/vercel/next.js/issues/31013

My understanding is that there are currently two functions `Router.change` and `Router.prefetch` leading to `Router._preflightRequest` and they pass `options.as` URL differently regarding base path.
In this PR, such difference will be handled in `Router._preflightRequest` to add base path before actually fetching.
Thanks for the review!

P.S.
Since middleware feature is a relatively new, official maintainers might not want external contributions around this area at this stage.
I totally understand such situation, so please let me know if that's the case. I can look for other issues to investigate instead.


## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
2021-11-09 19:03:05 +00:00
Javi Velasco
5fc4325aa6
Fix middleware i18n rewrites (#31174)
Fixes #30897

This PR fixes the linked issue where rewrites are not being applied for locale. It adds the corresponding test but also, as it was added in debugging process, it introduces a helper to read/write into the `request` object.

We are currently writing directly into the request by casting to `any` and then using flags like `_nextRewrote`. Instead, this PR puts all of this metadata under a symbol so it is not directly accessible. This also allows to have a single place where all of this metadata is listed so we can add comments describing the purpose of each flag.

In the same way, there is metadata written in the querystring. This is adding some types for it in order to throw some visibility on where is this metadata accessed. In an upcoming PR we can move all of it to the `request` object if possible to simplify the system.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
2021-11-09 01:28:39 +00:00
Javi Velasco
6e081e175f
Update middleware eval checks (#30883)
Co-authored-by: Tobias Koppers <sokra@users.noreply.github.com>

With this PR we are updating the way we check the usage of `eval` and other dynamic code evaluation (like `new Function`) for middleware. Now instead of simply showing a warning it will behave differently depending on if we are building or in development.

- Development: we replace the dynamic code with a wrapper so that we print a warning only when the code is used. We don't fail in this scenario as it is possible that once the application is built the code that uses `eval` is left out.
- Build: we detect with tree shaking if the code that will be bundled into the middleware includes any dynamic code and in such scenario we make the build fail as don't want to allow it for the production environment.

Closes #30674

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
2021-11-05 20:48:43 +00:00