DevTools: validate unsafe headers [chromium/src : main]
0 views
Skip to first unread message
Alex Rudenko (Gerrit)
Nov 8, 2024, 3:52:23 AMNov 8
to AyeAye, Andrey Kosyakov, chromium...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org
Attention needed from Andrey Kosyakov
Alex Rudenko added 1 comment![]( "Open in Gerrit")
File content/browser/devtools/devtools_url_loader_interceptor.cc
Line 1779, Patchset 1:
state_ == State::kResponseTaken || state_ == State::kRedirectReceived)Alex Rudenko . resolved
Andrey KosyakovI think that something looks broken but I have not investigated what it could be yet
Alex RudenkoYeah, it looks like it can easily happen in response to FollowRedirect():
https://source.chromium.org/chromium/chromium/src/+/main:services/network/url_loader.cc;l=1281;drc=0afc9ac9afcaab79fc54299039f4d27abf3a086d;bpv=1;bpt=1Any chance you could reproduce hitting this in a test?
I wonder if we should provide some error response to client in such case.
Oh, right, good point. Added a test and validation.
Related details
Attention is currently required from:
- Andrey Kosyakov
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Nov 8, 2024, 3:52:39 AMNov 8
to AyeAye, Andrey Kosyakov, chromium...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org
Attention needed from Andrey Kosyakov
Alex Rudenko added 1 comment![]( "Open in Gerrit")
Commit Message
Line 10, Patchset 1:
Or is something broken here?Andrey Kosyakov . resolved
Alex RudenkoPlease update this before landing.
Done
Related details
Attention is currently required from:
- Andrey Kosyakov
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Nov 12, 2024, 2:08:20 PMNov 12
to Alex Rudenko, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org
Attention needed from Alex Rudenko
Andrey Kosyakov voted and added 3 comments![]( "Open in Gerrit")
Votes added by Andrey Kosyakov
| Code-Review | +1 |
3 comments
Patchset-level comments
File content/browser/devtools/protocol/fetch_handler.cc
Line 175, Patchset 5 (Latest):
template <typename Callback>Andrey Kosyakov . unresolved
You don't really need it templated, given it's only called once.
Line 287, Patchset 5 (Latest):
if (!ValidateHeadersForRequest(entry.get(), callback.get())) {Andrey Kosyakov . unresolved
Should we just move `ValidateHeaders()` call above into `ValidateHeadersForRequest()`?
Related details
Attention is currently required from:
- Alex Rudenko
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Nov 12, 2024, 3:36:42 PMNov 12
to Andrey Kosyakov, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org
Alex Rudenko added 2 comments![]( "Open in Gerrit")
File content/browser/devtools/protocol/fetch_handler.cc
You don't really need it templated, given it's only called once.
Alex Rudenko
Done
Line 287, Patchset 5:
if (!ValidateHeadersForRequest(entry.get(), callback.get())) {Andrey Kosyakov . resolved
Should we just move `ValidateHeaders()` call above into `ValidateHeadersForRequest()`?
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Nov 13, 2024, 2:07:53 AMNov 13
to Andrey Kosyakov, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org
Alex Rudenko voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Nov 13, 2024, 2:11:13 AMNov 13
to Alex Rudenko, Andrey Kosyakov, AyeAye, chromium...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
5 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: content/browser/devtools/protocol/fetch_handler.cc
Insertions: 6, Deletions: 4.
The diff is too large to show. Please review the diff.
```
Change information
Commit message:
DevTools: validate unsafe headers
If request headers that are considered to be unsafe by the network
service are used in DevTools request interceptions, DCHECKs will be
hit. This CL adds validation for request headers in continueRequest to
disallow unsafe headers.
Fixed: 40864248
Change-Id: I4f33240415fec01c7686a1437b2ca6fb29c1fe8e
Reviewed-by: Andrey Kosyakov <ca...@chromium.org>
Commit-Queue: Alex Rudenko <alexr...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1382169}
Files:
- M content/browser/devtools/protocol/fetch_handler.cc
- A third_party/blink/web_tests/http/tests/inspector-protocol/fetch/redirect-headers-override-unsafe-expected.txt
- A third_party/blink/web_tests/http/tests/inspector-protocol/fetch/redirect-headers-override-unsafe.js
Change size: M
Delta: 3 files changed, 59 insertions(+), 1 deletion(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Andrey Kosyakov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages