Move Service Worker %2f validation logic from browser into Blink (3)

content/browser/service_worker/service_worker_dispatcher_host.cc

 // Copyright 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.
 
 #include "content/browser/service_worker/service_worker_dispatcher_host.h"
 
 #include "base/logging.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
@@ skipping 38 matching lines @@
 
 bool AllOriginsMatch(const GURL& url_a, const GURL& url_b, const GURL& url_c) {
   return url_a.GetOrigin() == url_b.GetOrigin() &&
          url_a.GetOrigin() == url_c.GetOrigin();
 }
 
 bool OriginCanAccessServiceWorkers(const GURL& url) {
   return url.SchemeIsHTTPOrHTTPS() && IsOriginSecure(url);
 }
 
+bool ContainsDisallowedCharacter(const GURL& url) {
+  DCHECK(url.is_valid());
+
+  std::string path = url.path();
+  DCHECK(base::IsStringUTF8(path));
+
+  // We should avoid these escaped characters in the path component because
+  // these can be handled differently depending on server implementation.
+  if (path.find("%2f") != std::string::npos ||
+      path.find("%2F") != std::string::npos) {
+    return true;
+  }
+  if (path.find("%5c") != std::string::npos ||
+      path.find("%5C") != std::string::npos) {
+    return true;
+  }
+  return false;
+}
+
 bool CanRegisterServiceWorker(const GURL& document_url,
                               const GURL& pattern,
                               const GURL& script_url) {
   DCHECK(document_url.is_valid());
   DCHECK(pattern.is_valid());
   DCHECK(script_url.is_valid());
   return AllOriginsMatch(document_url, pattern, script_url) &&
          OriginCanAccessServiceWorkers(document_url) &&
          OriginCanAccessServiceWorkers(pattern) &&
          OriginCanAccessServiceWorkers(script_url);
@@ skipping 256 matching lines @@
             base::ASCIIToUTF16(kNoDocumentURLErrorMessage)));
     return;
   }
 
   if (!CanRegisterServiceWorker(
       provider_host->document_url(), pattern, script_url)) {
     bad_message::ReceivedBadMessage(this, bad_message::SWDH_REGISTER_CANNOT);
     return;
   }
 
-  std::string error_message;
-  if (ServiceWorkerUtils::ContainsDisallowedCharacter(pattern, script_url,
-                                                      &error_message)) {
-    Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
-        thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity,
-        base::ASCIIToUTF16(kServiceWorkerRegisterErrorPrefix) +
-            base::UTF8ToUTF16(error_message)));
+  if (ContainsDisallowedCharacter(pattern) ||
+      ContainsDisallowedCharacter(script_url)) {

[nhiroki, 2015/07/28 05:32:13]

I'd prefer to reuse ServiceWorkerUtils::ContainsDisallowedCharacter because it's
generally difficult to keep duplicate code consistent. How about making the
function to accept null |error_message|?

+    bad_message::ReceivedBadMessage(this, bad_message::SWDH_REGISTER_CANNOT);
     return;
   }
 
   if (!GetContentClient()->browser()->AllowServiceWorker(
           pattern, provider_host->topmost_frame_url(), resource_context_,
           render_process_id_, provider_host->frame_id())) {
     Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError(
         thread_id, request_id, WebServiceWorkerError::ErrorTypeUnknown,
         base::ASCIIToUTF16(kServiceWorkerRegisterErrorPrefix) +
             base::ASCIIToUTF16(kUserDeniedPermissionMessage)));
@@ skipping 864 matching lines @@
   if (!handle) {
     bad_message::ReceivedBadMessage(this,
                                     bad_message::SWDH_TERMINATE_BAD_HANDLE);
     return;
   }
   handle->version()->StopWorker(
       base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
 }
 
 }  // namespace content