diff options
| -rw-r--r-- | net/data/proxy_resolver_v8_tracing_unittest/terminate.js | 20 | ||||
| -rw-r--r-- | net/data/proxy_resolver_v8_unittest/terminate.js | 26 | ||||
| -rw-r--r-- | net/proxy/proxy_resolver_perftest.cc | 3 | ||||
| -rw-r--r-- | net/proxy/proxy_resolver_v8.cc | 111 | ||||
| -rw-r--r-- | net/proxy/proxy_resolver_v8.h | 7 | ||||
| -rw-r--r-- | net/proxy/proxy_resolver_v8_tracing.cc | 20 | ||||
| -rw-r--r-- | net/proxy/proxy_resolver_v8_tracing_unittest.cc | 146 | ||||
| -rw-r--r-- | net/proxy/proxy_resolver_v8_unittest.cc | 56 |
8 files changed, 311 insertions, 78 deletions
diff --git a/net/data/proxy_resolver_v8_tracing_unittest/terminate.js b/net/data/proxy_resolver_v8_tracing_unittest/terminate.js new file mode 100644 index 0000000..88fe4cb --- /dev/null +++ b/net/data/proxy_resolver_v8_tracing_unittest/terminate.js @@ -0,0 +1,20 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +var g_iteration = 0; + +function FindProxyForURL(url, host) { + g_iteration++; + + var ip1 = dnsResolve("host1"); + var ip2 = dnsResolveEx("host2"); + + if (ip1 == "182.111.0.222" && ip2 == "111.33.44.55") + return "PROXY foopy:" + g_iteration; + + // If the script didn't terminate when abandoned, then it will reach this and + // hang. + for (;;) {} + throw "not reached"; +} diff --git a/net/data/proxy_resolver_v8_unittest/terminate.js b/net/data/proxy_resolver_v8_unittest/terminate.js new file mode 100644 index 0000000..08d9289 --- /dev/null +++ b/net/data/proxy_resolver_v8_unittest/terminate.js @@ -0,0 +1,26 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +function FindProxyForURL(url, host) { + if (host != 'hang') + return 'PROXY ' + host + ':88'; + + var ip = dnsResolve("host1"); + + // The following may or may not be executed, even if dnsResolve() terminates + // the script execution. + dnsResolveEx("host2"); + dnsResolveEx("host3"); + alert("hahaha"); + + // Hang! + for (;;) {} + + // The following definitely won't be executed, since control should never + // make it past the preceding hang. + dnsResolve("host4"); + dnsResolve("host5"); + alert("uhm..."); + throw "not reached"; +} diff --git a/net/proxy/proxy_resolver_perftest.cc b/net/proxy/proxy_resolver_perftest.cc index 561efb51..762e053 100644 --- a/net/proxy/proxy_resolver_perftest.cc +++ b/net/proxy/proxy_resolver_perftest.cc @@ -205,7 +205,8 @@ class MockJSBindings : public net::ProxyResolverV8::JSBindings { virtual bool ResolveDns(const std::string& host, ResolveDnsOperation op, - std::string* output) OVERRIDE { + std::string* output, + bool* terminate) OVERRIDE { CHECK(false); return false; } diff --git a/net/proxy/proxy_resolver_v8.cc b/net/proxy/proxy_resolver_v8.cc index 4a33910..c28dead 100644 --- a/net/proxy/proxy_resolver_v8.cc +++ b/net/proxy/proxy_resolver_v8.cc @@ -493,13 +493,14 @@ class ProxyResolverV8::Context { // Handle an exception thrown by V8. void HandleError(v8::Handle<v8::Message> message) { - if (message.IsEmpty()) - return; - - // Otherwise dispatch to the bindings. - int line_number = message->GetLineNumber(); string16 error_message; - V8ObjectToUTF16String(message->Get(), &error_message); + int line_number = -1; + + if (!message.IsEmpty()) { + line_number = message->GetLineNumber(); + V8ObjectToUTF16String(message->Get(), &error_message); + } + js_bindings()->OnError(line_number, error_message); } @@ -547,92 +548,70 @@ class ProxyResolverV8::Context { // V8 callback for when "myIpAddress()" is invoked by the PAC script. static v8::Handle<v8::Value> MyIpAddressCallback(const v8::Arguments& args) { - Context* context = - static_cast<Context*>(v8::External::Cast(*args.Data())->Value()); - - std::string result; - bool success; - - { - v8::Unlocker unlocker(args.GetIsolate()); - // We shouldn't be called with any arguments, but will not complain if - // we are. - success = context->js_bindings()->ResolveDns( - "", JSBindings::MY_IP_ADDRESS, &result); - } - - if (!success) - return ASCIILiteralToV8String("127.0.0.1"); - return ASCIIStringToV8String(result); + return DnsResolveCallbackHelper(args, JSBindings::MY_IP_ADDRESS); } // V8 callback for when "myIpAddressEx()" is invoked by the PAC script. static v8::Handle<v8::Value> MyIpAddressExCallback( const v8::Arguments& args) { - Context* context = - static_cast<Context*>(v8::External::Cast(*args.Data())->Value()); - - std::string ip_address_list; - bool success; - - { - v8::Unlocker unlocker(args.GetIsolate()); - // We shouldn't be called with any arguments, but will not complain if - // we are. - success = context->js_bindings()->ResolveDns( - "", JSBindings::MY_IP_ADDRESS_EX, &ip_address_list); - } - - if (!success) - ip_address_list = std::string(); - return ASCIIStringToV8String(ip_address_list); + return DnsResolveCallbackHelper(args, JSBindings::MY_IP_ADDRESS_EX); } // V8 callback for when "dnsResolve()" is invoked by the PAC script. static v8::Handle<v8::Value> DnsResolveCallback(const v8::Arguments& args) { - Context* context = - static_cast<Context*>(v8::External::Cast(*args.Data())->Value()); - - // We need at least one string argument. - std::string hostname; - if (!GetHostnameArgument(args, &hostname)) - return v8::Null(); - - std::string ip_address; - bool success; - - { - v8::Unlocker unlocker(args.GetIsolate()); - success = context->js_bindings()->ResolveDns( - hostname, JSBindings::DNS_RESOLVE, &ip_address); - } - - return success ? ASCIIStringToV8String(ip_address) : v8::Null(); + return DnsResolveCallbackHelper(args, JSBindings::DNS_RESOLVE); } // V8 callback for when "dnsResolveEx()" is invoked by the PAC script. static v8::Handle<v8::Value> DnsResolveExCallback(const v8::Arguments& args) { + return DnsResolveCallbackHelper(args, JSBindings::DNS_RESOLVE_EX); + } + + // Shared code for implementing: + // - myIpAddress(), myIpAddressEx(), dnsResolve(), dnsResolveEx(). + static v8::Handle<v8::Value> DnsResolveCallbackHelper( + const v8::Arguments& args, JSBindings::ResolveDnsOperation op) { Context* context = static_cast<Context*>(v8::External::Cast(*args.Data())->Value()); - // We need at least one string argument. std::string hostname; - if (!GetHostnameArgument(args, &hostname)) - return v8::Undefined(); - std::string ip_address_list; + // dnsResolve() and dnsResolveEx() need at least 1 argument. + if (op == JSBindings::DNS_RESOLVE || op == JSBindings::DNS_RESOLVE_EX) { + if (!GetHostnameArgument(args, &hostname)) + return (op == JSBindings::DNS_RESOLVE) ? v8::Null() : v8::Undefined(); + } + + std::string result; bool success; + bool terminate = false; { v8::Unlocker unlocker(args.GetIsolate()); success = context->js_bindings()->ResolveDns( - hostname, JSBindings::DNS_RESOLVE_EX, &ip_address_list); + hostname, op, &result, &terminate); } - if (!success) - ip_address_list = std::string(); + if (terminate) + v8::V8::TerminateExecution(args.GetIsolate()); + + if (success) + return ASCIIStringToV8String(result); + + // Each function handles resolution errors differently. + switch (op) { + case JSBindings::DNS_RESOLVE: + return v8::Null(); + case JSBindings::DNS_RESOLVE_EX: + return v8::String::Empty(); + case JSBindings::MY_IP_ADDRESS: + return ASCIILiteralToV8String("127.0.0.1"); + case JSBindings::MY_IP_ADDRESS_EX: + return v8::String::Empty(); + } - return ASCIIStringToV8String(ip_address_list); + NOTREACHED(); + return v8::Undefined(); } // V8 callback for when "sortIpAddressList()" is invoked by the PAC script. diff --git a/net/proxy/proxy_resolver_v8.h b/net/proxy/proxy_resolver_v8.h index 326b5a4..3adc672 100644 --- a/net/proxy/proxy_resolver_v8.h +++ b/net/proxy/proxy_resolver_v8.h @@ -44,17 +44,18 @@ class NET_EXPORT_PRIVATE ProxyResolverV8 : public ProxyResolver { DNS_RESOLVE_EX, MY_IP_ADDRESS, MY_IP_ADDRESS_EX, - NUM_DNS_OPERATIONS, }; JSBindings() {} // Handler for "dnsResolve()", "dnsResolveEx()", "myIpAddress()", // "myIpAddressEx()". Returns true on success and fills |*output| with the - // result. + // result. If |*terminate| is set to true, then the script execution will + // be aborted. Note that termination may not happen right away. virtual bool ResolveDns(const std::string& host, ResolveDnsOperation op, - std::string* output) = 0; + std::string* output, + bool* terminate) = 0; // Handler for "alert(message)" virtual void Alert(const string16& message) = 0; diff --git a/net/proxy/proxy_resolver_v8_tracing.cc b/net/proxy/proxy_resolver_v8_tracing.cc index 0d5160c..c15c3c2 100644 --- a/net/proxy/proxy_resolver_v8_tracing.cc +++ b/net/proxy/proxy_resolver_v8_tracing.cc @@ -155,7 +155,8 @@ class ProxyResolverV8Tracing::Job // Implementation of ProxyResolverv8::JSBindings virtual bool ResolveDns(const std::string& host, ResolveDnsOperation op, - std::string* output) OVERRIDE; + std::string* output, + bool* terminate) OVERRIDE; virtual void Alert(const string16& message) OVERRIDE; virtual void OnError(int line_number, const string16& error) OVERRIDE; @@ -165,7 +166,8 @@ class ProxyResolverV8Tracing::Job bool ResolveDnsNonBlocking(const std::string& host, ResolveDnsOperation op, - std::string* output); + std::string* output, + bool* terminate); bool PostDnsOperationAndWait(const std::string& host, ResolveDnsOperation op, @@ -658,9 +660,12 @@ int ProxyResolverV8Tracing::Job::ExecuteProxyResolver() { bool ProxyResolverV8Tracing::Job::ResolveDns(const std::string& host, ResolveDnsOperation op, - std::string* output) { - if (cancelled_.IsSet()) + std::string* output, + bool* terminate) { + if (cancelled_.IsSet()) { + *terminate = true; return false; + } if ((op == DNS_RESOLVE || op == DNS_RESOLVE_EX) && host.empty()) { // a DNS resolve with an empty hostname is considered an error. @@ -669,7 +674,7 @@ bool ProxyResolverV8Tracing::Job::ResolveDns(const std::string& host, return blocking_dns_ ? ResolveDnsBlocking(host, op, output) : - ResolveDnsNonBlocking(host, op, output); + ResolveDnsNonBlocking(host, op, output, terminate); } void ProxyResolverV8Tracing::Job::Alert(const string16& message) { @@ -712,7 +717,8 @@ bool ProxyResolverV8Tracing::Job::ResolveDnsBlocking(const std::string& host, bool ProxyResolverV8Tracing::Job::ResolveDnsNonBlocking(const std::string& host, ResolveDnsOperation op, - std::string* output) { + std::string* output, + bool* terminate) { CheckIsOnWorkerThread(); if (abandoned_) { @@ -736,6 +742,7 @@ bool ProxyResolverV8Tracing::Job::ResolveDnsNonBlocking(const std::string& host, if (num_dns_ <= last_num_dns_) { // The sequence of DNS operations is different from last time! ScheduleRestartWithBlockingDns(); + *terminate = true; return false; } @@ -759,6 +766,7 @@ bool ProxyResolverV8Tracing::Job::ResolveDnsNonBlocking(const std::string& host, // been started. Abandon this invocation of FindProxyForURL(), it will be // restarted once the DNS request completes. abandoned_ = true; + *terminate = true; last_num_dns_ = num_dns_; return false; } diff --git a/net/proxy/proxy_resolver_v8_tracing_unittest.cc b/net/proxy/proxy_resolver_v8_tracing_unittest.cc index 9342d05..1c6aa13 100644 --- a/net/proxy/proxy_resolver_v8_tracing_unittest.cc +++ b/net/proxy/proxy_resolver_v8_tracing_unittest.cc @@ -945,6 +945,152 @@ TEST_F(ProxyResolverV8TracingTest, CancelSetPacWhileOutstandingBlockingDns) { EXPECT_EQ(1, host_resolver.num_cancelled_requests()); } +// This tests that the execution of a PAC script is terminated when the DNS +// dependencies are missing. If the test fails, then it will hang. +TEST_F(ProxyResolverV8TracingTest, Terminate) { + CapturingNetLog log; + CapturingBoundNetLog request_log; + MockCachingHostResolver host_resolver; + MockErrorObserver* error_observer = new MockErrorObserver; + ProxyResolverV8Tracing resolver(&host_resolver, error_observer, &log); + + host_resolver.rules()->AddRule("host1", "182.111.0.222"); + host_resolver.rules()->AddRule("host2", "111.33.44.55"); + + InitResolver(&resolver, "terminate.js"); + + TestCompletionCallback callback; + ProxyInfo proxy_info; + + int rv = resolver.GetProxyForURL( + GURL("http://foopy/req1"), + &proxy_info, + callback.callback(), + NULL, + request_log.bound()); + + EXPECT_EQ(ERR_IO_PENDING, rv); + EXPECT_EQ(OK, callback.WaitForResult()); + + // The test does 2 DNS resolutions. + EXPECT_EQ(2u, host_resolver.num_resolve()); + + EXPECT_EQ("foopy:3", proxy_info.proxy_server().ToURI()); + + // No errors. + EXPECT_EQ("", error_observer->GetOutput()); + + EXPECT_EQ(0u, log.GetSize()); + EXPECT_EQ(0u, request_log.GetSize()); +} + +// Tests that multiple instances of ProxyResolverV8Tracing can coexist and run +// correctly at the same time. This is relevant because at the moment (time +// this test was written) each ProxyResolverV8Tracing creates its own thread to +// run V8 on, however each thread is operating on the same v8::Isolate. +TEST_F(ProxyResolverV8TracingTest, MultipleResolvers) { + // ------------------------ + // Setup resolver0 + // ------------------------ + MockHostResolver host_resolver0; + host_resolver0.rules()->AddRuleForAddressFamily( + "host1", ADDRESS_FAMILY_IPV4, "166.155.144.44"); + host_resolver0.rules()->AddIPLiteralRule("host1", "::1,192.168.1.1", ""); + host_resolver0.rules()->AddSimulatedFailure("host2"); + host_resolver0.rules()->AddRule("host3", "166.155.144.33"); + host_resolver0.rules()->AddRule("host5", "166.155.144.55"); + host_resolver0.rules()->AddSimulatedFailure("host6"); + host_resolver0.rules()->AddRuleForAddressFamily( + "*", ADDRESS_FAMILY_IPV4, "122.133.144.155"); + host_resolver0.rules()->AddRule("*", "133.122.100.200"); + ProxyResolverV8Tracing resolver0( + &host_resolver0, new MockErrorObserver, NULL); + InitResolver(&resolver0, "dns.js"); + + // ------------------------ + // Setup resolver1 + // ------------------------ + ProxyResolverV8Tracing resolver1( + &host_resolver0, new MockErrorObserver, NULL); + InitResolver(&resolver1, "dns.js"); + + // ------------------------ + // Setup resolver2 + // ------------------------ + ProxyResolverV8Tracing resolver2( + &host_resolver0, new MockErrorObserver, NULL); + InitResolver(&resolver2, "simple.js"); + + // ------------------------ + // Setup resolver3 + // ------------------------ + MockHostResolver host_resolver3; + host_resolver3.rules()->AddRule("foo", "166.155.144.33"); + ProxyResolverV8Tracing resolver3( + &host_resolver3, new MockErrorObserver, NULL); + InitResolver(&resolver3, "simple_dns.js"); + + // ------------------------ + // Queue up work for each resolver (which will be running in parallel). + // ------------------------ + + ProxyResolverV8Tracing* resolver[] = { + &resolver0, &resolver1, &resolver2, &resolver3, + }; + + const size_t kNumResolvers = arraysize(resolver); + const size_t kNumIterations = 20; + const size_t kNumResults = kNumResolvers * kNumIterations; + TestCompletionCallback callback[kNumResults]; + ProxyInfo proxy_info[kNumResults]; + + for (size_t i = 0; i < kNumResults; ++i) { + size_t resolver_i = i % kNumResolvers; + int rv = resolver[resolver_i]->GetProxyForURL( + GURL("http://foo/"), &proxy_info[i], callback[i].callback(), NULL, + BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + } + + // ------------------------ + // Verify all of the results. + // ------------------------ + + const char* kExpectedForDnsJs = + "122.133.144.155-" // myIpAddress() + "null-" // dnsResolve('') + "__1_192.168.1.1-" // dnsResolveEx('host1') + "null-" // dnsResolve('host2') + "166.155.144.33-" // dnsResolve('host3') + "122.133.144.155-" // myIpAddress() + "166.155.144.33-" // dnsResolve('host3') + "__1_192.168.1.1-" // dnsResolveEx('host1') + "122.133.144.155-" // myIpAddress() + "null-" // dnsResolve('host2') + "-" // dnsResolveEx('host6') + "133.122.100.200-" // myIpAddressEx() + "166.155.144.44" // dnsResolve('host1') + ":99"; + + for (size_t i = 0; i < kNumResults; ++i) { + size_t resolver_i = i % kNumResolvers; + EXPECT_EQ(OK, callback[i].WaitForResult()); + + std::string proxy_uri = proxy_info[i].proxy_server().ToURI(); + + if (resolver_i == 0 || resolver_i == 1) { + EXPECT_EQ(kExpectedForDnsJs, proxy_uri); + } else if (resolver_i == 2) { + EXPECT_EQ("foo:99", proxy_uri); + } else if (resolver_i == 3) { + EXPECT_EQ("166.155.144.33:", + proxy_uri.substr(0, proxy_uri.find(':') + 1)); + } else { + NOTREACHED(); + } + } +} + } // namespace } // namespace net diff --git a/net/proxy/proxy_resolver_v8_unittest.cc b/net/proxy/proxy_resolver_v8_unittest.cc index eb8876c..250aaf3 100644 --- a/net/proxy/proxy_resolver_v8_unittest.cc +++ b/net/proxy/proxy_resolver_v8_unittest.cc @@ -23,7 +23,8 @@ namespace { // list, for later verification. class MockJSBindings : public ProxyResolverV8::JSBindings { public: - MockJSBindings() : my_ip_address_count(0), my_ip_address_ex_count(0) {} + MockJSBindings() : my_ip_address_count(0), my_ip_address_ex_count(0), + should_terminate(false) {} virtual void Alert(const string16& message) OVERRIDE { VLOG(1) << "PAC-alert: " << message; // Helpful when debugging. @@ -32,7 +33,10 @@ class MockJSBindings : public ProxyResolverV8::JSBindings { virtual bool ResolveDns(const std::string& host, ResolveDnsOperation op, - std::string* output) OVERRIDE { + std::string* output, + bool* terminate) OVERRIDE { + *terminate = should_terminate; + if (op == MY_IP_ADDRESS) { my_ip_address_count++; *output = my_ip_address_result; @@ -83,6 +87,9 @@ class MockJSBindings : public ProxyResolverV8::JSBindings { std::vector<std::string> dns_resolves_ex; int my_ip_address_count; int my_ip_address_ex_count; + + // Whether ResolveDns() should terminate script execution. + bool should_terminate; }; // This is the same as ProxyResolverV8, but it uses mock bindings in place of @@ -573,5 +580,50 @@ TEST(ProxyResolverV8Test, IPv6HostnamesNotBracketed) { EXPECT_EQ("abcd::efff", resolver.mock_js_bindings()->dns_resolves_ex[0]); } +// Test that terminating a script within DnsResolve() leads to eventual +// termination of the script. Also test that repeatedly calling terminate is +// safe, and running the script again after termination still works. +TEST(ProxyResolverV8Test, Terminate) { + ProxyResolverV8WithMockBindings resolver; + int result = resolver.SetPacScriptFromDisk("terminate.js"); + EXPECT_EQ(OK, result); + + MockJSBindings* bindings = resolver.mock_js_bindings(); + + // Terminate script execution upon reaching dnsResolve(). Note that + // termination may not take effect right away (so the subsequent dnsResolve() + // and alert() may be run). + bindings->should_terminate = true; + + ProxyInfo proxy_info; + result = resolver.GetProxyForURL( + GURL("http://hang/"), &proxy_info, + CompletionCallback(), NULL, BoundNetLog()); + + // The script execution was terminated. + EXPECT_EQ(ERR_PAC_SCRIPT_FAILED, result); + + EXPECT_EQ(1U, resolver.mock_js_bindings()->dns_resolves.size()); + EXPECT_GE(2U, resolver.mock_js_bindings()->dns_resolves_ex.size()); + EXPECT_GE(1U, bindings->alerts.size()); + + EXPECT_EQ(1U, bindings->errors.size()); + + // Termination shows up as an uncaught exception without any message. + EXPECT_EQ("", bindings->errors[0]); + + bindings->errors.clear(); + + // Try running the script again, this time with a different input which won't + // cause a termination+hang. + result = resolver.GetProxyForURL( + GURL("http://kittens/"), &proxy_info, + CompletionCallback(), NULL, BoundNetLog()); + + EXPECT_EQ(OK, result); + EXPECT_EQ(0u, bindings->errors.size()); + EXPECT_EQ("kittens:88", proxy_info.proxy_server().ToURI()); +} + } // namespace } // namespace net |