Throw SyntaxError for non-turn/turns/stun URLs

third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 288 matching lines @@
       } else {
         exceptionState.throwTypeError("Malformed RTCIceServer");
         return WebRTCConfiguration();
       }
 
       String username = iceServer.username();
       String credential = iceServer.credential();
 
       for (const String& urlString : urlStrings) {
         KURL url(KURL(), urlString);
         if (!url.isValid() ||

[hta - Chromium, 2016/10/25 13:51:01]

Having to call back my LGTM: You have to throw a different message for the
"isValid = false" case, I think. Otherwise the error generated on
"turn:groovy_illegal!character" will be misleading.

[foolip, 2016/10/27 13:32:41]

exceptionState.throwDOMException(SyntaxError, "'" + url + "' is not a valid
URL.") is used in a bunch of places, so I think I'll copy that.

But I can't find a string starting with "turn:" that KURL considers invalid.
What kind of groovy characters should I try? I've tried ! [ :

[hta - Chromium, 2016/11/08 12:12:30]

:-) on the error generating problem - typical "syntax too permissive, please try
again"!

Query: Does turn:%GG (illegal hex escape) give !isValid?

[foolip, 2016/11/08 15:39:07]

Nope, that's still considered valid.

             !(url.protocolIs("turn") || url.protocolIs("turns") ||
               url.protocolIs("stun"))) {
-          exceptionState.throwTypeError("Malformed URL");
+          exceptionState.throwDOMException(
+              SyntaxError,
+              "Unsupported URL \"" + urlString +
+                  "\". URL scheme must be \"stun\", \"turn\" or \"turns\".");

[hta - Chromium, 2016/10/25 13:43:43]

Hyper-nit: Should one use a StringBuilder instead of string concatenation here,
just to be style-consistent?

[foolip, 2016/10/27 13:32:41]

Some places use String::format(), and a bunch use the "'" + url + "..." that
I've copied. Happy to convert to any form, I guess whatever produces a smaller
binary is better given that perf doesn't matter here.

           return WebRTCConfiguration();
         }
         iceServers.append(WebRTCIceServer{url, username, credential});
       }
     }
     webConfiguration.iceServers = iceServers;
   }
 
   if (configuration.hasCertificates()) {
     const HeapVector<Member<RTCCertificate>>& certificates =
@@ skipping 1142 matching lines @@
 DEFINE_TRACE(RTCPeerConnection) {
   visitor->trace(m_localStreams);
   visitor->trace(m_remoteStreams);
   visitor->trace(m_dispatchScheduledEventRunner);
   visitor->trace(m_scheduledEvents);
   EventTargetWithInlineData::trace(visitor);
   ActiveDOMObject::trace(visitor);
 }
 
 }  // namespace blink