[Presentation API / Media Router] Relax PresentationRequest URL check.

chrome/common/media_router/media_source_helper.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/common/media_router/media_source_helper.h"
 
 #include <stdio.h>
 
+#include <algorithm>
+
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/common/media_router/media_source.h"
 #include "url/gurl.h"
 
 namespace media_router {
 
 namespace {
 
 // Prefixes used to format and detect various protocols' media source URNs.
 // See: https://www.ietf.org/rfc/rfc3406.txt
 constexpr char kTabMediaUrnFormat[] = "urn:x-org.chromium.media:source:tab:%d";
 constexpr char kDesktopMediaUrn[] = "urn:x-org.chromium.media:source:desktop";
 constexpr char kTabRemotingUrnFormat[] =
     "urn:x-org.chromium.media:source:tab_content_remoting:%d";
 constexpr char kCastPresentationUrlDomain[] = "google.com";
 constexpr char kCastPresentationUrlPath[] = "/cast";
 
 // This value must be the same as |chrome.cast.AUTO_JOIN_PRESENTATION_ID| in the
 // component extension.
 constexpr char kAutoJoinPresentationId[] = "auto-join";
 
+// List of non-http(s) schemes that are allowed in a Presentation URL.
+constexpr std::array<const char* const, 4> kAllowedSchemes{

[mark a. foltz, 2017/06/06 22:49:08]

Is std::array a type alias to POD or does it have a constructor?  If the latter,
it shouldn't have static lifetime.

Can this be constexpr char* kAllowedSchemes[kNumAllowedSchemes] = { ... }
instead?

[imcheng, 2017/06/07 00:53:43]

Yep, this is a POD.
https://stackoverflow.com/questions/3674247/is-stdarrayt-s-guaranteed-to-be-p...

This seems a bit nicer to use than C style array, since it comes with iterator
support (though not needed, strictly speaking...)

+    {"cast", "dial", "remote-playback", "test"}};
+
+bool IsUrlAllowed(const GURL& url) {

[mark a. foltz, 2017/06/06 22:49:08]

IsSchemeAllowed?

[imcheng, 2017/06/07 00:53:43]

Done.

+  return url.SchemeIsHTTPOrHTTPS() ||
+         std::any_of(

[mark a. foltz, 2017/06/06 22:49:08]

I think a for loop over a kAllowedSchemes array would involve less code overall
by not requiring a lambda definition (unless the compiler can optimize this into
equivalent code).  Not trying to over-optimize, but if there is a more compact
way to do the same thing...

[imcheng, 2017/06/07 00:53:43]

A decent compiler should be able to optimize this. The written code using
std::any_of is slightly more compact then using a for loop. Not a huge
difference either way.

+             kAllowedSchemes.begin(), kAllowedSchemes.end(),
+             [&url](const char* const scheme) { return url.SchemeIs(scheme); });
+}
+
 }  // namespace
 
 MediaSource MediaSourceForTab(int tab_id) {
   return MediaSource(base::StringPrintf(kTabMediaUrnFormat, tab_id));
 }
 
 MediaSource MediaSourceForTabContentRemoting(int tab_id) {
   return MediaSource(base::StringPrintf(kTabRemotingUrnFormat, tab_id));
 }
 
@@ skipping 39 matching lines @@
     return -1;
 }
 
 bool IsValidMediaSource(const MediaSource& source) {
   return TabIdFromMediaSource(source) > 0 ||
          IsDesktopMirroringMediaSource(source) ||
          IsValidPresentationUrl(GURL(source.id()));
 }
 
 bool IsValidPresentationUrl(const GURL& url) {
-  return url.is_valid() && url.SchemeIsHTTPOrHTTPS();
+  return url.is_valid() && IsUrlAllowed(url);
 }
 
 bool IsAutoJoinPresentationId(const std::string& presentation_id) {
   return presentation_id == kAutoJoinPresentationId;
 }
 
 }  // namespace media_router