Properly decode IDN in interstitials

Decode IDN hostnames in SecurityInterstitialPage before displaying
them. Add browser tests for SSLBlockingPage and
CaptivePortalBlockingPage to test this behavior. Other blocking pages
can subclass the new SecurityInterstitialIDNTest class to do similar
tests.

BUG=450883

Review URL: https://codereview.chromium.org/1000333003

Cr-Commit-Position: refs/heads/master@{#322128}

chrome/browser/interstitials/security_interstitial_page.cc

@@ -5,12 +5,16 @@
 #include "chrome/browser/interstitials/security_interstitial_page.h"
 
 #include "base/i18n/rtl.h"
+#include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/grit/browser_resources.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/web_contents.h"
+#include "net/base/net_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/webui/jstemplate_builder.h"
 #include "ui/base/webui/web_ui_util.h"
@@ -55,7 +59,12 @@ void SecurityInterstitialPage::Show() {
 }
 
 base::string16 SecurityInterstitialPage::GetFormattedHostName() const {
-  base::string16 host(base::UTF8ToUTF16(request_url_.host()));
+  std::string languages;
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents()->GetBrowserContext());
+  if (profile)
+    languages = profile->GetPrefs()->GetString(prefs::kAcceptLanguages);
+  base::string16 host = net::IDNToUnicode(request_url_.host(), languages);
   if (base::i18n::IsRTL())
     base::i18n::WrapStringWithLTRFormatting(&host);
   return host;

chrome/browser/interstitials/security_interstitial_page.h

@@ -24,6 +24,9 @@ class SecurityInterstitialPage : public content::InterstitialPageDelegate {
   // DO NOT reorder or change these without also changing the JavaScript!
   // See chrome/browser/resources/security_warnings/interstitial_v2.js
   enum SecurityInterstitialCommands {
+    // Used by tests
+    CMD_TEXT_FOUND = -2,
+    CMD_TEXT_NOT_FOUND = -1,
     // Decisions
     CMD_DONT_PROCEED = 0,
     CMD_PROCEED = 1,

chrome/browser/interstitials/security_interstitial_page_test_utils.cc

+// Copyright 2015 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.
+
+#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
+
+#include <string>
+
+#include "base/prefs/pref_service.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/interstitials/security_interstitial_page.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/pref_names.h"
+#include "content/public/browser/interstitial_page.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace chrome_browser_interstitials {
+
+bool IsInterstitialDisplayingText(content::InterstitialPage* interstitial,
+                                  const std::string& text) {
+  // It's valid for |text| to contain "\'", but simply look for "'" instead
+  // since this function is used for searching host names and a predefined
+  // string.
+  DCHECK(text.find("\'") == std::string::npos);
+  std::string command = base::StringPrintf(
+      "var hasText = document.body.textContent.indexOf('%s') >= 0;"
+      "window.domAutomationController.send(hasText ? %d : %d);",
+      text.c_str(), SecurityInterstitialPage::CMD_TEXT_FOUND,
+      SecurityInterstitialPage::CMD_TEXT_NOT_FOUND);
+  int result = 0;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractInt(interstitial->GetMainFrame(),
+                                                  command, &result));
+  return result == SecurityInterstitialPage::CMD_TEXT_FOUND;
+}
+
+void SecurityInterstitialIDNTest::SetUpOnMainThread() {
+  // Clear AcceptLanguages to force punycode decoding.
+  browser()->profile()->GetPrefs()->SetString(prefs::kAcceptLanguages,
+                                              std::string());
+}
+
+testing::AssertionResult SecurityInterstitialIDNTest::VerifyIDNDecoded() const {
+  const char kHostname[] = "xn--d1abbgf6aiiy.xn--p1ai";
+  const char kHostnameJSUnicode[] =
+      "\\u043f\\u0440\\u0435\\u0437\\u0438\\u0434\\u0435\\u043d\\u0442."
+      "\\u0440\\u0444";
+  std::string request_url_spec = base::StringPrintf("https://%s/", kHostname);
+  GURL request_url(request_url_spec);
+
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  DCHECK(contents);
+  SecurityInterstitialPage* blocking_page =
+      CreateInterstitial(contents, request_url);
+  blocking_page->Show();
+
+  WaitForInterstitialAttach(contents);
+  if (!WaitForRenderFrameReady(contents->GetInterstitialPage()->GetMainFrame()))
+    return testing::AssertionFailure() << "Render frame not ready";
+
+  if (IsInterstitialDisplayingText(contents->GetInterstitialPage(),
+                                   kHostnameJSUnicode)) {
+    return testing::AssertionSuccess();
+  }
+  return testing::AssertionFailure() << "Interstitial not displaying text";
+}
+}  // namespace chrome_browser_interstitials

chrome/browser/interstitials/security_interstitial_page_test_utils.h

+// Copyright 2015 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.
+
+#ifndef CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_TEST_UTILS_H_
+#define CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_TEST_UTILS_H_
+
+#include <string>
+
+#include "chrome/test/base/in_process_browser_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+class InterstitialPage;
+class WebContents;
+}
+
+class GURL;
+class SecurityInterstitialPage;
+
+namespace chrome_browser_interstitials {
+
+bool IsInterstitialDisplayingText(content::InterstitialPage* interstitial,
+                                  const std::string& text);
+
+// This class is used for testing the display of IDN names in security
+// interstitials.
+class SecurityInterstitialIDNTest : public InProcessBrowserTest {
+ public:
+  // InProcessBrowserTest implementation
+  void SetUpOnMainThread() override;
+
+  // Run a test that creates an interstitial with an IDN request URL
+  // and checks that it is properly decoded.
+  testing::AssertionResult VerifyIDNDecoded() const;
+
+ protected:
+  virtual SecurityInterstitialPage* CreateInterstitial(
+      content::WebContents* contents,
+      const GURL& request_url) const = 0;
+};
+
+}  // namespace chrome_browser_interstitials
+
+#endif  // CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_TEST_UTILS_H_

chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/stringprintf.h"
+#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -23,6 +24,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+using chrome_browser_interstitials::IsInterstitialDisplayingText;
+using chrome_browser_interstitials::SecurityInterstitialIDNTest;
+
 namespace {
 // Partial text in the captive portal interstitial's main paragraph when the
 // login domain isn't displayed.
@@ -30,23 +34,6 @@ const char kGenericLoginURLText[] = "its login page";
 const char kBrokenSSL[] = "https://broken.ssl";
 const char kWiFiSSID[] = "WiFiSSID";
 
-// Returns true if the interstitial contains |text| in its body.
-bool IsInterstitialDisplayingText(content::InterstitialPage* interstitial,
-                                  const std::string& text) {
-  // It's valid for |text| to contain "\'", but simply look for "'" instead
-  // since this function is used for searching host names and a predefined
-  // string.
-  DCHECK(text.find("\'") == std::string::npos);
-  std::string command = base::StringPrintf(
-      "var hasText = document.body.textContent.indexOf('%s') >= 0;"
-      "window.domAutomationController.send(hasText ? 1 : 0);",
-      text.c_str());
-  int result = 0;
-  EXPECT_TRUE(content::ExecuteScriptAndExtractInt(
-      interstitial->GetMainFrame(), command, &result));
-  return result == 1;
-}
-
 enum ExpectWiFi {
   EXPECT_WIFI_NO,
   EXPECT_WIFI_YES
@@ -220,29 +207,25 @@ IN_PROC_BROWSER_TEST_F(CaptivePortalBlockingPageTest,
                    EXPECT_WIFI_YES, EXPECT_WIFI_SSID_NO, EXPECT_LOGIN_URL_NO);
 }
 
-class CaptivePortalBlockingPageIDNTest : public CaptivePortalBlockingPageTest {
-public:
-  // InProcessBrowserTest:
-  void SetUpOnMainThread() override {
-    // Clear AcceptLanguages to force punycode decoding.
-    browser()->profile()->GetPrefs()->SetString(prefs::kAcceptLanguages,
-                                                std::string());
+class CaptivePortalBlockingPageIDNTest : public SecurityInterstitialIDNTest {
+ protected:
+  // SecurityInterstitialIDNTest implementation
+  SecurityInterstitialPage* CreateInterstitial(
+      content::WebContents* contents,
+      const GURL& request_url) const override {
+    // Delegate is owned by the blocking page.
+    FakeConnectionInfoDelegate* delegate =
+        new FakeConnectionInfoDelegate(false, "");
+    // Blocking page is owned by the interstitial.
+    CaptivePortalBlockingPage* blocking_page = new CaptivePortalBlockingPage(
+        contents, GURL(kBrokenSSL), request_url, base::Callback<void(bool)>());
+    blocking_page->SetDelegateForTesting(delegate);
+    return blocking_page;
   }
 };
 
-// Same as CaptivePortalBlockingPageTest.WiredNetwork_LoginURL, except the login
-// domain is an IDN.
+// Test that an IDN login domain is decoded properly.
 IN_PROC_BROWSER_TEST_F(CaptivePortalBlockingPageIDNTest,
                        ShowLoginIDNIfPortalRedirectsDetectionURL) {
-  const char kHostname[] =
-      "xn--d1abbgf6aiiy.xn--p1ai";
-  const char kHostnameJSUnicode[] =
-      "\\u043f\\u0440\\u0435\\u0437\\u0438\\u0434\\u0435\\u043d\\u0442."
-      "\\u0440\\u0444";
-  std::string landing_url_spec =
-      base::StringPrintf("http://%s/landing_url", kHostname);
-  GURL landing_url(landing_url_spec);
-
-  TestInterstitial(false, "", landing_url, EXPECT_WIFI_NO, EXPECT_WIFI_SSID_NO,
-                   EXPECT_LOGIN_URL_YES, kHostnameJSUnicode);
+  EXPECT_TRUE(VerifyIDNDecoded());
 }

chrome/browser/ssl/ssl_blocking_page.cc

@@ -456,6 +456,12 @@ void SSLBlockingPage::OverrideEntry(NavigationEntry* entry) {
 // This handles the commands sent from the interstitial JavaScript.
 // DO NOT reorder or change this logic without also changing the JavaScript!
 void SSLBlockingPage::CommandReceived(const std::string& command) {
+  if (command == "\"pageLoadComplete\"") {
+    // content::WaitForRenderFrameReady sends this message when the page
+    // load completes. Ignore it.
+    return;
+  }
+
   int cmd = 0;
   bool retval = base::StringToInt(command, &cmd);
   DCHECK(retval);

chrome/browser/ssl/ssl_browser_tests.cc

@@ -4,6 +4,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/callback.h"
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
@@ -12,6 +13,7 @@
 #include "base/time/time.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
 #include "chrome/browser/ui/browser.h"
@@ -44,6 +46,8 @@
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/cert_status_flags.h"
+#include "net/cert/x509_certificate.h"
+#include "net/ssl/ssl_info.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 
 #if defined(USE_NSS)
@@ -53,6 +57,7 @@
 #endif  // defined(USE_NSS)
 
 using base::ASCIIToUTF16;
+using chrome_browser_interstitials::SecurityInterstitialIDNTest;
 using content::InterstitialPage;
 using content::NavigationController;
 using content::NavigationEntry;
@@ -1928,6 +1933,25 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, InterstitialNotAffectedByHideShow) {
   EXPECT_TRUE(tab->GetRenderWidgetHostView()->IsShowing());
 }
 
+class SSLBlockingPageIDNTest : public SecurityInterstitialIDNTest {
+ protected:
+  // SecurityInterstitialIDNTest implementation
+  SecurityInterstitialPage* CreateInterstitial(
+      content::WebContents* contents,
+      const GURL& request_url) const override {
+    net::SSLInfo ssl_info;
+    ssl_info.cert = new net::X509Certificate(
+        request_url.host(), "CA", base::Time::Max(), base::Time::Max());
+    return new SSLBlockingPage(
+        contents, net::ERR_CERT_CONTAINS_ERRORS, ssl_info, request_url, 0,
+        base::Time::NowFromSystemTime(), base::Callback<void(bool)>());
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(SSLBlockingPageIDNTest, SSLBlockingPageDecodesIDN) {
+  EXPECT_TRUE(VerifyIDNDecoded());
+}
+
 // TODO(jcampan): more tests to do below.
 
 // Visit a page over https that contains a frame with a redirect.

chrome/chrome_tests.gypi

@@ -299,6 +299,8 @@
       'browser/importer/importer_unittest_utils.cc',
       'browser/importer/importer_unittest_utils.h',
       'browser/infobars/infobars_browsertest.cc',
+      'browser/interstitials/security_interstitial_page_test_utils.cc',
+      'browser/interstitials/security_interstitial_page_test_utils.h',
       'browser/invalidation/profile_invalidation_provider_factory_browsertest.cc',
       'browser/lifetime/browser_close_manager_browsertest.cc',
       'browser/loadtimes_extension_bindings_browsertest.cc',