Skip to content

Commit

Permalink
fix(service-worker): only consider GET requests as navigation requests (
Browse files Browse the repository at this point in the history
#47263)

Previously, the criteria for determining if a request was a
[navigation request][1] did not account for the request method. This
incorrectly identified HTML form submit POST requests as navigation
requests and served `index.html` instead of passing them through to the
server, thus breaking the form submission.

This commit fixes this by ensuring that only GET requests are considered
navigation requests.

> **Note**
> HTML forms with their method set to `GET` will still be affected by
> the issue. This is not a big concern, because using `GET` for form
> submission is quite uncommon and generally discouraged (due to
> limitations and security considerations).

[1]: https://angular.io/guide/service-worker-config#handling-navigation-requests

Fixes #36368

PR Close #47263
  • Loading branch information
gkalpak authored and AndrewKushnir committed Sep 6, 2022
1 parent 56304d6 commit 28d3350
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 2 deletions.
1 change: 1 addition & 0 deletions aio/content/guide/service-worker-config.md
Expand Up @@ -376,6 +376,7 @@ This optional section enables you to specify a custom list of URLs that will be
The ServiceWorker redirects navigation requests that don't match any `asset` or `data` group to the specified [index file](#index-file).
A request is considered to be a navigation request if:

* Its [method](https://developer.mozilla.org/docs/Web/API/Request/method) is `GET`
* Its [mode](https://developer.mozilla.org/docs/Web/API/Request/mode) is `navigation`
* It accepts a `text/html` response as determined by the value of the `Accept` header
* Its URL matches the following criteria:
Expand Down
4 changes: 2 additions & 2 deletions packages/service-worker/worker/src/app-version.ts
Expand Up @@ -203,10 +203,10 @@ export class AppVersion implements UpdateSource {

/**
* Determine whether the request is a navigation request.
* Takes into account: Request mode, `Accept` header, `navigationUrls` patterns.
* Takes into account: Request method and mode, `Accept` header, `navigationUrls` patterns.
*/
isNavigationRequest(req: Request): boolean {
if (req.mode !== 'navigate') {
if (req.method !== 'GET' || req.mode !== 'navigate') {
return false;
}

Expand Down
8 changes: 8 additions & 0 deletions packages/service-worker/worker/test/happy_spec.ts
Expand Up @@ -1704,6 +1704,14 @@ describe('Driver', () => {
server.assertNoOtherRequests();
});

it('does not redirect to index on a non-GET request', async () => {
expect(await navRequest('/baz', {method: 'POST'})).toBeNull();
server.assertSawRequestFor('/baz');

expect(await navRequest('/qux', {method: 'PUT'})).toBeNull();
server.assertSawRequestFor('/qux');
});

it('does not redirect to index on a non-navigation request', async () => {
expect(await navRequest('/baz', {mode: undefined})).toBeNull();
server.assertSawRequestFor('/baz');
Expand Down

0 comments on commit 28d3350

Please sign in to comment.