Avoid blocking the PNaCl translator processes when chrome NaCl GDB flag is on.

chrome/browser/nacl_host/nacl_browser_delegate_impl.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 "chrome/browser/nacl_host/nacl_browser_delegate_impl.h"
 
 #include "base/path_service.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 146 matching lines @@
     debug_patterns.erase(0, 1);
   }
   if (debug_patterns.empty()) {
     return;
   }
   std::vector<std::string> patterns;
   base::SplitString(debug_patterns, ',', &patterns);
   for (std::vector<std::string>::iterator iter = patterns.begin();
        iter != patterns.end(); ++iter) {
     URLPattern pattern;
+    // Allow chrome:// schema, which is used to filter out the internal
+    // PNaCl translator. Also allow chrome-extension:// schema (which
+    // can have NaCl modules). The default is to disallow these schema
+    // since they can be dangerous in the context of chrome extension
+    // permissions, but they are okay here, for NaCl GDB avoidance.
+    pattern.SetValidSchemes(URLPattern::SCHEME_ALL);

[teravest, 2014/02/25 18:51:29]

I'm confused. Why do you want to set the scheme here, even when the pattern
isn't parsed successfully?

It seems like the existing code already sets valid schemes properly for
 all parsed patterns. Is the problem that Parse() fails without
SetValidSchemes()?


It's weird to have "pattern.SetValidSchemes(URLPattern::SCHEME_ALL)" called
twice here. I don't understand why that would be necessary.

[jvoung (off chromium), 2014/02/25 23:52:31]

Yes, the problem is that Parse() would otherwise fail when it sees a pattern
like "chrome://*/foo*". It would however be happy with the less strict
"*://*/foo*".
There's two points at which the scheme matter:

(1) When you are setting up the pattern matcher. E.g., when you're trying to
parse *://ssh*, or chrome://*

(2) When you're doing the match against a real URL. E.g.,
chrome-extension://abcdefg/ssh_client.nmf.

The added early setting for SCHEME_ALL helps with case (1).

The existing setting for SCHEME_ALL helps with *:// in case (1) plus
chrome-extensions://abcdefb/ssh_client.nmf in case (2). It did not require
setting SCHEME_ALL prior to Parse() because the pattern had a wild card scheme.

I believe that if we had made the SSH pattern more strict like
"chrome-extension://*/*ssh_client.nmf instead of having a wild card at the
schema position, then the pattern matcher would not have reset to only allowing
http and https.

So, we could change the pattern and get rid of the second SetValidSchemes(). An
argument to keep the second one is if there is a chance that someone specifies a
filter pattern manually on the commandline and puts a *:// even though they knew
it only needed to match chrome-extension://.

As for why set it unconditionally to begin with, even when Parse() returns
false? I don't think that matters much since the pattern won't get added to the
debug_patterns_ vector and will be dropped.

     if (pattern.Parse(*iter) == URLPattern::PARSE_SUCCESS) {
       // If URL pattern has scheme equal to *, Parse method resets valid
       // schemes mask to http and https only, so we need to reset it after
-      // Parse to include chrome-extension scheme that can be used by NaCl
-      // manifest files.
+      // Parse to re-include chrome-extension and chrome schema.
       pattern.SetValidSchemes(URLPattern::SCHEME_ALL);
       debug_patterns_.push_back(pattern);
     }
   }
 }
 
 bool NaClBrowserDelegateImpl::URLMatchesDebugPatterns(
     const GURL& manifest_url) {
   // Empty patterns are forbidden so we ignore them.
   if (debug_patterns_.empty()) {
@@ skipping 72 matching lines @@
     return false;
 
   *file_path = resource_file_path;
   return true;
 }
 
 content::BrowserPpapiHost::OnKeepaliveCallback
 NaClBrowserDelegateImpl::GetOnKeepaliveCallback() {
   return base::Bind(&OnKeepalive);
 }