Chrome · Proxy › Net
CVE-2026-113040
Input Validation in Proxy (Net)
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifnet/proxy_resolution/configured_proxy_resolution_request.cc |
modified | |
ifnet/proxy_resolution/configured_proxy_resolution_service.cc |
modified | |
SyncFailProxyResolverFactorynet/proxy_resolution/configured_proxy_resolution_service_unittest.cc |
modified | |
async_factory_net/proxy_resolution/configured_proxy_resolution_service_unittest.cc |
modified | |
ifnet/proxy_resolution/configured_proxy_resolution_service_unittest.cc |
modified |
Files Changed
net/proxy_resolution/configured_proxy_resolution_service_unittest.ccnet/proxy_resolution/configured_proxy_resolution_request.ccnet/proxy_resolution/configured_proxy_resolution_service.cc
Patch
From 2ed13d0826b0941eb5fdf6b469cb172b9b8dd2af Mon Sep 17 00:00:00 2001 From: Zonghan Xu <[email protected]> Date: Tue, 13 Oct 2026 03:58:26 -0700 Subject: [PATCH] [Net][CPRS] Improve lifetime management in CPRRequest When an in-flight proxy resolution request fails with ERR_PAC_SCRIPT_TERMINATED, ConfiguredProxyResolutionService resets its config and may synchronously re-initialize other pending requests. During this synchronous re-initialization, a sibling request's callback can cancel and destroy the completing request while its completion stack frames are still active. This change improves request lifetime management: - End the PROXY_RESOLUTION_SERVICE NetLog event before resetting the proxy config in DidFinishResolvingProxy(). - Clear `service_` and `results_` in ConfiguredProxyResolutionRequest before calling DidFinishResolvingProxy(). - Guard completion paths with base::WeakPtr to return early if the request is destroyed during re-entrant callbacks. Bug: 568906040 Change-Id: I270e5efe43ebaabc29c8ceead1bae85f3462082d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8560657 Reviewed-by: Kenichi Ishibashi <[email protected]> Commit-Queue: Zonghan Xu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1717484} --- diff --git a/net/proxy_resolution/configured_proxy_resolution_request.cc b/net/proxy_resolution/configured_proxy_resolution_request.cc index e42676f..460f0c4 100644 --- a/net/proxy_resolution/configured_proxy_resolution_request.cc +++ b/net/proxy_resolution/configured_proxy_resolution_request.cc @@ -169,26 +169,38 @@ Reset(); } - // Note that DidFinishResolvingProxy might modify `results_`. - int rv = service_->DidFinishResolvingProxy(url_, network_anonymization_key_, - method_, results_, result_code, - net_log_); + // Detach `service_` and `results_` before `DidFinishResolvingProxy()` in case + // a sibling callback synchronously destroys `this` or `results` (avoids + // double-removal in our destructor and dangling raw_ptrs). + ConfiguredProxyResolutionService* service = std::exchange(service_, nullptr); + ProxyInfo* results = std::exchange(results_, nullptr); + base::WeakPtr<ConfiguredProxyResolutionRequest> weak_this = + weak_factory_.GetWeakPtr(); + + // Note that DidFinishResolvingProxy might modify `results`. + int rv = + service->DidFinishResolvingProxy(url_, network_anonymization_key_, + method_, results, result_code, net_log_); + if (!weak_this) { + return ERR_ABORTED; + } // Make a note in the results which configuration was in use at the // time of the resolve. - results_->set_proxy_resolve_start_time(creation_time_); - results_->set_proxy_resolve_end_time(base::TimeTicks::Now()); + results->set_proxy_resolve_start_time(creation_time_); + results->set_proxy_resolve_end_time(base::TimeTicks::Now()); // If annotation is not already set, e.g. through TryToCompleteSynchronously // function, use in-progress-resolve annotation. - if (!results_->traffic_annotation().is_valid()) - results_->set_traffic_annotation(traffic_annotation_); + if (!results->traffic_annotation().is_valid()) { + results->set_traffic_annotation(traffic_annotation_); + } // If proxy is set without error, ensure that an annotation is provided. if (result_code != ERR_ABORTED && !rv) { // TODO(crbug.com/469006851): Turn this back into a CHECK once crashes were // fully fixed. - DCHECK(results_->traffic_annotation().is_valid()); + DCHECK(results->traffic_annotation().is_valid()); } // Reset the state associated with in-progress-resolve. @@ -199,9 +211,7 @@ int ConfiguredProxyResolutionRequest::QueryDidCompleteSynchronously( int result_code) { - int rv = QueryDidComplete(result_code); - service_ = nullptr; - return rv; + return QueryDidComplete(result_code); } LoadState ConfiguredProxyResolutionRequest::GetLoadState() const { @@ -222,8 +232,12 @@ // prevent re-entrant dispatch if the service re-initializes synchronously. service_->RemovePendingRequest(this); + base::WeakPtr<ConfiguredProxyResolutionRequest> weak_this = + weak_factory_.GetWeakPtr(); result_code = QueryDidComplete(result_code); - service_ = nullptr; + if (!weak_this) { + return; + } CompletionOnceCallback callback = std::move(user_callback_); user_callback_.Reset(); diff --git a/net/proxy_resolution/configured_proxy_resolution_service.cc b/net/proxy_resolution/configured_proxy_resolution_service.cc index 600274c9..71de633e 100644 --- a/net/proxy_resolution/configured_proxy_resolution_service.cc +++ b/net/proxy_resolution/configured_proxy_resolution_service.cc @@ -1293,6 +1293,7 @@ !pac_retry_throttler_.HasFailedDuringCurrentLoad()) { ResetPacRetryThrottler(); } + net_log.EndEvent(NetLogEventType::PROXY_RESOLUTION_SERVICE); } else { net_log.AddEventWithNetErrorCode( NetLogEventType::PROXY_RESOLUTION_SERVICE_RESOLVED_PROXY_LIST, @@ -1335,6 +1336,10 @@ } } + // End the NetLog event before resetting the config, as synchronous + // re-initialization can destroy the completing request (and `net_log`). + net_log.EndEvent(NetLogEventType::PROXY_RESOLUTION_SERVICE); + if (reset_config) { ResetProxyConfig(false); if (!pending_requests_.empty()) { @@ -1346,7 +1351,6 @@ } } - net_log.EndEvent(NetLogEventType::PROXY_RESOLUTION_SERVICE); return result_code; } diff --git a/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc b/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc index 7e9d649..46b80616 100644 --- a/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc +++ b/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc @@ -531,6 +531,105 @@ return proxy_list; } +// ProxyResolverFactory that delegates to MockAsyncProxyResolverFactory by +// default, or fails synchronously when `synchronous_fail_` is set. +class SyncFailProxyResolverFactory : public ProxyResolverFactory { + public: + explicit SyncFailProxyResolverFactory(bool resolvers_expect_pac_bytes) + : ProxyResolverFactory(resolvers_expect_pac_bytes), + async_factory_(resolvers_expect_pac_bytes) {} + + // Returns ERR_PAC_SCRIPT_FAILED synchronously if `synchronous_fail_` is set, + // or forwards to `async_factory_` otherwise. + int CreateProxyResolver( + const scoped_refptr<PacFileData>& pac_script, + std::unique_ptr<ProxyResolver>* resolver, + CompletionOnceCallback callback, + std::unique_ptr<ProxyResolverFactory::Request>* request_handle) override { + if (synchronous_fail_) { + return ERR_PAC_SCRIPT_FAILED; + } + return async_factory_.CreateProxyResolver( + pac_script, resolver, std::move(callback), request_handle); + } + + void set_synchronous_fail(bool synchronous_fail) { + synchronous_fail_ = synchronous_fail; + } + + const MockAsyncProxyResolverFactory::RequestsList& pending_requests() const { + return async_factory_.pending_requests(); + } + + private: + bool synchronous_fail_ = false; + MockAsyncProxyResolverFactory async_factory_; +}; + +// Bundles the caller-owned state for a single proxy resolution request in +// tests. +struct TestResolveRequest { + std::unique_ptr<ProxyInfo> info = std::make_unique<ProxyInfo>(); + std::unique_ptr<ProxyResolutionRequest> request; + NetLogWithSource net_log = NetLogWithSource::Make(NetLogSourceType::NONE); + CompletionOnceCallback callback; +}; + +// Starts two concurrent PAC requests and fails the first with +// ERR_PAC_SCRIPT_TERMINATED while forcing synchronous PAC re-initialization. +void SimulatePacTerminatedWithSyncReinit(TestResolveRequest& req1, + TestResolveRequest& req2) { + auto config_service = + std::make_unique<MockProxyConfigService>("http://foopy/proxy.pac"); + + MockAsyncProxyResolver resolver; + auto resolver_factory = std::make_unique<SyncFailProxyResolverFactory>(false); + auto* resolver_factory_ptr = resolver_factory.get(); + + ConfiguredProxyResolutionService service( + std::move(config_service), std::move(resolver_factory), + /*host_resolver_for_override_rules=*/nullptr, /*net_log=*/nullptr, + /*quick_check_enabled=*/true); + + GURL url1("http://www.google.com/"); + GURL url2("https://www.google.com/"); + + // Request 1: In-flight resolve request that will encounter + // ERR_PAC_SCRIPT_TERMINATED and trigger proxy re-initialization. + int rv1 = service.ResolveProxy(url1, std::string(), NetworkAnonymizationKey(), + handles::kInvalidNetworkHandle, + req1.info.get(), std::move(req1.callback), + &req1.request, req1.net_log, DEFAULT_PRIORITY); + + // Request 2: Concurrent in-flight resolve request that will be paused in + // `pending_requests_` and restarted upon re-initialization. + int rv2 = service.ResolveProxy(url2, std::string(), NetworkAnonymizationKey(), + handles::kInvalidNetworkHandle, + req2.info.get(), std::move(req2.callback), + &req2.request, req2.net_log, DEFAULT_PRIORITY); + + // Both requests should be waiting for PAC compilation now. + EXPECT_THAT(rv1, IsError(ERR_IO_PENDING)); + EXPECT_THAT(rv2, IsError(ERR_IO_PENDING)); + + // Complete PAC script compilation so the service becomes ready and + // dispatches `url1` and `url2` to the resolver. + ASSERT_EQ(1u, resolver_factory_ptr->pending_requests().size()); + EXPECT_EQ(GURL("http://foopy/proxy.pac"), + resolver_factory_ptr->pending_requests()[0]->script_data()->url()); + resolver_factory_ptr->pending_requests()[0]->CompleteNowWithForwarder( + OK, &resolver); + + // Both `url1` and `url2` are now running as in-flight jobs in the resolver. + JobMap jobs = GetPendingJobsForURLs(resolver, url1, url2); + + // Force synchronous re-initialization failure and crash `url1`. + resolver_factory_ptr->set_synchronous_fail(true); + jobs[url1]->CompleteNow(ERR_PAC_SCRIPT_TERMINATED); + EXPECT_FALSE(req1.request); + EXPECT_FALSE(req2.request); +} + } // namespace // When a PAC runtime error occurs (ERR_PAC_SCRIPT_FAILED), subsequent @@ -1930,135 +2029,71 @@ // Test what happens when the ProxyResolver fails with // ERR_PAC_SCRIPT_TERMINATED while multiple requests are in progress, and the // subsequent proxy re-initialization completes synchronously (e.g. fails and -// falls back to DIRECT). Make sure no use-after-free occurs. +// falls back to DIRECT). Make sure no use-after-free occurs when each request +// destroys itself in its completion callback (crbug.com/533534913). TEST_F(ConfiguredProxyResolutionServiceTest, - ProxyResolverTerminatedDuringRequestWithSyncReinit) { - class SyncFailProxyResolverFactory : public ProxyResolverFactory { - public: - explicit SyncFailProxyResolverFactory(bool resolvers_expect_pac_bytes) - : ProxyResolverFactory(resolvers_expect_pac_bytes), - async_factory_(resolvers_expect_pac_bytes) {} - - int CreateProxyResolver(const scoped_refptr<PacFileData>& pac_script, - std::unique_ptr<ProxyResolver>* resolver, - CompletionOnceCallback callback, - std::unique_ptr<ProxyResolverFactory::Request>* - request_handle) override { - if (synchronous_fail_) { - return ERR_PAC_SCRIPT_FAILED; - } - return async_factory_.CreateProxyResolver( - pac_script, resolver, std::move(callback), request_handle); - } - - void set_synchronous_fail(bool synchronous_fail) { - synchronous_fail_ = synchronous_fail; - } - - const MockAsyncProxyResolverFactory::RequestsList& pending_requests() - const { - return async_factory_.pending_requests(); - } - - private: - bool synchronous_fail_ = false; - MockAsyncProxyResolverFactory async_factory_; - }; - - auto config_service = - std::make_unique<MockProxyConfigService>("http://foopy/proxy.pac"); - - MockAsyncProxyResolver resolver; - auto resolver_factory = std::make_unique<SyncFailProxyResolverFactory>(false); - auto* resolver_factory_ptr = resolver_factory.get(); - - ConfiguredProxyResolutionService service(std::move(config_service), - std::move(resolver_factory), - mock_host_resolver_.get(), nullptr, - /*quick_check_enabled=*/true); - - // Start two resolve requests. - GURL url1("http://www.google.com/"); - GURL url2("https://www.google.com/"); - ProxyInfo info1, info2; - std::unique_ptr<ProxyResolutionRequest> request1, request2; + ProxyResolverTerminatedSyncReinitSelfDeletion) { + TestResolveRequest req1, req2; int callback1_result = ERR_IO_PENDING; - base::RunLoop run_loop1; - auto callback1 = base::BindLambdaForTesting([&](int rv) { + req1.callback = base::BindLambdaForTesting([&](int rv) { callback1_result = rv; // Destroy the request inside the completion callback, simulating the // behavior of consumers like HttpStreamFactory::JobController. - request1.reset(); - run_loop1.Quit(); + req1.request.reset(); }); int callback2_result = ERR_IO_PENDING; - base::RunLoop run_loop2; - auto callback2 = base::BindLambdaForTesting([&](int rv) { + req2.callback = base::BindLambdaForTesting([&](int rv) { callback2_result = rv; - request2.reset(); - run_loop2.Quit(); + req2.request.reset(); }); - // Request 1: In-flight resolve request that will encounter - // ERR_PAC_SCRIPT_TERMINATED and trigger proxy re-initialization. - int rv1 = service.ResolveProxy(url1, std::string(), NetworkAnonymizationKey(), - handles::kInvalidNetworkHandle, &info1, - std::move(callback1), &request1, - NetLogWithSource(), DEFAULT_PRIORITY); + SimulatePacTerminatedWithSyncReinit(req1, req2); - // Request 2: Concurrent in-flight resolve request that will be paused in - // `pending_requests_` and restarted upon re-initialization. - int rv2 = service.ResolveProxy(url2, std::string(), NetworkAnonymizationKey(), - handles::kInvalidNetworkHandle, &info2, - std::move(callback2), &request2, - NetLogWithSource(), DEFAULT_PRIORITY); - - // Both requests should be waiting for PAC compilation now. - EXPECT_THAT(rv1, IsError(ERR_IO_PENDING)); - EXPECT_THAT(rv2, IsError(ERR_IO_PENDING)); - - // The service must first create a ProxyResolver by fetching and compiling - // the PAC script. We should have one PAC factory creation request for - // "http://foopy/proxy.pac". - ASSERT_EQ(1u, resolver_factory_ptr->pending_requests().size()); - EXPECT_EQ(GURL("http://foopy/proxy.pac"), - resolver_factory_ptr->pending_requests()[0]->script_data()->url()); - - // Complete PAC script compilation now so the service becomes ready and - // dispatches `url1` and `url2` to the resolver. - resolver_factory_ptr->pending_requests()[0]->CompleteNowWithForwarder( - OK, &resolver); - - // Both `url1` and `url2` are now running as in-flight jobs in the resolver. - JobMap jobs = GetPendingJobsForURLs(resolver, url1, url2); - - // Configure the next CreateProxyResolver call (when the service attempts to - // reset and re-initialize the PAC resolver after `url1` crashes) to fail - // synchronously. - resolver_factory_ptr->set_synchronous_fail(true); - - // Simulate a PAC script crash on `url1`. DidFinishResolvingProxy() will then - // reset the configuration, attempt re-initialization (which fails - // synchronously), and fall back to DIRECT. - // - // Note: this covers the edge case reported in crbug.com/533534913 and - // confirms that destroying `request1` inside its completion callback does not - // cause a use-after-free. - jobs[url1]->CompleteNow(ERR_PAC_SCRIPT_TERMINATED); - - // `request1` fell back to DIRECT per-request as the error fallback for + // `req1` fell back to DIRECT per-request as the error fallback for // PAC runtime script crash (ERR_PAC_SCRIPT_TERMINATED). - run_loop1.Run(); EXPECT_THAT(callback1_result, IsOk()); - EXPECT_TRUE(info1.is_direct()); + EXPECT_TRUE(req1.info->is_direct()); // PAC re-initialization failed, so the entire service entered DIRECT fallback - // mode and restarted `request2` against this new DIRECT configuration. - run_loop2.Run(); + // mode and restarted `req2` against this new DIRECT configuration. EXPECT_THAT(callback2_result, IsOk()); - EXPECT_TRUE(info2.is_direct()); + EXPECT_TRUE(req2.info->is_direct()); +} + +// Verifies that cancelling the completing request from a sibling request's +// callback during synchronous PAC re-initialization is handled safely. +TEST_F(ConfiguredProxyResolutionServiceTest, + ProxyResolverTerminatedSyncReinitSiblingDeletion) { + TestResolveRequest req1, req2; + RecordingNetLogObserver net_log_observer; + ... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc b/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc
index 7e9d649..46b80616 100644
--- a/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc
+++ b/net/proxy_resolution/configured_proxy_resolution_service_unittest.cc
@@ -531,6 +531,105 @@
return proxy_list;
}
+// ProxyResolverFactory that delegates to MockAsyncProxyResolverFactory by
+// default, or fails synchronously when `synchronous_fail_` is set.
+class SyncFailProxyResolverFactory : public ProxyResolverFactory {
+ public:
+ explicit SyncFailProxyResolverFactory(bool resolvers_expect_pac_bytes)
+ : ProxyResolverFactory(resolvers_expect_pac_bytes),
+ async_factory_(resolvers_expect_pac_bytes) {}
+
+ // Returns ERR_PAC_SCRIPT_FAILED synchronously if `synchronous_fail_` is set,
+ // or forwards to `async_factory_` otherwise.
+ int CreateProxyResolver(
+ const scoped_refptr<PacFileData>& pac_script,
+ std::unique_ptr<ProxyResolver>* resolver,
+ CompletionOnceCallback callback,
+ std::unique_ptr<ProxyResolverFactory::Request>* request_handle) override {
+ if (synchronous_fail_) {
+ return ERR_PAC_SCRIPT_FAILED;
+ }
+ return async_factory_.CreateProxyResolver(
+ pac_script, resolver, std::move(callback), request_handle);
+ }
+
+ void set_synchronous_fail(bool synchronous_fail) {
+ synchronous_fail_ = synchronous_fail;
+ }
+
+ const MockAsyncProxyResolverFactory::RequestsList& pending_requests() const {
+ return async_factory_.pending_requests();
+ }
+
+ private:
+ bool synchronous_fail_ = false;
+ MockAsyncProxyResolverFactory async_factory_;
+};
+
+// Bundles the caller-owned state for a single proxy resolution request in
+// tests.
+struct TestResolveRequest {
+ std::unique_ptr<ProxyInfo> info = std::make_unique<ProxyInfo>();
+ std::unique_ptr<ProxyResolutionRequest> request;
+ NetLogWithSource net_log = NetLogWithSource::Make(NetLogSourceType::NONE);
+ CompletionOnceCallback callback;
+};
+
+// Starts two concurrent PAC requests and fails the first with
+// ERR_PAC_SCRIPT_TERMINATED while forcing synchronous PAC re-initialization.
+void SimulatePacTerminatedWithSyncReinit(TestResolveRequest& req1,
+ TestResolveRequest& req2) {
+ auto config_service =
+ std::make_unique<MockProxyConfigService>("http://foopy/proxy.pac");
+
+ MockAsyncProxyResolver resolver;
+ auto resolver_factory = std::make_unique<SyncFailProxyResolverFactory>(false);
+ auto* resolver_factory_ptr = resolver_factory.get();
+
+ ConfiguredProxyResolutionService service(
+ std::move(config_service), std::move(resolver_factory),
+ /*host_resolver_for_override_rules=*/nullptr, /*net_log=*/nullptr,
+ /*quick_check_enabled=*/true);
+
+ GURL url1("http://www.google.com/");
+ GURL url2("https://www.google.com/");
+
+ // Request 1: In-flight resolve request that will encounter
+ // ERR_PAC_SCRIPT_TERMINATED and trigger proxy re-initialization.
+ int rv1 = service.ResolveProxy(url1, std::string(), NetworkAnonymizationKey(),
+ handles::kInvalidNetworkHandle,
+ req1.info.get(), std::move(req1.callback),
+ &req1.request, req1.net_log, DEFAULT_PRIORITY);
+
+ // Request 2: Concurrent in-flight resolve request that will be paused in
+ // `pending_requests_` and restarted upon re-initialization.
+ int rv2 = service.ResolveProxy(url2, std::string(), NetworkAnonymizationKey(),
+ handles::kInvalidNetworkHandle,
+ req2.info.get(), std::move(req2.callback),
+ &req2.request, req2.net_log, DEFAULT_PRIORITY);
+
+ // Both requests should be waiting for PAC compilation now.
+ EXPECT_THAT(rv1, IsError(ERR_IO_PENDING));
+ EXPECT_THAT(rv2, IsError(ERR_IO_PENDING));
+
+ // Complete PAC script compilation so the service becomes ready and
+ // dispatches `url1` and `url2` to the resolver.
+ ASSERT_EQ(1u, resolver_factory_ptr->pending_requests().size());
+ EXPECT_EQ(GURL("http://foopy/proxy.pac"),
+ resolver_factory_ptr->pending_requests()[0]->script_data()->url());
+ resolver_factory_ptr->pending_requests()[0]->CompleteNowWithForwarder(
+ OK, &resolver);
+
+ // Both `url1` and `url2` are now running as in-flight jobs in the resolver.
+ JobMap jobs = GetPendingJobsForURLs(resolver, url1, url2);
+
+ // Force synchronous re-initialization failure and crash `url1`.
+ resolver_factory_ptr->set_synchronous_fail(true);
+ jobs[url1]->CompleteNow(ERR_PAC_SCRIPT_TERMINATED);
+ EXPECT_FALSE(req1.request);
+ EXPECT_FALSE(req2.request);
+}
+
} // namespace
// When a PAC runtime error occurs (ERR_PAC_SCRIPT_FAILED), subsequent
@@ -1930,135 +2029,71 @@
// Test what happens when the ProxyResolver fails with
// ERR_PAC_SCRIPT_TERMINATED while multiple requests are in progress, and the
// subsequent proxy re-initialization completes synchronously (e.g. fails and
-// falls back to DIRECT). Make sure no use-after-free occurs.
+// falls back to DIRECT). Make sure no use-after-free occurs when each request
+// destroys itself in its completion callback (crbug.com/533534913).
TEST_F(ConfiguredProxyResolutionServiceTest,
- ProxyResolverTerminatedDuringRequestWithSyncReinit) {
- class SyncFailProxyResolverFactory : public ProxyResolverFactory {
- public:
- explicit SyncFailProxyResolverFactory(bool resolvers_expect_pac_bytes)
- : ProxyResolverFactory(resolvers_expect_pac_bytes),
- async_factory_(resolvers_expect_pac_bytes) {}
-
- int CreateProxyResolver(const scoped_refptr<PacFileData>& pac_script,
- std::unique_ptr<ProxyResolver>* resolver,
- CompletionOnceCallback callback,
- std::unique_ptr<ProxyResolverFactory::Request>*
- request_handle) override {
- if (synchronous_fail_) {
- return ERR_PAC_SCRIPT_FAILED;
- }
- return async_factory_.CreateProxyResolver(
- pac_script, resolver, std::move(callback), request_handle);
- }
-
- void set_synchronous_fail(bool synchronous_fail) {
- synchronous_fail_ = synchronous_fail;
- }
-
- const MockAsyncProxyResolverFactory::RequestsList& pending_requests()
- const {
- return async_factory_.pending_requests();
- }
-
- private:
- bool synchronous_fail_ = false;
- MockAsyncProxyResolverFactory async_factory_;
- };
-
- auto config_service =
- std::make_unique<MockProxyConfigService>("http://foopy/proxy.pac");
-
- MockAsyncProxyResolver resolver;
- auto resolver_factory = std::make_unique<SyncFailProxyResolverFactory>(false);
- auto* resolver_factory_ptr = resolver_factory.get();
-
- ConfiguredProxyResolutionService service(std::move(config_service),
- std::move(resolver_factory),
- mock_host_resolver_.get(), nullptr,
- /*quick_check_enabled=*/true);
-
- // Start two resolve requests.
- GURL url1("http://www.google.com/");
- GURL url2("https://www.google.com/");
- ProxyInfo info1, info2;
- std::unique_ptr<ProxyResolutionRequest> request1, request2;
+ ProxyResolverTerminatedSyncReinitSelfDeletion) {
+ TestResolveRequest req1, req2;
int callback1_result = ERR_IO_PENDING;
- base::RunLoop run_loop1;
- auto callback1 = base::BindLambdaForTesting([&](int rv) {
+ req1.callback = base::BindLambdaForTesting([&](int rv) {
callback1_result = rv;
// Destroy the request inside the completion callback, simulating the
// behavior of consumers like HttpStreamFactory::JobController.
- request1.reset();
- run_loop1.Quit();
+ req1.request.reset();
});
int callback2_result = ERR_IO_PENDING;
- base::RunLoop run_loop2;
- auto callback2 = base::BindLambdaForTesting([&](int rv) {
+ req2.callback = base::BindLambdaForTesting([&](int rv) {
callback2_result = rv;
- request2.reset();
- run_loop2.Quit();
+ req2.request.reset();
});
- // Request 1: In-flight resolve request that will encounter
- // ERR_PAC_SCRIPT_TERMINATED and trigger proxy re-initialization.
- int rv1 = service.ResolveProxy(url1, std::string(), NetworkAnonymizationKey(),
- handles::kInvalidNetworkHandle, &info1,
- std::move(callback1), &request1,
- NetLogWithSource(), DEFAULT_PRIORITY);
+ SimulatePacTerminatedWithSyncReinit(req1, req2);
- // Request 2: Concurrent in-flight resolve request that will be paused in
- // `pending_requests_` and restarted upon re-initialization.
- int rv2 = service.ResolveProxy(url2, std::string(), NetworkAnonymizationKey(),
- handles::kInvalidNetworkHandle, &info2,
- std::move(callback2), &request2,
- NetLogWithSource(), DEFAULT_PRIORITY);
-
- // Both requests should be waiting for PAC compilation now.
- EXPECT_THAT(rv1, IsError(ERR_IO_PENDING));
- EXPECT_THAT(rv2, IsError(ERR_IO_PENDING));
-
- // The service must first create a ProxyResolver by fetching and compiling
- // the PAC script. We should have one PAC factory creation request for
- // "http://foopy/proxy.pac".
- ASSERT_EQ(1u, resolver_factory_ptr->pending_requests().size());
- EXPECT_EQ(GURL("http://foopy/proxy.pac"),
- resolver_factory_ptr->pending_requests()[0]->script_data()->url());
-
- // Complete PAC script compilation now so the service becomes ready and
- // dispatches `url1` and `url2` to the resolver.
- resolver_factory_ptr->pending_requests()[0]->CompleteNowWithForwarder(
- OK, &resolver);
-
- // Both `url1` and `url2` are now running as in-flight jobs in the resolver.
- JobMap jobs = GetPendingJobsForURLs(resolver, url1, url2);
-
- // Configure the next CreateProxyResolver call (when the service attempts to
- // reset and re-initialize the PAC resolver after `url1` crashes) to fail
- // synchronously.
- resolver_factory_ptr->set_synchronous_fail(true);
-
- // Simulate a PAC script crash on `url1`. DidFinishResolvingProxy() will then
- // reset the configuration, attempt re-initialization (which fails
- // synchronously), and fall back to DIRECT.
- //
- // Note: this covers the edge case reported in crbug.com/533534913 and
- // confirms that destroying `request1` inside its completion callback does not
- // cause a use-after-free.
- jobs[url1]->CompleteNow(ERR_PAC_SCRIPT_TERMINATED);
-
- // `request1` fell back to DIRECT per-request as the error fallback for
+ // `req1` fell back to DIRECT per-request as the error fallback for
// PAC runtime script crash (ERR_PAC_SCRIPT_TERMINATED).
- run_loop1.Run();
EXPECT_THAT(callback1_result, IsOk());
- EXPECT_TRUE(info1.is_direct());
+ EXPECT_TRUE(req1.info->is_direct());
// PAC re-initialization failed, so the entire service entered DIRECT fallback
- // mode and restarted `request2` against this new DIRECT configuration.
- run_loop2.Run();
+ // mode and restarted `req2` against this new DIRECT configuration.
EXPECT_THAT(callback2_result, IsOk());
- EXPECT_TRUE(info2.is_direct());
+ EXPECT_TRUE(req2.info->is_direct());
+}
+
+// Verifies that cancelling the completing request from a sibling request's
+// callback during synchronous PAC re-initialization is handled safely.
+TEST_F(ConfiguredProxyResolutionServiceTest,
+ ProxyResolverTerminatedSyncReinitSiblingDeletion) {
+ TestResolveRequest req1, req2;
+ RecordingNetLogObserver net_log_observer;
+
+ bool callback1_called = false;
+ req1.callback =
+ base::BindLambdaForTesting([&](int rv) { callback1_called = true; });
+
+ int callback2_result = ERR_IO_PENDING;
+ req2.callback = base::BindLambdaForTesting([&](int rv) {
+ callback2_result = rv;
+ // Cancel `req1` while its completion frames are still on the stack.
+ req1.info.reset();
+ req1.request.reset();
+ req2.request.reset();
+ });
+
+ SimulatePacTerminatedWithSyncReinit(req1, req2);
+
+ EXPECT_THAT(callback2_result, IsOk());
+ EXPECT_TRUE(req2.info->is_direct());
+ EXPECT_FALSE(callback1_called);
+
+ auto entries1 = net_log_observer.GetEntriesForSource(req1.net_log.source());
+ ASSERT_EQ(5u, entries1.size());
+ EXPECT_TRUE(LogContainsBeginEvent(entries1, 0,
+ NetLogEventType::PROXY_RESOLUTION_SERVICE));
+ EXPECT_TRUE(LogContainsEndEvent(entries1, 4,
+ NetLogEventType::PROXY_RESOLUTION_SERVICE));
}
TEST_F(ConfiguredProxyResolutionServiceTest,
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.
References
On This Page