New issue
Advanced search Search tips

Issue 868545 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 10
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Subresource requests cannot redirect to some schemes with network service

Project Member Reported by cduvall@chromium.org, Jul 27

Issue description

With network service enabled, subresource requests can only redirect to http/https/data schemes.
 
Labels: OS-Windows
Status: Available (was: Untriaged)
Owner: cduvall@chromium.org
Status: Started (was: Available)
Labels: -Pri-2 Pri-1
Labels: OS-Android OS-Chrome OS-Linux OS-Mac
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2680c227c2594e12eaab1962fc9d9e350616a19

commit b2680c227c2594e12eaab1962fc9d9e350616a19
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Aug 10 15:27:27 2018

Enable subresource redirects to all schemes with network service

Previously, subresource requests could not redirect to schemes
other than http/https/data with the network service. Now they can
redirect to all schemes, and have proper safety checks.

This gets rid of the RedirectChecker class and instead just checks
the redirect in WebRequestProxyingURLLoaderFactory if the request
is being proxied. The checks farther downstream will be skipped if
the request is proxied. This should be simpler, and avoids complicated
logic with the RedirectChecker that would have been necessary for
subresource redirects.

The bypass_redirect_checks bit is set if the request is being proxied
by WebRequestProxyingURLLoaderFactory. In that case, all redirect
checks will happen in WebRequestProxyingURLLoaderFactory, because the
webRequest API has special permissions for allowing redirects. For
subresource requests, the bypass_redirect_checks bit is set on the
URLLoaderFactoryBundleInfo that is sent to the renderer from
RenderFrameHostImpl. This is used in ResourceDispatcher to ignore
redirect checks if necessary.

Bug:  868545 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ib11bf81e5b4b1ec9fa1b8d9178467322df91b30c
Reviewed-on: https://chromium-review.googlesource.com/1157549
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582174}
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/chrome/test/data/extensions/api_test/webrequest/test_redirects.js
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/chrome/test/data/extensions/api_test/webrequest/test_subresource_redirects.js
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/browser/loader/loader_browsertest.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/common/throttling_url_loader.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/common/throttling_url_loader.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/common/url_loader_factory_bundle.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/common/url_loader_factory_bundle.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/common/url_loader_factory_bundle.mojom
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/common/url_loader_factory_bundle_struct_traits.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/browser/BUILD.gn
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/browser/content_browser_client.h
[delete] https://crrev.com/10814c45a2507895d6a5f63e1d73fd822f0a3264/content/public/browser/redirect_checker.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/browser/render_frame_host.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/common/url_utils.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/common/url_utils.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/child_url_loader_factory_bundle.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/child_url_loader_factory_bundle.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/resource_dispatcher.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/tracked_child_url_loader_factory_bundle.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/tracked_child_url_loader_factory_bundle.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/url_loader_client_impl.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/content/renderer/loader/url_loader_client_impl.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/extensions/browser/api/web_request/web_request_api.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/extensions/shell/browser/shell_content_browser_client.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/extensions/shell/browser/shell_content_browser_client.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/services/network/public/cpp/shared_url_loader_factory.cc
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/services/network/public/cpp/shared_url_loader_factory.h
[modify] https://crrev.com/b2680c227c2594e12eaab1962fc9d9e350616a19/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8dc4e50a1217a8dbea0402753e5c7c2ef043752d

commit 8dc4e50a1217a8dbea0402753e5c7c2ef043752d
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Sep 07 01:51:12 2018

Make bypass redirect checks logic more robust

The bypass_redirects_check bit used to be set from the return value of
WillCreateURLLoaderFactory, because the WebRequest proxy was the only
proxy that would cause that to be true. Now there is also a signin proxy
which can return true, so pass in a separate bool* for
bypass_redirects_checks that can be set based on WebRequest.

Bug:  868545 
Change-Id: I361a4d89393a72552954471c8c5566bc7b1d7a17
Reviewed-on: https://chromium-review.googlesource.com/1211808
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589406}
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/content/public/browser/content_browser_client.h
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/extensions/shell/browser/shell_content_browser_client.cc
[modify] https://crrev.com/8dc4e50a1217a8dbea0402753e5c7c2ef043752d/extensions/shell/browser/shell_content_browser_client.h

Sign in to add a comment