[DevUI DFM] Improve usability of chrome://dev-ui-loader URL redirection.

Visiting the chrome:// URL of a host that's in the DevUI DFM triggers
install / load flow through chrome://dev-ui-loader, which then
redirects to the original chrome:// URL on completion. Previously the
flow suffers from the following:
* Bug: The state "installed" should be "ready".
* /path?query/fragment of the original URL is discarded.
* Redirect to chrome://dev-ui-loader is possible, leading to bizarre
  redirect chains.

This CL fixes the above by replacing "?page=<host>" with
"?url=<escaped URL>". This involves escaping the URL in the (C++) URL
rendering code and unescaping the URL in the (JS) URL reading code.
Invalid "?url=" simply leads to chrome://chrome-urls.

Bug: 927131
Change-Id: Ia7c1c7874f76ee6d80b347fc49a73dec50a5c80c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1783519
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Dan Beam <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693206}

chrome/browser/android/dev_ui/dev_ui_url_handler.cc

@@ -8,6 +8,7 @@
 
 #include "chrome/browser/android/dev_ui/dev_ui_module_provider.h"
 #include "chrome/common/webui_url_constants.h"
+#include "net/base/url_util.h"
 #include "url/gurl.h"
 
 namespace chrome {
@@ -32,12 +33,11 @@ bool HandleDfmifiedDevUiPageURL(
     // No need to check ModuleInstalled(): It's implied by ModuleLoaded().
     return false;
   }
-  // Provide |url->host()| so the DevUI loader can redirect to the proper page
-  // after install / load.
-  // TODO(huangs): Escape /path?query#fragment and pass these; unescape in
-  // chrome://dev-ui-loader when redirecting.
-  *url = GURL(std::string(kChromeUIDevUiLoaderURL) +
-              "dev_ui_loader.html?page=" + url->host());
+  // Create URL to the DevUI loader with "?url=<escaped original URL>" so that
+  // after install / load, the loader can redirect to the original URL.
+  *url = net::AppendQueryParameter(
+      GURL(std::string(kChromeUIDevUiLoaderURL) + "dev_ui_loader.html"), "url",
+      url->spec());
   return true;
 }
 

chrome/browser/resources/dev_ui_loader/dev_ui_loader.js

@@ -5,29 +5,44 @@
 (function() {
 'use strict';
 
-/** @param {string} page */
-function redirectToChromePage(page) {
+/** @return {!URL} */
+function getRedirectUrl() {
+  try {  // For potential TypeError from new URL().
+    const urlString = new URL(location).searchParams.get('url');
+    if (urlString) {
+      const url = new URL(decodeURIComponent(urlString));
+      // Perform basic filtering. Also skip 'dev-ui-loader' to avoid redirect
+      // cycle (benign but bizarre). Note that |url.host| is always lowercase.
+      if (url.protocol === 'chrome:' && url.host.match(/[a-z0-9_\-]+/) &&
+          url.host !== 'dev-ui-loader') {
+        return url;
+      }
+    }
+  } catch (e) {
+  }
+  return new URL('chrome://chrome-urls');
+}
+
+function redirectToChromePage() {
   // Use replace() so the current page disappears from history.
-  location.replace('chrome://' + page);
+  location.replace(getRedirectUrl());
 }
 
 function doInstall() {
   cr.sendWithPromise('installAndLoadDevUiDfm').then((response) => {
-    const query = new URL(window.location).searchParams;
-    const targetPage = (query.get('page') || '').replace(/[^a-z0-9_\-]/g, '');
     const status = response[0];
-    if (status === 'failure') {
+    if (status === 'success' || status === 'noop') {
+      redirectToChromePage();
+    } else if (status === 'failure') {
       document.querySelector('#failure-message').hidden = false;
-    } else if (status === 'success' || status === 'noop') {
-      redirectToChromePage(targetPage);
     }
   });
 }
 
 document.addEventListener('DOMContentLoaded', () => {
   cr.sendWithPromise('getDevUiDfmState').then((state) => {
-    if (state === 'installed') {
-      redirectToChromePage('chrome-urls');
+    if (state === 'ready') {
+      redirectToChromePage();
     } else if (state === 'not-installed' || 'not-loaded') {
       doInstall();
     }