Network Service: 302 redirects are not dropping the POST data |
||||||||
Issue descriptionScott today mentioned that he's seeing errors on bugs.chromium.org when he submits a comment. It only failed with network service. I used https://bugs.chromium.org/p/chromium/issues/detail?id=923573 to test this, feel free to spam it. The issue is that when submitting a comment, the browser POSTS to the url. The server then does a 302 redirect. 302s are supposed to lose the body and always switch to GET (only 307s keep the method). We are correctly switching the method to GET on the redirect, but the body is still there. I got lucky in diagnosing this because it was a regression between the two most recent beta builds. It also needs an extension with webRequest installed. It's due to https://chromium.googlesource.com/chromium/src/+/066fecfb1d2b12aa2359ade87ef2f036437cfaec, which has already been merged to 72. If there's a quick fix for this, then we can try merging it before the next beta build on Tuesday. I think that'll be too late though, in which case we might want to just revert the cl (at least on 72 branch) before Tuesday's 72 build is done.
,
Jan 19
(3 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4922fd55ae6ed13071fc457529eb87f61ef8f25b commit 4922fd55ae6ed13071fc457529eb87f61ef8f25b Author: Clark DuVall <cduvall@chromium.org> Date: Sat Jan 19 06:28:37 2019 Clear request body on redirect if new method is GET in webRequest proxy This is similar to what is done in CorsURLLoader: https://cs.chromium.org/chromium/src/services/network/cors/cors_url_loader.cc?l=138&rcl=cc2c755f47023edd66e89d836e467e505d124320 Bug: 923622 Change-Id: I26e15c250b08a0fa0c1f6fcdd1ceb21b776cae89 Reviewed-on: https://chromium-review.googlesource.com/c/1423919 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Clark DuVall <cduvall@chromium.org> Cr-Commit-Position: refs/heads/master@{#624454} [modify] https://crrev.com/4922fd55ae6ed13071fc457529eb87f61ef8f25b/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
,
Yesterday
(43 hours ago)
Tested this issue on Windows 10 and Mac OS 10.13.6 on the build without fix 73.0.3670.0 and latest M-73 73.0.3679.0 build by following the below steps. 1. Launched Chrome and navigated to https://bugs.chromium.org/p/chromium/issues/detail?id=923573. 2. Added a comment and couldn't observe any 302 error when submitting a comment on crbug. Attached is the screen cast for reference. cduvall@ Request you to check and confirm if anything is missed from our end in verifying the issue and help us to verify the fix on the latest M-73 build. Thanks...
,
Yesterday
(31 hours ago)
to reproduce this you need to install an extension that uses webrequest, eg Adblock.
,
Today
(21 hours ago)
I can confirm that this started to happen in chrome 72 https://i.imgur.com/g3cO2cr.png https://www.dropbox.com/s/9iq5y0pl4ojdi2v/2019-01-22_12-12-48.mp4?dl=0
,
Today
(17 hours ago)
Re-tested this issue on Windows 10 as per comment #4 on the build without fix 73.0.3650.0 and somehow unable to reproduce the issue by enabling the #network-service flag and adding AdBlock extension. Attached is the screen cast for reference. cduvall@ Could you please check and help us in verifying the fix on Dev RC 73.0.3679.0. Thanks...
,
Today
(14 hours ago)
If you take a packet capture, you can see the request body from the POST request in the redirected GET request. (FYI, this is actually a serious security risk, because many login forms that use POST method to send credentials respond with a redirect, that then receives a request body (containing username/password etc) that is not intended for this destination)
,
Today
(12 hours ago)
,
Today
(12 hours ago)
The reason you were probably not able to repro is that this bug only affects chrome versions between 73.0.3672.0 and 73.0.3678.0 (and the latest beta which had 066fecfb1d2b12aa2359ade87ef2f036437cfaec merged). I verified this on the latest canary. Requesting a merge to M72 for change in comment #2, this only affects the network service code path.
,
Today
(12 hours ago)
This bug requires manual review: We are only 6 days from stable. Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Today
(12 hours ago)
branch:3626
,
Today
(12 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2893bb53a0bd5a0829d21b630d3a3266ede8037 commit d2893bb53a0bd5a0829d21b630d3a3266ede8037 Author: Clark DuVall <cduvall@chromium.org> Date: Tue Jan 22 17:47:58 2019 Clear request body on redirect if new method is GET in webRequest proxy This is similar to what is done in CorsURLLoader: https://cs.chromium.org/chromium/src/services/network/cors/cors_url_loader.cc?l=138&rcl=cc2c755f47023edd66e89d836e467e505d124320 Bug: 923622 Change-Id: I26e15c250b08a0fa0c1f6fcdd1ceb21b776cae89 Reviewed-on: https://chromium-review.googlesource.com/c/1423919 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Clark DuVall <cduvall@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#624454}(cherry picked from commit 4922fd55ae6ed13071fc457529eb87f61ef8f25b) Reviewed-on: https://chromium-review.googlesource.com/c/1427019 Reviewed-by: Clark DuVall <cduvall@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#752} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/d2893bb53a0bd5a0829d21b630d3a3266ede8037/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
,
Today
(12 hours ago)
,
Today
(12 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2893bb53a0bd5a0829d21b630d3a3266ede8037 Commit: d2893bb53a0bd5a0829d21b630d3a3266ede8037 Author: cduvall@chromium.org Commiter: cduvall@chromium.org Date: 2019-01-22 17:47:58 +0000 UTC Clear request body on redirect if new method is GET in webRequest proxy This is similar to what is done in CorsURLLoader: https://cs.chromium.org/chromium/src/services/network/cors/cors_url_loader.cc?l=138&rcl=cc2c755f47023edd66e89d836e467e505d124320 Bug: 923622 Change-Id: I26e15c250b08a0fa0c1f6fcdd1ceb21b776cae89 Reviewed-on: https://chromium-review.googlesource.com/c/1423919 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Clark DuVall <cduvall@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#624454}(cherry picked from commit 4922fd55ae6ed13071fc457529eb87f61ef8f25b) Reviewed-on: https://chromium-review.googlesource.com/c/1427019 Reviewed-by: Clark DuVall <cduvall@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#752} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by jam@chromium.org
, Jan 19 (4 days ago)