Enable the disabled test ProxyResolverV8Test.ReturnUnicode, by addressing some encoding issues.

Switched some uses of UTF-8 encoded std::strings to UTF-16 encoded string16 -- this avoids the confusion on which strings are unicode, since these std::string were getting passed around and later assumed to be ASCII.

BUG=46608, 47234

Review URL: http://codereview.chromium.org/2827018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50549 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 202 insertions, 111 deletions

diff --git a/net/proxy/proxy_resolver_js_bindings.cc b/net/proxy/proxy_resolver_js_bindings.cc
index 347b953..6704f18 100644
--- a/net/proxy/proxy_resolver_js_bindings.cc
+++ b/net/proxy/proxy_resolver_js_bindings.cc
@@ -5,6 +5,7 @@
 #include "net/proxy/proxy_resolver_js_bindings.h"
 
 #include "base/logging.h"
+#include "base/string_util.h"
 #include "net/base/address_list.h"
 #include "net/base/host_cache.h"
 #include "net/base/host_resolver.h"
@@ -24,25 +25,31 @@ class DefaultJSBindings : public ProxyResolverJSBindings {
       : host_resolver_(host_resolver) {}
 
   // Handler for "alert(message)".
-  virtual void Alert(const std::string& message) {
+  virtual void Alert(const string16& message) {
     LOG(INFO) << "PAC-alert: " << message;
   }
 
-  // Handler for "myIpAddress()". Returns empty string on failure.
+  // Handler for "myIpAddress()".
   // TODO(eroman): Perhaps enumerate the interfaces directly, using
   // getifaddrs().
-  virtual std::string MyIpAddress() {
-    // DnsResolve("") returns "", so no need to check for failure.
-    return DnsResolve(GetHostName());
+  virtual bool MyIpAddress(std::string* first_ip_address) {
+    std::string my_hostname = GetHostName();
+    if (my_hostname.empty())
+      return false;
+    return DnsResolve(my_hostname, first_ip_address);
   }
 
-  // Handler for "myIpAddressEx()". Returns empty string on failure.
-  virtual std::string MyIpAddressEx() {
-    return DnsResolveEx(GetHostName());
+  // Handler for "myIpAddressEx()".
+  virtual bool MyIpAddressEx(std::string* ip_address_list) {
+    std::string my_hostname = GetHostName();
+    if (my_hostname.empty())
+      return false;
+    return DnsResolveEx(my_hostname, ip_address_list);
   }
 
-  // Handler for "dnsResolve(host)". Returns empty string on failure.
-  virtual std::string DnsResolve(const std::string& host) {
+  // Handler for "dnsResolve(host)".
+  virtual bool DnsResolve(const std::string& host,
+                          std::string* first_ip_address) {
     // Do a sync resolve of the hostname.
     // Disable IPv6 results. We do this because the PAC specification isn't
     // really IPv6 friendly, and Internet Explorer also restricts to IPv4.
@@ -55,26 +62,31 @@ class DefaultJSBindings : public ProxyResolverJSBindings {
 
     int result = DnsResolveHelper(info, &address_list);
     if (result != OK)
-      return std::string();  // Failed.
+      return false;
 
     // TODO(eroman): Is this check really needed? Can I remove it?
     if (!address_list.head())
-      return std::string();
+      return false;
 
     // There may be multiple results; we will just use the first one.
     // This returns empty string on failure.
-    return NetAddressToString(address_list.head());
+    *first_ip_address = net::NetAddressToString(address_list.head());
+    if (first_ip_address->empty())
+      return false;
+
+    return true;
   }
 
-  // Handler for "dnsResolveEx(host)". Returns empty string on failure.
-  virtual std::string DnsResolveEx(const std::string& host) {
+  // Handler for "dnsResolveEx(host)".
+  virtual bool DnsResolveEx(const std::string& host,
+                            std::string* ip_address_list) {
     // Do a sync resolve of the hostname.
     HostResolver::RequestInfo info(host, 80);  // Port doesn't matter.
     AddressList address_list;
     int result = DnsResolveHelper(info, &address_list);
 
     if (result != OK)
-      return std::string();  // Failed.
+      return false;
 
     // Stringify all of the addresses in the address list, separated
     // by semicolons.
@@ -83,15 +95,19 @@ class DefaultJSBindings : public ProxyResolverJSBindings {
     while (current_address) {
       if (!address_list_str.empty())
         address_list_str += ";";
-      address_list_str += NetAddressToString(current_address);
+      const std::string address_string = NetAddressToString(current_address);
+      if (address_string.empty())
+        return false;
+      address_list_str += address_string;
       current_address = current_address->ai_next;
     }
 
-    return address_list_str;
+    *ip_address_list = address_list_str;
+    return true;
   }
 
   // Handler for when an error is encountered. |line_number| may be -1.
-  virtual void OnError(int line_number, const std::string& message) {
+  virtual void OnError(int line_number, const string16& message) {
     if (line_number == -1)
       LOG(INFO) << "PAC-error: " << message;
     else
diff --git a/net/proxy/proxy_resolver_js_bindings.h b/net/proxy/proxy_resolver_js_bindings.h
index 6fc377b..834ea73 100644
--- a/net/proxy/proxy_resolver_js_bindings.h
+++ b/net/proxy/proxy_resolver_js_bindings.h
@@ -7,6 +7,8 @@
 
 #include <string>
 
+#include "base/string16.h"
+
 class MessageLoop;
 
 namespace net {
@@ -22,29 +24,35 @@ class ProxyResolverJSBindings {
   virtual ~ProxyResolverJSBindings() {}
 
   // Handler for "alert(message)"
-  virtual void Alert(const std::string& message) = 0;
+  virtual void Alert(const string16& message) = 0;
 
-  // Handler for "myIpAddress()". Returns empty string on failure.
-  virtual std::string MyIpAddress() = 0;
+  // Handler for "myIpAddress()". Returns true on success and fills
+  // |*first_ip_address| with the result.
+  virtual bool MyIpAddress(std::string* first_ip_address) = 0;
 
-  // Handler for "myIpAddressEx()". Returns empty string on failure.
+  // Handler for "myIpAddressEx()". Returns true on success and fills
+  // |*ip_address_list| with the result.
   //
   // This is a Microsoft extension to PAC for IPv6, see:
   // http://blogs.msdn.com/wndp/articles/IPV6_PAC_Extensions_v0_9.aspx
-  virtual std::string MyIpAddressEx() = 0;
+  virtual bool MyIpAddressEx(std::string* ip_address_list) = 0;
 
-  // Handler for "dnsResolve(host)". Returns empty string on failure.
-  virtual std::string DnsResolve(const std::string& host) = 0;
+  // Handler for "dnsResolve(host)". Returns true on success and fills
+  // |*first_ip_address| with the result.
+  virtual bool DnsResolve(const std::string& host,
+                          std::string* first_ip_address) = 0;
 
-  // Handler for "dnsResolveEx(host)". Returns empty string on failure.
+  // Handler for "dnsResolveEx(host)". Returns true on success and fills
+  // |*ip_address_list| with the result.
   //
   // This is a Microsoft extension to PAC for IPv6, see:
   // http://blogs.msdn.com/wndp/articles/IPV6_PAC_Extensions_v0_9.aspx
-  virtual std::string DnsResolveEx(const std::string& host) = 0;
+  virtual bool DnsResolveEx(const std::string& host,
+                            std::string* ip_address_list) = 0;
 
   // Handler for when an error is encountered. |line_number| may be -1
   // if a line number is not applicable to this error.
-  virtual void OnError(int line_number, const std::string& error) = 0;
+  virtual void OnError(int line_number, const string16& error) = 0;
 
   // Creates a default javascript bindings implementation that will:
   //   - Send script error messages to LOG(INFO)
diff --git a/net/proxy/proxy_resolver_js_bindings_unittest.cc b/net/proxy/proxy_resolver_js_bindings_unittest.cc
index cdb1800..df30a29 100644
--- a/net/proxy/proxy_resolver_js_bindings_unittest.cc
+++ b/net/proxy/proxy_resolver_js_bindings_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/scoped_ptr.h"
+#include "base/string_util.h"
 #include "net/base/address_list.h"
 #include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
@@ -113,18 +114,21 @@ TEST(ProxyResolverJSBindingsTest, DnsResolve) {
   scoped_ptr<ProxyResolverJSBindings> bindings(
       ProxyResolverJSBindings::CreateDefault(host_resolver));
 
+  std::string ip_address;
+
   // Empty string is not considered a valid host (even though on some systems
   // requesting this will resolve to localhost).
   host_resolver->rules()->AddSimulatedFailure("");
-  EXPECT_EQ("", bindings->DnsResolve(""));
+  EXPECT_FALSE(bindings->DnsResolve("", &ip_address));
 
   // Should call through to the HostResolver.
   host_resolver->rules()->AddRule("google.com", "192.168.1.1");
-  EXPECT_EQ("192.168.1.1", bindings->DnsResolve("google.com"));
+  EXPECT_TRUE(bindings->DnsResolve("google.com", &ip_address));
+  EXPECT_EQ("192.168.1.1", ip_address);
 
   // Resolve failures should give empty string.
   host_resolver->rules()->AddSimulatedFailure("fail");
-  EXPECT_EQ("", bindings->DnsResolve("fail"));
+  EXPECT_FALSE(bindings->DnsResolve("fail", &ip_address));
 
   // TODO(eroman): would be nice to have an IPV6 test here too, but that
   // won't work on all systems.
@@ -137,7 +141,8 @@ TEST(ProxyResolverJSBindingsTest, MyIpAddress) {
 
   // Our IP address is always going to be 127.0.0.1, since we are using a
   // mock host resolver.
-  std::string my_ip_address = bindings->MyIpAddress();
+  std::string my_ip_address;
+  EXPECT_TRUE(bindings->MyIpAddress(&my_ip_address));
 
   EXPECT_EQ("127.0.0.1", my_ip_address);
 }
@@ -186,14 +191,25 @@ TEST(ProxyResolverJSBindingsTest, RestrictAddressFamily) {
                                        BoundNetLog()));
   EXPECT_EQ("192.168.1.1", NetAddressToString(address_list.head()));
 
+  std::string ip_address;
   // Now the actual test.
-  EXPECT_EQ("192.168.1.2", bindings->MyIpAddress());       // IPv4 restricted.
-  EXPECT_EQ("192.168.1.1", bindings->DnsResolve("foo"));   // IPv4 restricted.
-  EXPECT_EQ("192.168.1.2", bindings->DnsResolve("foo2"));  // IPv4 restricted.
+  EXPECT_TRUE(bindings->MyIpAddress(&ip_address));
+  EXPECT_EQ("192.168.1.2", ip_address);  // IPv4 restricted.
+
+  EXPECT_TRUE(bindings->DnsResolve("foo", &ip_address));
+  EXPECT_EQ("192.168.1.1", ip_address);  // IPv4 restricted.
+
+  EXPECT_TRUE(bindings->DnsResolve("foo2", &ip_address));
+  EXPECT_EQ("192.168.1.2", ip_address);  // IPv4 restricted.
+
+  EXPECT_TRUE(bindings->MyIpAddressEx(&ip_address));
+  EXPECT_EQ("192.168.2.2", ip_address);  // Unrestricted.
 
-  EXPECT_EQ("192.168.2.2", bindings->MyIpAddressEx());       // Unrestricted.
-  EXPECT_EQ("192.168.2.1", bindings->DnsResolveEx("foo"));   // Unrestricted.
-  EXPECT_EQ("192.168.2.2", bindings->DnsResolveEx("foo2"));  // Unrestricted.
+  EXPECT_TRUE(bindings->DnsResolveEx("foo", &ip_address));
+  EXPECT_EQ("192.168.2.1", ip_address);  // Unrestricted.
+
+  EXPECT_TRUE(bindings->DnsResolveEx("foo2", &ip_address));
+  EXPECT_EQ("192.168.2.2", ip_address);  // Unrestricted.
 }
 
 // Test that myIpAddressEx() and dnsResolveEx() both return a semi-colon
@@ -207,11 +223,13 @@ TEST(ProxyResolverJSBindingsTest, ExFunctionsReturnList) {
   scoped_ptr<ProxyResolverJSBindings> bindings(
       ProxyResolverJSBindings::CreateDefault(host_resolver));
 
-  EXPECT_EQ("192.168.1.1;172.22.34.1;200.100.1.2",
-            bindings->MyIpAddressEx());
+  std::string ip_addresses;
+
+  EXPECT_TRUE(bindings->MyIpAddressEx(&ip_addresses));
+  EXPECT_EQ("192.168.1.1;172.22.34.1;200.100.1.2", ip_addresses);
 
-  EXPECT_EQ("192.168.1.1;172.22.34.1;200.100.1.2",
-            bindings->DnsResolveEx("FOO"));
+  EXPECT_TRUE(bindings->DnsResolveEx("FOO", &ip_addresses));
+  EXPECT_EQ("192.168.1.1;172.22.34.1;200.100.1.2", ip_addresses);
 }
 
 TEST(ProxyResolverJSBindingsTest, PerRequestDNSCache) {
@@ -222,13 +240,15 @@ TEST(ProxyResolverJSBindingsTest, PerRequestDNSCache) {
   scoped_ptr<ProxyResolverJSBindings> bindings(
       ProxyResolverJSBindings::CreateDefault(host_resolver));
 
+  std::string ip_address;
+
   // Call DnsResolve() 4 times for the same hostname -- this should issue
   // 4 separate calls to the underlying host resolver, since there is no
   // current request context.
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
   EXPECT_EQ(4, host_resolver->count());
 
   host_resolver->ResetCount();
@@ -242,20 +262,20 @@ TEST(ProxyResolverJSBindingsTest, PerRequestDNSCache) {
   ProxyResolverRequestContext context(NULL, NULL, &cache);
   bindings->set_current_request_context(&context);
 
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
-  EXPECT_EQ("", bindings->DnsResolve("foo"));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
+  EXPECT_FALSE(bindings->DnsResolve("foo", &ip_address));
   EXPECT_EQ(1, host_resolver->count());
 
   host_resolver->ResetCount();
 
   // The "Ex" version shares this same cache, however since the flags
   // are different it won't reuse this particular entry.
-  EXPECT_EQ("", bindings->DnsResolveEx("foo"));
+  EXPECT_FALSE(bindings->DnsResolveEx("foo", &ip_address));
   EXPECT_EQ(1, host_resolver->count());
-  EXPECT_EQ("", bindings->DnsResolveEx("foo"));
-  EXPECT_EQ("", bindings->DnsResolveEx("foo"));
+  EXPECT_FALSE(bindings->DnsResolveEx("foo", &ip_address));
+  EXPECT_FALSE(bindings->DnsResolveEx("foo", &ip_address));
   EXPECT_EQ(1, host_resolver->count());
 
   bindings->set_current_request_context(NULL);
diff --git a/net/proxy/proxy_resolver_v8.cc b/net/proxy/proxy_resolver_v8.cc
index 22c4a95..b17ba4a 100644
--- a/net/proxy/proxy_resolver_v8.cc
+++ b/net/proxy/proxy_resolver_v8.cc
@@ -70,22 +70,25 @@ const char kPacResourceName[] = "proxy-pac-script.js";
 // Pseudo-name for the PAC utility script.
 const char kPacUtilityResourceName[] = "proxy-pac-utility-script.js";
 
-// Convert a V8 String to a std::string.
-std::string V8StringToStdString(v8::Handle<v8::String> s) {
-  int len = s->Utf8Length();
-  std::string result;
-  s->WriteUtf8(WriteInto(&result, len + 1), len);
+// Converts a V8 String to a UTF16 string16.
+string16 V8StringToUTF16(v8::Handle<v8::String> s) {
+  int len = s->Length();
+  string16 result;
+  // Note that the reinterpret cast is because on Windows string16 is an alias
+  // to wstring, and hence has character type wchar_t not uint16_t.
+  s->Write(reinterpret_cast<uint16_t*>(WriteInto(&result, len + 1)), 0, len);
   return result;
 }
 
-// Convert a std::string (UTF8) to a V8 string.
-v8::Local<v8::String> StdStringToV8String(const std::string& s) {
+// Converts a std::string (UTF8) to a V8 string.
+v8::Local<v8::String> UTF8StdStringToV8String(const std::string& s) {
   return v8::String::New(s.data(), s.size());
 }
 
-// String-ize a V8 object by calling its toString() method. Returns true
+// Stringizes a V8 object by calling its toString() method. Returns true
 // on success. This may fail if the toString() throws an exception.
-bool V8ObjectToString(v8::Handle<v8::Value> object, std::string* result) {
+bool V8ObjectToUTF16String(v8::Handle<v8::Value> object,
+                           string16* utf16_result) {
   if (object.IsEmpty())
     return false;
 
@@ -93,7 +96,25 @@ bool V8ObjectToString(v8::Handle<v8::Value> object, std::string* result) {
   v8::Local<v8::String> str_object = object->ToString();
   if (str_object.IsEmpty())
     return false;
-  *result = V8StringToStdString(str_object);
+  *utf16_result = V8StringToUTF16(str_object);
+  return true;
+}
+
+// Extracts an hostname argument from |args|. On success returns true
+// and fills |*hostname| with the result.
+bool GetHostnameArgument(const v8::Arguments& args, std::string* hostname) {
+  // The first argument should be a string.
+  if (args.Length() == 0 || args[0].IsEmpty() || !args[0]->IsString())
+    return false;
+
+  const string16 hostname_utf16 = V8StringToUTF16(args[0]->ToString());
+
+  // TODO(eroman): Convert international domain names to punycode. Right
+  //               now we fail if the hostname isn't entered in ASCII.
+  //               crbug.com/47234
+  if (!IsStringASCII(hostname_utf16))
+    return false;
+  *hostname = UTF16ToASCII(hostname_utf16);
   return true;
 }
 
@@ -123,13 +144,14 @@ class ProxyResolverV8::Context {
 
     v8::Local<v8::Value> function;
     if (!GetFindProxyForURL(&function)) {
-      js_bindings_->OnError(-1, "FindProxyForURL() is undefined.");
+      js_bindings_->OnError(
+          -1, ASCIIToUTF16("FindProxyForURL() is undefined."));
       return ERR_PAC_SCRIPT_FAILED;
     }
 
     v8::Handle<v8::Value> argv[] = {
-      StdStringToV8String(query_url.spec()),
-      StdStringToV8String(query_url.host()),
+      UTF8StdStringToV8String(query_url.spec()),
+      UTF8StdStringToV8String(query_url.host()),
     };
 
     v8::TryCatch try_catch;
@@ -142,14 +164,26 @@ class ProxyResolverV8::Context {
     }
 
     if (!ret->IsString()) {
-      js_bindings_->OnError(-1, "FindProxyForURL() did not return a string.");
+      js_bindings_->OnError(
+          -1, ASCIIToUTF16("FindProxyForURL() did not return a string."));
       return ERR_PAC_SCRIPT_FAILED;
     }
 
-    std::string ret_str = V8StringToStdString(ret->ToString());
-
-    results->UsePacString(ret_str);
+    string16 ret_str = V8StringToUTF16(ret->ToString());
+
+    if (!IsStringASCII(ret_str)) {
+      // TODO(eroman): Rather than failing when a wide string is returned, we
+      //               could extend the parsing to handle IDNA hostnames by
+      //               converting them to ASCII punycode.
+      //               crbug.com/47234
+      string16 error_message =
+          ASCIIToUTF16("FindProxyForURL() returned a non-ASCII string "
+                       "(crbug.com/47234): ") + ret_str;
+      js_bindings_->OnError(-1, error_message);
+      return ERR_PAC_SCRIPT_FAILED;
+    }
 
+    results->UsePacString(UTF16ToASCII(ret_str));
     return OK;
   }
 
@@ -243,8 +277,8 @@ class ProxyResolverV8::Context {
 
     // Otherwise dispatch to the bindings.
     int line_number = message->GetLineNumber();
-    std::string error_message;
-    V8ObjectToString(message->Get(), &error_message);
+    string16 error_message;
+    V8ObjectToUTF16String(message->Get(), &error_message);
     js_bindings_->OnError(line_number, error_message);
   }
 
@@ -254,7 +288,7 @@ class ProxyResolverV8::Context {
     v8::TryCatch try_catch;
 
     // Compile the script.
-    v8::Local<v8::String> text = StdStringToV8String(script_utf8);
+    v8::Local<v8::String> text = UTF8StdStringToV8String(script_utf8);
     v8::ScriptOrigin origin = v8::ScriptOrigin(v8::String::New(script_name));
     v8::Local<v8::Script> code = v8::Script::Compile(text, &origin);
 
@@ -278,11 +312,11 @@ class ProxyResolverV8::Context {
 
     // Like firefox we assume "undefined" if no argument was specified, and
     // disregard any arguments beyond the first.
-    std::string message;
+    string16 message;
     if (args.Length() == 0) {
-      message = "undefined";
+      message = ASCIIToUTF16("undefined");
     } else {
-      if (!V8ObjectToString(args[0], &message))
+      if (!V8ObjectToUTF16String(args[0], &message))
         return v8::Undefined();  // toString() threw an exception.
     }
 
@@ -296,6 +330,7 @@ class ProxyResolverV8::Context {
         static_cast<Context*>(v8::External::Cast(*args.Data())->Value());
 
     std::string result;
+    bool success;
 
     {
       v8::Unlocker unlocker;
@@ -307,7 +342,7 @@ class ProxyResolverV8::Context {
 
       // We shouldn't be called with any arguments, but will not complain if
       // we are.
-      result = context->js_bindings_->MyIpAddress();
+      success = context->js_bindings_->MyIpAddress(&result);
 
       LogEventToCurrentRequest(context,
                                NetLog::PHASE_END,
@@ -315,9 +350,9 @@ class ProxyResolverV8::Context {
                                NULL);
     }
 
-    if (result.empty())
+    if (!success)
       result = "127.0.0.1";
-    return StdStringToV8String(result);
+    return UTF8StdStringToV8String(result);
   }
 
   // V8 callback for when "myIpAddressEx()" is invoked by the PAC script.
@@ -326,7 +361,8 @@ class ProxyResolverV8::Context {
     Context* context =
         static_cast<Context*>(v8::External::Cast(*args.Data())->Value());
 
-    std::string result;
+    std::string ip_address_list;
+    bool success;
 
     {
       v8::Unlocker unlocker;
@@ -338,7 +374,7 @@ class ProxyResolverV8::Context {
 
       // We shouldn't be called with any arguments, but will not complain if
       // we are.
-      context->js_bindings_->MyIpAddressEx();
+      success = context->js_bindings_->MyIpAddressEx(&ip_address_list);
 
       LogEventToCurrentRequest(context,
                                NetLog::PHASE_END,
@@ -346,7 +382,9 @@ class ProxyResolverV8::Context {
                                NULL);
     }
 
-    return StdStringToV8String(result);
+    if (!success)
+      ip_address_list = std::string();
+    return UTF8StdStringToV8String(ip_address_list);
   }
 
   // V8 callback for when "dnsResolve()" is invoked by the PAC script.
@@ -355,11 +393,12 @@ class ProxyResolverV8::Context {
         static_cast<Context*>(v8::External::Cast(*args.Data())->Value());
 
     // We need at least one string argument.
-    if (args.Length() == 0 || args[0].IsEmpty() || !args[0]->IsString())
+    std::string hostname;
+    if (!GetHostnameArgument(args, &hostname))
       return v8::Null();
-    std::string host = V8StringToStdString(args[0]->ToString());
 
-    std::string result;
+    std::string ip_address;
+    bool success;
 
     {
       v8::Unlocker unlocker;
@@ -369,7 +408,7 @@ class ProxyResolverV8::Context {
                                NetLog::TYPE_PROXY_RESOLVER_V8_DNS_RESOLVE,
                                NULL);
 
-      result = context->js_bindings_->DnsResolve(host);
+      success = context->js_bindings_->DnsResolve(hostname, &ip_address);
 
       LogEventToCurrentRequest(context,
                                NetLog::PHASE_END,
@@ -377,8 +416,7 @@ class ProxyResolverV8::Context {
                                NULL);
     }
 
-    // DnsResolve() returns empty string on failure.
-    return result.empty() ? v8::Null() : StdStringToV8String(result);
+    return success ? UTF8StdStringToV8String(ip_address) : v8::Null();
   }
 
   // V8 callback for when "dnsResolveEx()" is invoked by the PAC script.
@@ -387,11 +425,12 @@ class ProxyResolverV8::Context {
         static_cast<Context*>(v8::External::Cast(*args.Data())->Value());
 
     // We need at least one string argument.
-    if (args.Length() == 0 || args[0].IsEmpty() || !args[0]->IsString())
+    std::string hostname;
+    if (!GetHostnameArgument(args, &hostname))
       return v8::Undefined();
-    std::string host = V8StringToStdString(args[0]->ToString());
 
-    std::string result;
+    std::string ip_address_list;
+    bool success;
 
     {
       v8::Unlocker unlocker;
@@ -401,7 +440,8 @@ class ProxyResolverV8::Context {
                                NetLog::TYPE_PROXY_RESOLVER_V8_DNS_RESOLVE_EX,
                                NULL);
 
-      result = context->js_bindings_->DnsResolveEx(host);
+      success = context->js_bindings_->DnsResolveEx(hostname,
+                                                    &ip_address_list);
 
       LogEventToCurrentRequest(context,
                                NetLog::PHASE_END,
@@ -409,7 +449,10 @@ class ProxyResolverV8::Context {
                                NULL);
     }
 
-    return StdStringToV8String(result);
+    if (!success)
+      ip_address_list = std::string();
+
+    return UTF8StdStringToV8String(ip_address_list);
   }
 
   static void LogEventToCurrentRequest(Context* context,
diff --git a/net/proxy/proxy_resolver_v8_unittest.cc b/net/proxy/proxy_resolver_v8_unittest.cc
index b6d8a47..5146fc6 100644
--- a/net/proxy/proxy_resolver_v8_unittest.cc
+++ b/net/proxy/proxy_resolver_v8_unittest.cc
@@ -3,8 +3,9 @@
 // found in the LICENSE file.
 
 #include "base/file_util.h"
-#include "base/string_util.h"
 #include "base/path_service.h"
+#include "base/string_util.h"
+#include "base/utf_string_conversions.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_log_unittest.h"
 #include "net/base/net_errors.h"
@@ -23,36 +24,41 @@ class MockJSBindings : public ProxyResolverJSBindings {
  public:
   MockJSBindings() : my_ip_address_count(0), my_ip_address_ex_count(0) {}
 
-  virtual void Alert(const std::string& message) {
+  virtual void Alert(const string16& message) {
     LOG(INFO) << "PAC-alert: " << message;  // Helpful when debugging.
-    alerts.push_back(message);
+    alerts.push_back(UTF16ToUTF8(message));
   }
 
-  virtual std::string MyIpAddress() {
+  virtual bool MyIpAddress(std::string* ip_address) {
     my_ip_address_count++;
-    return my_ip_address_result;
+    *ip_address = my_ip_address_result;
+    return !my_ip_address_result.empty();
   }
 
-  virtual std::string MyIpAddressEx() {
+  virtual bool MyIpAddressEx(std::string* ip_address_list) {
     my_ip_address_ex_count++;
-    return my_ip_address_ex_result;
+    *ip_address_list = my_ip_address_ex_result;
+    return !my_ip_address_ex_result.empty();
   }
 
-  virtual std::string DnsResolve(const std::string& host) {
+  virtual bool DnsResolve(const std::string& host, std::string* ip_address) {
     dns_resolves.push_back(host);
-    return dns_resolve_result;
+    *ip_address = dns_resolve_result;
+    return !dns_resolve_result.empty();
   }
 
-  virtual std::string DnsResolveEx(const std::string& host) {
+  virtual bool DnsResolveEx(const std::string& host,
+                            std::string* ip_address_list) {
     dns_resolves_ex.push_back(host);
-    return dns_resolve_ex_result;
+    *ip_address_list = dns_resolve_ex_result;
+    return !dns_resolve_ex_result.empty();
   }
 
-  virtual void OnError(int line_number, const std::string& message) {
+  virtual void OnError(int line_number, const string16& message) {
     // Helpful when debugging.
     LOG(INFO) << "PAC-error: [" << line_number << "] " << message;
 
-    errors.push_back(message);
+    errors.push_back(UTF16ToUTF8(message));
     errors_line_number.push_back(line_number);
   }
 
@@ -306,9 +312,7 @@ TEST(ProxyResolverV8Test, UnhandledException) {
   EXPECT_EQ(3, bindings->errors_line_number[0]);
 }
 
-// TODO(eroman): This test fails right now, since the parsing of
-// host/port doesn't check for non-ascii characters. http://crbug.com/46608
-TEST(ProxyResolverV8Test, FAILS_ReturnUnicode) {
+TEST(ProxyResolverV8Test, ReturnUnicode) {
   ProxyResolverV8WithMockBindings resolver;
   int result = resolver.SetPacScriptFromDisk("return_unicode.js");
   EXPECT_EQ(OK, result);
@@ -318,7 +322,7 @@ TEST(ProxyResolverV8Test, FAILS_ReturnUnicode) {
                                    BoundNetLog());
 
   // The result from this resolve was unparseable, because it
-  // wasn't ascii.
+  // wasn't ASCII.
   EXPECT_EQ(ERR_PAC_SCRIPT_FAILED, result);
 }