Set Sec-Fetch-Dest to email-verification for those requests [chromium/src : main]
4 views
Skip to first unread message
Nicolás Peña (Gerrit)
Oct 30, 2025, 1:30:17 PM (7 days ago) Oct 30
to Christian Biesinger, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger
Nicolás Peña voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Christian Biesinger
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christian Biesinger (Gerrit)
Oct 30, 2025, 2:34:16 PM (7 days ago) Oct 30
to Nicolás Peña, Chromium LUCI CQ, Christian Biesinger, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Nicolás Peña
Christian Biesinger added 7 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Christian Biesinger . resolved
You should probably add email verification to https://source.chromium.org/chromium/chromium/src/+/main:services/network/cors/cors_url_loader_factory.cc;l=767 as well
Commit Message
Line 7, Patchset 2 (Latest):
Set Sec-Fetch-Dest to email-verification for those requestsChristian Biesinger . unresolved
Maybe add a link to https://github.com/WICG/email-verification-protocol in the description
File content/browser/webid/idp_network_request_manager.h
Line 371, Patchset 2 (Latest):
network::mojom::RequestDestination destination,Christian Biesinger . unresolved
why this change? isn't this always WebIdentity?
File content/browser/webid/idp_network_request_manager.cc
Line 1044, Patchset 2 (Latest):
network::mojom::RequestDestination::kWebIdentity,Christian Biesinger . unresolved
I almost wonder if this should be a constructor argument, since the IDP subclass always uses webidentity and the email verification subclass alwas uses email verification
File content/browser/webid/idp_network_request_manager_unittest.cc
Line 979, Patchset 2 (Latest):
TEST_F(IdpNetworkRequestManagerTest, FetchWellKnownRequestDestination) {Christian Biesinger . unresolved
guess these can't hurt but fwiw we do check this in wpt as well (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/fedcm/support/request-params-check.py;l=1?q=commonchec%20fedcm&ss=chromium)
File services/network/public/cpp/request_destination.cc
Line 47, Patchset 2 (Latest):
constexpr char kEmailVerification[] = "email-verification";Christian Biesinger . unresolved
No other destination uses a dash, I don't think this one should be the exception
File third_party/blink/renderer/core/fetch/request.idl
Line 28, Patchset 2 (Latest):
"email-verification",Christian Biesinger . unresolved
Weird, not sure why https://crrev.com/c/5925915 added webidentity here, the renderer should never see it.
Anyway, this should probably not be under the FedCM heading, maybe link to https://github.com/WICG/email-verification-protocol ?
Related details
Attention is currently required from:
- Nicolás Peña
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolás Peña (Gerrit)
Oct 30, 2025, 6:42:50 PM (7 days ago) Oct 30
to Chromium LUCI CQ, Christian Biesinger, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger
Nicolás Peña voted and added 7 comments![]( "Open in Gerrit")
Votes added by Nicolás Peña
| Commit-Queue | +1 |
Christian Biesinger . resolved
You should probably add email verification to https://source.chromium.org/chromium/chromium/src/+/main:services/network/cors/cors_url_loader_factory.cc;l=767 as well
Nicolás Peña
Done
Commit Message
Line 7, Patchset 2:
Set Sec-Fetch-Dest to email-verification for those requestsChristian Biesinger . resolved
Maybe add a link to https://github.com/WICG/email-verification-protocol in the description
Nicolás Peña
Done
File content/browser/webid/idp_network_request_manager.h
why this change? isn't this always WebIdentity?
Nicolás Peña
Done
File content/browser/webid/idp_network_request_manager.cc
Line 1044, Patchset 2:
network::mojom::RequestDestination::kWebIdentity,Christian Biesinger . resolved
I almost wonder if this should be a constructor argument, since the IDP subclass always uses webidentity and the email verification subclass alwas uses email verification
Nicolás Peña
Oh good idea. Done
File content/browser/webid/idp_network_request_manager_unittest.cc
Line 979, Patchset 2:
TEST_F(IdpNetworkRequestManagerTest, FetchWellKnownRequestDestination) {Christian Biesinger . resolved
guess these can't hurt but fwiw we do check this in wpt as well (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/fedcm/support/request-params-check.py;l=1?q=commonchec%20fedcm&ss=chromium)
Nicolás Peña
Acknowledged
File services/network/public/cpp/request_destination.cc
Line 47, Patchset 2:
constexpr char kEmailVerification[] = "email-verification";Christian Biesinger . unresolved
No other destination uses a dash, I don't think this one should be the exception
Nicolás Peña
Hmm maybe it should be `emailverification`?
File third_party/blink/renderer/core/fetch/request.idl
Weird, not sure why https://crrev.com/c/5925915 added webidentity here, the renderer should never see it.
Anyway, this should probably not be under the FedCM heading, maybe link to https://github.com/WICG/email-verification-protocol ?
Nicolás Peña
I guess we can add notreached for these in the converter since it is only used for the request destination attribute getter which should never return these... Done
Related details
Attention is currently required from:
- Christian Biesinger
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christian Biesinger (Gerrit)
Oct 31, 2025, 4:55:16 PM (6 days ago) Oct 31
to Nicolás Peña, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Nicolás Peña
Christian Biesinger voted and added 2 comments![]( "Open in Gerrit")
Votes added by Christian Biesinger
| Code-Review | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 4 (Latest):
Christian Biesinger . resolved
lgtm.
Now that https://crrev.com/c/7017375 has landed you could consider adding kEmailVerification here as well:
https://chromium-review.googlesource.com/c/chromium/src/+/7017375/20/services/network/url_loader_util.cc
https://chromium-review.googlesource.com/c/chromium/src/+/7017375/20/services/network/cors/cors_url_loader_factory.cc
(and maybe https://chromium-review.googlesource.com/c/chromium/src/+/7017375/20/content/browser/devtools/devtools_url_loader_interceptor.cc)
File services/network/public/cpp/request_destination.cc
Line 47, Patchset 2:
constexpr char kEmailVerification[] = "email-verification";Christian Biesinger . unresolved
Nicolás PeñaNo other destination uses a dash, I don't think this one should be the exception
Hmm maybe it should be `emailverification`?
Attention is currently required from:
- Nicolás Peña
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolás Peña (Gerrit)
Oct 31, 2025, 5:17:16 PM (6 days ago) Oct 31
to Matthew Denton, Colin Blundell, Sophie Chang, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger, Colin Blundell, Matthew Denton and Sophie Chang
Nicolás Peña added 3 comments![]( "Open in Gerrit")
Patchset-level comments
Christian Biesinger . resolved
lgtm.
Now that https://crrev.com/c/7017375 has landed you could consider adding kEmailVerification here as well:
https://chromium-review.googlesource.com/c/chromium/src/+/7017375/20/services/network/url_loader_util.cc
https://chromium-review.googlesource.com/c/chromium/src/+/7017375/20/services/network/cors/cors_url_loader_factory.cc
(and maybe https://chromium-review.googlesource.com/c/chromium/src/+/7017375/20/content/browser/devtools/devtools_url_loader_interceptor.cc)
Nicolás Peña
I don't follow what the last one does but the other two done
File-level comment, Patchset 5 (Latest):
Nicolás Peña . resolved
PTAL
mpdenton@: third_party/blink/renderer/core/fetch/request_util.cc and services/network/public/mojom/fetch_api.mojom
blundell@: rest of services/network and components/services/storage/service_worker
sophiechang@: chrome/browser/predictors
File services/network/public/cpp/request_destination.cc
Line 47, Patchset 2:
constexpr char kEmailVerification[] = "email-verification";Christian Biesinger . resolved
Nicolás PeñaNo other destination uses a dash, I don't think this one should be the exception
Christian BiesingerHmm maybe it should be `emailverification`?
Yeah I think that would be better.
Attention is currently required from:
- Christian Biesinger
- Colin Blundell
- Matthew Denton
- Sophie Chang
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolás Peña (Gerrit)
Oct 31, 2025, 5:18:34 PM (6 days ago) Oct 31
to Kelvin Jiang, Matthew Denton, Colin Blundell, Sophie Chang, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger, Colin Blundell, Kelvin Jiang, Matthew Denton and Sophie Chang
Nicolás Peña voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nicolás Peña
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Nicolás Peña . resolved
Missed one :( kelvinjiang@ PTAL extensions/browser/api/web_request/ web_request_resource_type.cc
Related details
Attention is currently required from:
- Christian Biesinger
- Colin Blundell
- Kelvin Jiang
- Matthew Denton
- Sophie Chang
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sophie Chang (Gerrit)
Oct 31, 2025, 5:26:25 PM (6 days ago) Oct 31
to Nicolás Peña, Kelvin Jiang, Matthew Denton, Colin Blundell, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger, Colin Blundell, Kelvin Jiang, Matthew Denton and Nicolás Peña
Sophie Chang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Sophie Chang
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Christian Biesinger
- Colin Blundell
- Kelvin Jiang
- Matthew Denton
- Nicolás Peña
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Matthew Denton (Gerrit)
Oct 31, 2025, 6:04:47 PM (6 days ago) Oct 31
to Nicolás Peña, Sophie Chang, Kelvin Jiang, Colin Blundell, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger, Colin Blundell, Kelvin Jiang and Nicolás Peña
Matthew Denton added 1 comment![]( "Open in Gerrit")
File services/network/cors/cors_url_loader_factory.cc
Line 767, Patchset 5 (Latest):
// Only the browser process is allowed to initiate FedCM or email
// verification requests.Matthew Denton . unresolved
Can we do an allowlist here instead of a denylist? Otherwise it seems quite easy to miss adding something here.
Related details
Attention is currently required from:
- Christian Biesinger
- Colin Blundell
- Kelvin Jiang
- Nicolás Peña
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Nov 3, 2025, 3:22:38 AM (3 days ago) Nov 3
to Nicolás Peña, Tsuyoshi Horo, Sophie Chang, Kelvin Jiang, Matthew Denton, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org, Colin Blundell
Attention needed from Christian Biesinger, Kelvin Jiang, Nicolás Peña and Tsuyoshi Horo
Colin Blundell added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Colin Blundell . resolved
Thanks! ->horo@ as closer OWNER of //components/services/storage and //services/network
Related details
Attention is currently required from:
- Christian Biesinger
- Kelvin Jiang
- Nicolás Peña
- Tsuyoshi Horo
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolás Peña (Gerrit)
Nov 3, 2025, 12:47:16 PM (3 days ago) Nov 3
to Tsuyoshi Horo, Sophie Chang, Kelvin Jiang, Matthew Denton, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger, Kelvin Jiang, Matthew Denton and Tsuyoshi Horo
Nicolás Peña voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nicolás Peña
| Commit-Queue | +1 |
1 comment
File services/network/cors/cors_url_loader_factory.cc
Line 767, Patchset 5:
// Only the browser process is allowed to initiate FedCM or email
// verification requests.Matthew Denton . resolved
Can we do an allowlist here instead of a denylist? Otherwise it seems quite easy to miss adding something here.
Nicolás Peña
Done but I don't know what should belong in the allowlist. And for the purpose of not introducing changes in this CL, I have added all except the two we checked below.
Related details
Attention is currently required from:
- Christian Biesinger
- Kelvin Jiang
- Matthew Denton
- Tsuyoshi Horo
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kelvin Jiang (Gerrit)
Nov 3, 2025, 5:48:26 PM (3 days ago) Nov 3
to Nicolás Peña, Tsuyoshi Horo, Sophie Chang, Matthew Denton, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger, Matthew Denton, Nicolás Peña and Tsuyoshi Horo
Kelvin Jiang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kelvin Jiang
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Christian Biesinger
- Matthew Denton
- Nicolás Peña
- Tsuyoshi Horo
Matthew Denton (Gerrit)
Nov 3, 2025, 6:36:40 PM (3 days ago) Nov 3
to Nicolás Peña, Kelvin Jiang, Tsuyoshi Horo, Sophie Chang, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger, Nicolás Peña and Tsuyoshi Horo
Matthew Denton voted and added 2 comments![]( "Open in Gerrit")
Votes added by Matthew Denton
| Code-Review | +1 |
2 comments
File services/network/cors/cors_url_loader_factory.cc
Line 767, Patchset 5:
// Only the browser process is allowed to initiate FedCM or email
// verification requests.Matthew Denton . resolved
Nicolás PeñaCan we do an allowlist here instead of a denylist? Otherwise it seems quite easy to miss adding something here.
Done but I don't know what should belong in the allowlist. And for the purpose of not introducing changes in this CL, I have added all except the two we checked below.
Matthew Denton
Sounds good, thanks!
Line 768, Patchset 6 (Latest):
// Allowed destinations from the browser process:Matthew Denton . unresolved
I think this should say unprivileged process or something
```suggestion
// Allowed destinations from unprivileged process:
```
Related details
Attention is currently required from:
- Christian Biesinger
- Nicolás Peña
- Tsuyoshi Horo
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tsuyoshi Horo (Gerrit)
Nov 3, 2025, 10:26:14 PM (2 days ago) Nov 3
to Nicolás Peña, Matthew Denton, Kelvin Jiang, Sophie Chang, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger and Nicolás Peña
Tsuyoshi Horo voted and added 3 comments![]( "Open in Gerrit")
Votes added by Tsuyoshi Horo
| Code-Review | +1 |
3 comments
Patchset-level comments
Tsuyoshi Horo . resolved
lgtm with some nits.
File services/network/cors/cors_url_loader_factory.cc
Line 797, Patchset 6 (Latest):
default:Tsuyoshi Horo . unresolved
Could you please avoid using a default case here? That way, we'll be forced to update this code whenever a new RequestDestination is added.
File third_party/blink/renderer/core/fetch/request.idl
Line 26, Patchset 6 (Parent):
// https://w3c-fedid.github.io/FedCM/
"webidentity",Tsuyoshi Horo . unresolved
I believe you are removing webidentity here because the RequestDestination for webidentity is never set on the Blink side. Please write about it in the commit comment.
Related details
Attention is currently required from:
- Christian Biesinger
- Nicolás Peña
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolás Peña (Gerrit)
Nov 4, 2025, 10:27:27 AM (2 days ago) Nov 4
to Tsuyoshi Horo, Matthew Denton, Kelvin Jiang, Sophie Chang, Christian Biesinger, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger
Nicolás Peña voted and added 3 comments![]( "Open in Gerrit")
Votes added by Nicolás Peña
| Commit-Queue | +2 |
3 comments
File services/network/cors/cors_url_loader_factory.cc
I think this should say unprivileged process or something
```suggestion
// Allowed destinations from unprivileged process:
```
Nicolás Peña
Done
Could you please avoid using a default case here? That way, we'll be forced to update this code whenever a new RequestDestination is added.
Nicolás Peña
Done
File third_party/blink/renderer/core/fetch/request.idl
Line 26, Patchset 6 (Parent):
// https://w3c-fedid.github.io/FedCM/
"webidentity",Tsuyoshi Horo . resolved
I believe you are removing webidentity here because the RequestDestination for webidentity is never set on the Blink side. Please write about it in the commit comment.
Nicolás Peña
Done
Related details
Attention is currently required from:
- Christian Biesinger
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Nov 4, 2025, 11:17:23 AM (2 days ago) Nov 4
to Nicolás Peña, Tsuyoshi Horo, Matthew Denton, Kelvin Jiang, Sophie Chang, Christian Biesinger, chromium...@chromium.org, Kaan Icer, Nate Chapin, blink-...@chromium.org, chromium-a...@chromium.org, dmurph+watching...@chromium.org, edgesto...@microsoft.com, extension...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org, network-ser...@chromium.org, npm+...@chromium.org, storage...@chromium.org, yigu+...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: services/network/cors/cors_url_loader_factory.cc
Insertions: 3, Deletions: 2.
@@ -765,7 +765,7 @@
}
switch (request.destination) {
- // Allowed destinations from the browser process:
+ // Allowed destinations from unprivileged process:
case network::mojom::RequestDestination::kEmpty:
case network::mojom::RequestDestination::kAudio:
case network::mojom::RequestDestination::kAudioWorklet:
@@ -794,7 +794,8 @@
case network::mojom::RequestDestination::kJson:
case network::mojom::RequestDestination::kSharedStorageWorklet:
break;
- default:
+ case network::mojom::RequestDestination::kWebIdentity:
+ case network::mojom::RequestDestination::kEmailVerification:
mojo::ReportBadMessage(
"CorsURLLoaderFactory: attempt to use forbidden destination from "
"renderer");
```
Change information
Commit message:
Set Sec-Fetch-Dest to email-verification for those requests
This CL aligns the implementation with the design described in
https://github.com/WICG/email-verification-protocol.
Also remove 'webidentity' from request.idl on Blink, since it is never
set on the Blink side.
Fixed: 452079347
Change-Id: I6d2f4e5d36f0a3e0fafd64a8aa326c283a7cd898
Reviewed-by: Sophie Chang <sophi...@chromium.org>
Reviewed-by: Tsuyoshi Horo <ho...@chromium.org>
Reviewed-by: Kelvin Jiang <kelvi...@chromium.org>
Reviewed-by: Matthew Denton <mpde...@chromium.org>
Commit-Queue: Nicolás Peña <n...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1540068}
Files:
- M chrome/browser/predictors/loading_predictor_tab_helper.cc
- M chrome/browser/predictors/prefetch_manager.cc
- M components/services/storage/service_worker/service_worker_database.cc
- M components/services/storage/service_worker/service_worker_database.proto
- M content/browser/webid/delegation/email_verifier_network_request_manager.cc
- A content/browser/webid/delegation/email_verifier_network_request_manager_unittest.cc
- M content/browser/webid/idp_network_request_manager.cc
- M content/browser/webid/idp_network_request_manager_unittest.cc
- M content/browser/webid/network_request_manager.cc
- M content/browser/webid/network_request_manager.h
- A content/browser/webid/network_request_manager_unittest.cc
- M content/test/BUILD.gn
- M extensions/browser/api/web_request/web_request_resource_type.cc
- M services/network/cors/cors_url_loader_factory.cc
- M services/network/cors/cors_url_loader_factory_unittest.cc
- M services/network/public/cpp/request_destination.cc
- M services/network/public/mojom/fetch_api.mojom
- M services/network/url_loader_util.cc
- M third_party/blink/renderer/core/fetch/request.cc
- M third_party/blink/renderer/core/fetch/request.idl
- M third_party/blink/renderer/core/fetch/request_util.cc
- M third_party/blink/renderer/platform/loader/fetch/fetch_utils.cc
Change size: L
Delta: 22 files changed, 372 insertions(+), 32 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Tsuyoshi Horo, +1 by Matthew Denton, +1 by Kelvin Jiang, +1 by Sophie Chang
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages