New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 645281 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 540509



Sign in to add a comment

Foreign Fetch incorrectly intercepts same origin fetches made from service workers

Project Member Reported by mek@chromium.org, Sep 8 2016

Issue description

The "initiator" of a request is not getting set correctly for requests made from workers. The result of this is that the foreign fetch code doesn't detect requests made by it's own service worker as being same origin. This results in infinite loops if you try to fetch(event.request) in a foreign fetch handler.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/65299f214fad8f609c5872bcdd5f4ea82ec29713

commit 65299f214fad8f609c5872bcdd5f4ea82ec29713
Author: mek <mek@chromium.org>
Date: Fri Sep 09 21:11:13 2016

Correctly set requestor origin for worker initiated requests.

ResourceRequest.requestTorOrigin() can never be null (since it is initialized
to a unique origin rather than null). Because of that the check in
FrameFetchContext::populateRequestData meant that this code would never
actually set the requestor origin of a request. For requests from frames the
code in RenderFrameImpl::willSendRequest would then replace the origin with
the correct one anyway, but no such code exists for shared/service workers.

BUG= 645281 , 625969 

Review-Url: https://codereview.chromium.org/2323143002
Cr-Commit-Position: refs/heads/master@{#417712}

[modify] https://crrev.com/65299f214fad8f609c5872bcdd5f4ea82ec29713/third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-basics.html
[modify] https://crrev.com/65299f214fad8f609c5872bcdd5f4ea82ec29713/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/foreign-fetch-worker.js
[modify] https://crrev.com/65299f214fad8f609c5872bcdd5f4ea82ec29713/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/65299f214fad8f609c5872bcdd5f4ea82ec29713/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp

Comment 2 by jeffy@google.com, Sep 12 2016

Labels: Merge-Request-54

Comment 3 by dimu@chromium.org, Sep 12 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 12 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416

commit 3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416
Author: Joshua Bell <jsbell@chromium.org>
Date: Mon Sep 12 17:16:38 2016

Correctly set requestor origin for worker initiated requests.

ResourceRequest.requestTorOrigin() can never be null (since it is initialized
to a unique origin rather than null). Because of that the check in
FrameFetchContext::populateRequestData meant that this code would never
actually set the requestor origin of a request. For requests from frames the
code in RenderFrameImpl::willSendRequest would then replace the origin with
the correct one anyway, but no such code exists for shared/service workers.

BUG= 645281 , 625969 

Review-Url: https://codereview.chromium.org/2323143002
Cr-Commit-Position: refs/heads/master@{#417712}
(cherry picked from commit 65299f214fad8f609c5872bcdd5f4ea82ec29713)

Review URL: https://codereview.chromium.org/2332023002 .

Cr-Commit-Position: refs/branch-heads/2840@{#303}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-basics.html
[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/foreign-fetch-worker.js
[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416

commit 3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416
Author: Joshua Bell <jsbell@chromium.org>
Date: Mon Sep 12 17:16:38 2016

Correctly set requestor origin for worker initiated requests.

ResourceRequest.requestTorOrigin() can never be null (since it is initialized
to a unique origin rather than null). Because of that the check in
FrameFetchContext::populateRequestData meant that this code would never
actually set the requestor origin of a request. For requests from frames the
code in RenderFrameImpl::willSendRequest would then replace the origin with
the correct one anyway, but no such code exists for shared/service workers.

BUG= 645281 , 625969 

Review-Url: https://codereview.chromium.org/2323143002
Cr-Commit-Position: refs/heads/master@{#417712}
(cherry picked from commit 65299f214fad8f609c5872bcdd5f4ea82ec29713)

Review URL: https://codereview.chromium.org/2332023002 .

Cr-Commit-Position: refs/branch-heads/2840@{#303}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-basics.html
[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/foreign-fetch-worker.js
[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/3fa4d0dbbd3f3839ec00a64e5c711bc0f3658416/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp

Comment 6 by mek@chromium.org, Nov 11 2016

Status: Fixed (was: Started)

Sign in to add a comment