Shivani Sharma (Gerrit)

unread,

Jun 10, 2026, 4:07:23 PM (14 days ago) Jun 10

to Andrew Verge, Nate Chapin, android-bu...@system.gserviceaccount.com, Blink W3C Test Autoroller, Mike West, Chromium LUCI CQ, loading...@chromium.org, blink-...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, gavinp...@chromium.org

Attention needed from Andrew Verge

Shivani Sharma added 2 comments

File third_party/blink/renderer/core/loader/base_fetch_context.cc

Line 357, Patchset 3 (Latest): // occurred.

Shivani Sharma . unresolved

nit: "occurred in the network service URLLoaderFactory checks"

Line 365, Patchset 3 (Latest):

      if (policies.connection_allowlists.enforced.has_value()) {
        if (!network::ConnectionAllowlistMatchesUrl(
                policies.connection_allowlists.enforced.value(), GURL(url))) {
          if (reporting_disposition == ReportingDisposition::kReport) {
            PrintAccessDeniedMessage(url);
            ConnectionAllowlistViolationReportBody::QueueServiceWorkerReport(
                url, V8ConnectionAllowlistDisposition::Enum::kEnforce,
                *GetExecutionContext());
          }
          return ResourceRequestBlockedReason::kOther;
        }
      }
      if (policies.connection_allowlists.report_only.has_value()) {
        if (!network::ConnectionAllowlistMatchesUrl(
                policies.connection_allowlists.report_only.value(),
                GURL(url))) {
          if (reporting_disposition == ReportingDisposition::kReport) {
            PrintAccessDeniedMessage(url);
            ConnectionAllowlistViolationReportBody::QueueServiceWorkerReport(
                url, V8ConnectionAllowlistDisposition::Enum::kReport,
                *GetExecutionContext());
          }

Shivani Sharma . unresolved

something like below to avoid duplication of code?

bool report_only = policies.connection_allowlists.report_only.has_value()
bool enforce = policies.connection_allowlists.enforced.has_value()

if (enforce || report_only) {
  if (!network::ConnectionAllowlistMatchesUrl(
                enforce? policies.connection_allowlists.enforced.value()
                :policies.connection_allowlists.report_only.value(), GURL(url))) {
          if (reporting_disposition == ReportingDisposition::kReport) {
            PrintAccessDeniedMessage(url);
            ConnectionAllowlistViolationReportBody::QueueServiceWorkerReport(
                url, 
                enforce? V8ConnectionAllowlistDisposition::Enum::kEnforce
                : V8ConnectionAllowlistDisposition::Enum::kReport,
                *GetExecutionContext());
          }
          if (enforce) {
            return ResourceRequestBlockedReason::kOther;
          }
      }   
}

Open in Gerrit

Related details

Attention is currently required from:

Andrew Verge

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Andrew Verge (Gerrit)

unread,

Jun 10, 2026, 5:23:52 PM (14 days ago) Jun 10

to Nate Chapin, android-bu...@system.gserviceaccount.com, Blink W3C Test Autoroller, Mike West, Chromium LUCI CQ, Shivani Sharma, loading...@chromium.org, blink-...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, gavinp...@chromium.org

Attention needed from Shivani Sharma

Andrew Verge added 2 comments

File third_party/blink/renderer/core/loader/base_fetch_context.cc

Line 357, Patchset 3: // occurred.

Shivani Sharma . resolved

nit: "occurred in the network service URLLoaderFactory checks"

Andrew Verge

Done

Line 365, Patchset 3: if (policies.connection_allowlists.enforced.has_value()) {


        if (!network::ConnectionAllowlistMatchesUrl(
                policies.connection_allowlists.enforced.value(), GURL(url))) {
          if (reporting_disposition == ReportingDisposition::kReport) {
            PrintAccessDeniedMessage(url);
            ConnectionAllowlistViolationReportBody::QueueServiceWorkerReport(
                url, V8ConnectionAllowlistDisposition::Enum::kEnforce,
                *GetExecutionContext());
          }
          return ResourceRequestBlockedReason::kOther;
        }
      }
      if (policies.connection_allowlists.report_only.has_value()) {
        if (!network::ConnectionAllowlistMatchesUrl(
                policies.connection_allowlists.report_only.value(),
                GURL(url))) {
          if (reporting_disposition == ReportingDisposition::kReport) {
            PrintAccessDeniedMessage(url);
            ConnectionAllowlistViolationReportBody::QueueServiceWorkerReport(
                url, V8ConnectionAllowlistDisposition::Enum::kReport,
                *GetExecutionContext());
          }

Shivani Sharma . resolved

something like below to avoid duplication of code?

bool report_only = policies.connection_allowlists.report_only.has_value()
bool enforce = policies.connection_allowlists.enforced.has_value()

if (enforce || report_only) {
  if (!network::ConnectionAllowlistMatchesUrl(
                enforce? policies.connection_allowlists.enforced.value()
                :policies.connection_allowlists.report_only.value(), GURL(url))) {
          if (reporting_disposition == ReportingDisposition::kReport) {
            PrintAccessDeniedMessage(url);
            ConnectionAllowlistViolationReportBody::QueueServiceWorkerReport(
                url, 
                enforce? V8ConnectionAllowlistDisposition::Enum::kEnforce
                : V8ConnectionAllowlistDisposition::Enum::kReport,
                *GetExecutionContext());
          }
          if (enforce) {
            return ResourceRequestBlockedReason::kOther;
          }
      }   
}

Andrew Verge

In theory both headers can be present, and unifying the code this way only checks one of them. I used a lambda to make this a bit more DRY.

Open in Gerrit

Related details

Attention is currently required from:

Shivani Sharma

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Shivani Sharma (Gerrit)

unread,

Jun 10, 2026, 5:31:38 PM (14 days ago) Jun 10

to Andrew Verge, Nate Chapin, android-bu...@system.gserviceaccount.com, Blink W3C Test Autoroller, Mike West, Chromium LUCI CQ, loading...@chromium.org, blink-...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, gavinp...@chromium.org

Attention needed from Andrew Verge

Shivani Sharma voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Andrew Verge

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Andrew Verge (Gerrit)

unread,

Jun 10, 2026, 5:34:39 PM (14 days ago) Jun 10

to Shivani Sharma, Nate Chapin, android-bu...@system.gserviceaccount.com, Blink W3C Test Autoroller, Mike West, Chromium LUCI CQ, loading...@chromium.org, blink-...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, gavinp...@chromium.org

Andrew Verge voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jun 10, 2026, 7:46:31 PM (14 days ago) Jun 10

to Andrew Verge, Shivani Sharma, Nate Chapin, android-bu...@system.gserviceaccount.com, Blink W3C Test Autoroller, Mike West, loading...@chromium.org, blink-...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, gavinp...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

[Connection-Allowlist] Reporting for Document->ServiceWorker fetches.

If a document is controlled by a Service Worker, its own Connection-
Allowlist headers will be checked in Blink before delegating the fetch
to the worker. If this fetch would violate the document's allowlist,
reports are now sent to the proper reporting endpoint.

Bug: 514301027

Fixed: 514301027

Change-Id: I717606f6c5da4b01bfdd1ab80e5c202f9054d7c2

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7913564

Reviewed-by: Mike West <mk...@chromium.org>

Reviewed-by: Shivani Sharma <shiva...@chromium.org>

Commit-Queue: Andrew Verge <ave...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1644978}

Files:

M third_party/blink/renderer/core/frame/connection_allowlist_violation_report_body.cc
M third_party/blink/renderer/core/frame/connection_allowlist_violation_report_body.h
M third_party/blink/renderer/core/loader/base_fetch_context.cc
A third_party/blink/web_tests/external/wpt/connection-allowlist/tentative/resources/service-worker-fetch-script-allow-all.js
A third_party/blink/web_tests/external/wpt/connection-allowlist/tentative/resources/service-worker-fetch-script-allow-all.js.headers
A third_party/blink/web_tests/external/wpt/connection-allowlist/tentative/service-worker-reporting.sub.https.window.js
A third_party/blink/web_tests/external/wpt/connection-allowlist/tentative/service-worker-reporting.sub.https.window.js.sub.headers

Change size: L

Delta: 7 files changed, 227 insertions(+), 49 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Shivani Sharma, +1 by Mike West

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Blink W3C Test Autoroller (Gerrit)

unread,

Jun 11, 2026, 12:34:13 AM (13 days ago) Jun 11

to Andrew Verge, Chromium LUCI CQ, Shivani Sharma, Nate Chapin, android-bu...@system.gserviceaccount.com, Mike West, loading...@chromium.org, blink-...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, gavinp...@chromium.org

Message from Blink W3C Test Autoroller

The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/60531

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[Connection-Allowlist] Reporting for Document->ServiceWorker fetches. [chromium/src : main]

Shivani Sharma (Gerrit)

Shivani Sharma added 2 comments

Related details

Andrew Verge (Gerrit)

Andrew Verge added 2 comments

Related details

Shivani Sharma (Gerrit)

Shivani Sharma voted Code-Review+1

Related details

Andrew Verge (Gerrit)

Andrew Verge voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Blink W3C Test Autoroller (Gerrit)

Message from Blink W3C Test Autoroller

Related details