fix(service-worker): include headers in requests for assets #47260

gkalpak · 2022-08-25T18:25:03Z

Previously, when requesting non-cached asset resources from the network, the ServiceWorker would strip off all request metadata (including headers). This was done in order to avoid issues with opaque responses, but it turned out to be overly aggressive, breaking/worsening legit usecases (such as requesting compressed data).

This commit fixes this by preserving the headers of such requests.

For reference, Workbox passes the original request as is. (See for example the NetworkFirst strategy).

Note
Data requests (i.e. requests for URLs that belong to a data-group) are not affected by this. They already use the original resource as is.

Fixes #24227

…it` --> `same-origin`) Previously, the default value of `MockRequest#credentials` (used in testing) was set to `omit`. This was different than the default value used in browsers, which is `same-origin` (see [Request docs on MDN][1]) and could lead to incorrect test results. This commit changes the default value for `credentials` to `same-origin` to match what would happen in the browser. [1]: https://developer.mozilla.org/en-US/docs/Web/API/Request/credentials

…erState` Previously, the `MockServerStateBuilder#withRedirect()` method did two things: (a) define a redirect from one path to another and (b) specify the contents of the redirect destination. This was confusing, because it deviated from the regular way of specifying file contents, which is via a `MockFileSystem` instance. This commit slightly simplifies the process of defining redirects by having the `withRedirect()` method only define the redirect and let the contents of the redirect destination be specified as usual via `MockFileSystem`. This makes `MockFileSystem` the single source of truth for file contents used with `MockServerState`.

Previously, when requesting non-cached asset resources from the network, the ServiceWorker would strip off all request metadata (including headers). This was done in order to avoid issues with opaque responses, but it turned out to be overly aggressive, breaking/worsening legit usecases (such as requesting compressed data). This commit fixes this by preserving the headers of such requests. For reference, Workbox passes the original request as is. (See for example the [NetworkFirst][1] strategy). > **Note** > Data requests (i.e. requests for URLs that belong to a data-group) are not affected by this. They already use the original resource as is. [1]: https://github.com/GoogleChrome/workbox/blob/95f97a207fd51efb3f8a653f6e3e58224183a778/packages/workbox-strategies/src/NetworkFirst.ts#L90 Fixes angular#24227

mary-poppins · 2022-08-25T18:36:44Z

You can preview 281e3b3 at https://pr47260-281e3b3.ngbuilds.io/.

josephperrott

LGTM

AndrewKushnir · 2022-09-06T17:03:16Z

This PR was merged into the repository by commit e1bb638.

…erState` (#47260) Previously, the `MockServerStateBuilder#withRedirect()` method did two things: (a) define a redirect from one path to another and (b) specify the contents of the redirect destination. This was confusing, because it deviated from the regular way of specifying file contents, which is via a `MockFileSystem` instance. This commit slightly simplifies the process of defining redirects by having the `withRedirect()` method only define the redirect and let the contents of the redirect destination be specified as usual via `MockFileSystem`. This makes `MockFileSystem` the single source of truth for file contents used with `MockServerState`. PR Close #47260

Previously, when requesting non-cached asset resources from the network, the ServiceWorker would strip off all request metadata (including headers). This was done in order to avoid issues with opaque responses, but it turned out to be overly aggressive, breaking/worsening legit usecases (such as requesting compressed data). This commit fixes this by preserving the headers of such requests. For reference, Workbox passes the original request as is. (See for example the [NetworkFirst][1] strategy). > **Note** > Data requests (i.e. requests for URLs that belong to a data-group) are not affected by this. They already use the original resource as is. [1]: https://github.com/GoogleChrome/workbox/blob/95f97a207fd51efb3f8a653f6e3e58224183a778/packages/workbox-strategies/src/NetworkFirst.ts#L90 Fixes #24227 PR Close #47260

…it` --> `same-origin`) (#47260) Previously, the default value of `MockRequest#credentials` (used in testing) was set to `omit`. This was different than the default value used in browsers, which is `same-origin` (see [Request docs on MDN][1]) and could lead to incorrect test results. This commit changes the default value for `credentials` to `same-origin` to match what would happen in the browser. [1]: https://developer.mozilla.org/en-US/docs/Web/API/Request/credentials PR Close #47260

…erState` (#47260) Previously, the `MockServerStateBuilder#withRedirect()` method did two things: (a) define a redirect from one path to another and (b) specify the contents of the redirect destination. This was confusing, because it deviated from the regular way of specifying file contents, which is via a `MockFileSystem` instance. This commit slightly simplifies the process of defining redirects by having the `withRedirect()` method only define the redirect and let the contents of the redirect destination be specified as usual via `MockFileSystem`. This makes `MockFileSystem` the single source of truth for file contents used with `MockServerState`. PR Close #47260

Previously, when requesting non-cached asset resources from the network, the ServiceWorker would strip off all request metadata (including headers). This was done in order to avoid issues with opaque responses, but it turned out to be overly aggressive, breaking/worsening legit usecases (such as requesting compressed data). This commit fixes this by preserving the headers of such requests. For reference, Workbox passes the original request as is. (See for example the [NetworkFirst][1] strategy). > **Note** > Data requests (i.e. requests for URLs that belong to a data-group) are not affected by this. They already use the original resource as is. [1]: https://github.com/GoogleChrome/workbox/blob/95f97a207fd51efb3f8a653f6e3e58224183a778/packages/workbox-strategies/src/NetworkFirst.ts#L90 Fixes #24227 PR Close #47260

…it` --> `same-origin`) (angular#47260) Previously, the default value of `MockRequest#credentials` (used in testing) was set to `omit`. This was different than the default value used in browsers, which is `same-origin` (see [Request docs on MDN][1]) and could lead to incorrect test results. This commit changes the default value for `credentials` to `same-origin` to match what would happen in the browser. [1]: https://developer.mozilla.org/en-US/docs/Web/API/Request/credentials PR Close angular#47260

…erState` (angular#47260) Previously, the `MockServerStateBuilder#withRedirect()` method did two things: (a) define a redirect from one path to another and (b) specify the contents of the redirect destination. This was confusing, because it deviated from the regular way of specifying file contents, which is via a `MockFileSystem` instance. This commit slightly simplifies the process of defining redirects by having the `withRedirect()` method only define the redirect and let the contents of the redirect destination be specified as usual via `MockFileSystem`. This makes `MockFileSystem` the single source of truth for file contents used with `MockServerState`. PR Close angular#47260

…47260) Previously, when requesting non-cached asset resources from the network, the ServiceWorker would strip off all request metadata (including headers). This was done in order to avoid issues with opaque responses, but it turned out to be overly aggressive, breaking/worsening legit usecases (such as requesting compressed data). This commit fixes this by preserving the headers of such requests. For reference, Workbox passes the original request as is. (See for example the [NetworkFirst][1] strategy). > **Note** > Data requests (i.e. requests for URLs that belong to a data-group) are not affected by this. They already use the original resource as is. [1]: https://github.com/GoogleChrome/workbox/blob/95f97a207fd51efb3f8a653f6e3e58224183a778/packages/workbox-strategies/src/NetworkFirst.ts#L90 Fixes angular#24227 PR Close angular#47260

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/14.2.0/14.2.1) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/14.2.0/14.2.1) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/14.2.0/14.2.1) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/14.2.0/14.2.1) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/14.2.0/14.2.1) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/14.2.0/14.2.1) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/14.2.0/14.2.1) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`14.2.0` -> `14.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/14.2.0/14.2.1) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v14.2.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1421-2022-09-07) [Compare Source](angular/angular@14.2.0...14.2.1) ##### common | Commit | Type | Description | | -- | -- | -- | | [c0d7ac9ec2](angular/angular@c0d7ac9) | fix | improve formatting of image warnings ([#47299](angular/angular#47299)) | | [1875ce520a](angular/angular@1875ce5) | fix | use DOCUMENT token to query for preconnect links ([#47353](angular/angular#47353)) | ##### compiler | Commit | Type | Description | | -- | -- | -- | | [0e35829580](angular/angular@0e35829) | fix | avoid errors for inputs with Object-builtin names ([#47220](angular/angular#47220)) | ##### service-worker | Commit | Type | Description | | -- | -- | -- | | [6091786696](angular/angular@6091786) | fix | include headers in requests for assets ([#47260](angular/angular#47260)) | | [28d33505fd](angular/angular@28d3350) | fix | only consider GET requests as navigation requests ([#47263](angular/angular#47263)) | #### Special Thanks Aristeidis Bampakos, Asaf M, Bingo's Code, Bob Watson, Daniel Ostrovsky, George Kalpakas, Giovanni Alberto Rivas, Jeremy Elbourn, Jobayer Hossain, Joe Martin (Crowdstaffing), Joey Perrott, JoostK, Kara Erickson, Kristiyan Kostadinov, Maina Wycliffe, Sabareesh Kappagantu, Simona Cotin, Sonu Sindhu, Yann Provoost, abergquist, jaybell and vyom1611  ##### router | Commit | Type | Description | | -- | -- | -- | | [5163e3d876](angular/angular@5163e3d) | feat | Add UrlTree constructor to public API ([#47186](angular/angular#47186)) | #### Special Thanks Alan Agius, Alex Rickabaugh, Andrew Scott, Joey Perrott, Kara Erickson, Kristiyan Kostadinov and Pawel Kozlowski  </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1538 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2022-10-07T16:09:15Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

gkalpak added 3 commits August 25, 2022 21:21

gkalpak added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: service-worker Issues related to the @angular/service-worker package labels Aug 25, 2022

ngbot bot modified the milestone: Backlog Aug 25, 2022

gkalpak force-pushed the fix-sw-asset-headers branch from a5b053c to 281e3b3 Compare August 25, 2022 18:28

gkalpak marked this pull request as ready for review August 25, 2022 18:53

gkalpak requested a review from alxhub August 25, 2022 18:54

josephperrott approved these changes Aug 26, 2022

View reviewed changes

gkalpak removed the request for review from alxhub August 26, 2022 18:17

gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 26, 2022

gkalpak requested review from alxhub and removed request for alxhub August 26, 2022 19:21

AndrewKushnir closed this in e842fd7 Sep 6, 2022

gkalpak deleted the fix-sw-asset-headers branch September 6, 2022 19:09

angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(service-worker): include headers in requests for assets #47260

fix(service-worker): include headers in requests for assets #47260

gkalpak commented Aug 25, 2022 •

edited

mary-poppins commented Aug 25, 2022

josephperrott left a comment

AndrewKushnir commented Sep 6, 2022

angular-automatic-lock-bot bot commented Oct 7, 2022

fix(service-worker): include headers in requests for assets #47260

fix(service-worker): include headers in requests for assets #47260

Conversation

gkalpak commented Aug 25, 2022 • edited

mary-poppins commented Aug 25, 2022

josephperrott left a comment

Choose a reason for hiding this comment

AndrewKushnir commented Sep 6, 2022

angular-automatic-lock-bot bot commented Oct 7, 2022

gkalpak commented Aug 25, 2022 •

edited