[Extensions] Propagate request_id to CORS preflights for proxy auth [chromium/src : main]
0 views
Skip to first unread message
Andrea Orru (Gerrit)
Sep 4, 2025, 2:04:21 PM (4 days ago) Sep 4
to Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Andrea Orru added 1 comment![]( "Open in Gerrit")
File chrome/browser/extensions/api/web_request/web_request_apitest.cc
Line 6158, Patchset 1 (Latest):
EXPECT_TRUE(result_catcher.GetNextResult());Andrea Orru . unresolved
Verified this fails without the fix.
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrea Orru (Gerrit)
Sep 4, 2025, 4:00:44 PM (4 days ago) Sep 4
to Devlin Cronin, Kelvin Jiang, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Attention needed from Devlin Cronin
Andrea Orru voted Commit-Queue+1![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrea Orru (Gerrit)
Sep 4, 2025, 4:03:55 PM (4 days ago) Sep 4
to Devlin Cronin, Kelvin Jiang, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Attention needed from Devlin Cronin
Andrea Orru added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Andrea Orru . unresolved
Devlin for the general approach from an extensions perspective.
Will find a services/network owner after.
Kelvin, JFYI.
Related details
Attention is currently required from:
- Devlin Cronin
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devlin Cronin (Gerrit)
Sep 4, 2025, 5:56:25 PM (4 days ago) Sep 4
to Andrea Orru, Devlin Cronin, Kelvin Jiang, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Attention needed from Andrea Orru
Devlin Cronin voted and added 4 comments![]( "Open in Gerrit")
Votes added by Devlin Cronin
| Code-Review | +1 |
4 comments
Patchset-level comments
Devlin Cronin . resolved
LGTM % nits; thanks, Andrea!
File chrome/browser/extensions/api/web_request/web_request_apitest.cc
Line 6129, Patchset 3 (Latest):
setTimeout(() => {Devlin Cronin . unresolved
why async? (Add a comment)
Line 6136, Patchset 3 (Latest):
chrome.test.sendMessage('ready');)";Devlin Cronin . unresolved
as flagged elsewhere, we _shouldn't_ generally need these, if the extension isn't doing any async setup work.
Line 6158, Patchset 1:
EXPECT_TRUE(result_catcher.GetNextResult());Andrea Orru . unresolved
Verified this fails without the fix.
Devlin Cronin
Presumably, the failure is just a timeout? (That's fine; just confirming)
Related details
Attention is currently required from:
- Andrea Orru
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrea Orru (Gerrit)
Sep 4, 2025, 6:10:57 PM (4 days ago) Sep 4
to Takashi Toyoshima, Devlin Cronin, Kelvin Jiang, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Attention needed from Takashi Toyoshima
Andrea Orru voted and added 4 comments![]( "Open in Gerrit")
Votes added by Andrea Orru
| Commit-Queue | +1 |
Devlin for the general approach from an extensions perspective.
Will find a services/network owner after.Kelvin, JFYI.
Andrea Orru
Done
File chrome/browser/extensions/api/web_request/web_request_apitest.cc
why async? (Add a comment)
Andrea Orru
Actually it doesn't necessarily have to be async.
as flagged elsewhere, we _shouldn't_ generally need these, if the extension isn't doing any async setup work.
Andrea Orru
Yes, I think it should be fine for this test (but apparently not **every** test...).
Line 6158, Patchset 1:
EXPECT_TRUE(result_catcher.GetNextResult());Andrea Orru . resolved
Devlin CroninVerified this fails without the fix.
Presumably, the failure is just a timeout? (That's fine; just confirming)
Attention is currently required from:
- Takashi Toyoshima
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Takashi Toyoshima (Gerrit)
Sep 5, 2025, 3:56:10 AM (3 days ago) Sep 5
to Andrea Orru, Devlin Cronin, Kelvin Jiang, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Attention needed from Andrea Orru
Takashi Toyoshima voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Andrea Orru
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrea Orru (Gerrit)
Sep 5, 2025, 1:25:26 PM (3 days ago) Sep 5
to Takashi Toyoshima, Devlin Cronin, Kelvin Jiang, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Andrea Orru voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Sep 5, 2025, 1:47:34 PM (3 days ago) Sep 5
to Andrea Orru, Takashi Toyoshima, Devlin Cronin, Kelvin Jiang, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, network-ser...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[Extensions] Propagate request_id to CORS preflights for proxy auth
The `webRequest.onAuthRequired` event is not firing for extensions
handling proxy authentication (HTTP 407) on CORS preflight requests.
This is because the preflight's URLLoader is created with a default
`request_id` of 0, which the WebRequest API could not associate with
the original request being proxied by the extension.
This change fixes the issue by propagating the `request_id` from the
original `CorsURLLoader` down to the `SimpleURLLoader` used for the
preflight request. This ensures the preflight and the main request are
treated as a single transaction, allowing the extension to correctly
handle the authentication challenge.
Bug: 359904347, 40928015
Change-Id: Ia5de24c3341209c848062dd84655319e22675ebc
Reviewed-by: Devlin Cronin <rdevlin...@chromium.org>
Commit-Queue: Andrea Orru <andre...@chromium.org>
Reviewed-by: Takashi Toyoshima <toyo...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1511712}
Files:
- M chrome/browser/extensions/api/web_request/web_request_apitest.cc
- M services/network/cors/cors_url_loader.cc
- M services/network/cors/preflight_controller.cc
- M services/network/cors/preflight_controller.h
- M services/network/cors/preflight_controller_unittest.cc
Change size: M
Delta: 5 files changed, 68 insertions(+), 9 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Takashi Toyoshima, +1 by Devlin Cronin
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages