Badging: Throw TypeError if the scope URL parses invalid.

Bug: 1001405
Change-Id: I918c57d6922915e624b731432b393311d8ab1a3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1802780
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696721}

third_party/blink/renderer/modules/badging/badge.cc

@@ -37,7 +37,8 @@ void Badge::set(ScriptState* script_state,
                 const BadgeOptions* options,
                 ExceptionState& exception_state) {
   BadgeFromState(script_state)
-      ->SetBadge(options->scope(), mojom::blink::BadgeValue::NewFlag(0));
+      ->SetBadge(options->scope(), mojom::blink::BadgeValue::NewFlag(0),
+                 exception_state);
 }
 
 // static
@@ -53,28 +54,38 @@ void Badge::set(ScriptState* script_state,
                 const BadgeOptions* options,
                 ExceptionState& exception_state) {
   if (content == 0) {
-    Badge::clear(script_state, options);
+    Badge::clear(script_state, options, exception_state);
   } else {
     BadgeFromState(script_state)
         ->SetBadge(options->scope(),
-                   mojom::blink::BadgeValue::NewNumber(content));
+                   mojom::blink::BadgeValue::NewNumber(content),
+                   exception_state);
   }
 }
 
 // static
-void Badge::clear(ScriptState* script_state, const BadgeOptions* options) {
-  BadgeFromState(script_state)->ClearBadge(options->scope());
+void Badge::clear(ScriptState* script_state,
+                  const BadgeOptions* options,
+                  ExceptionState& exception_state) {
+  BadgeFromState(script_state)->ClearBadge(options->scope(), exception_state);
 }
 
-void Badge::SetBadge(WTF::String scope, mojom::blink::BadgeValuePtr value) {
-  // Resolve |scope| against the URL of the current document/worker.
-  KURL scope_url = KURL(execution_context_->Url(), scope);
-  badge_service_->SetBadge(scope_url, std::move(value));
+void Badge::SetBadge(WTF::String scope,
+                     mojom::blink::BadgeValuePtr value,
+                     ExceptionState& exception_state) {
+  base::Optional<KURL> scope_url = ScopeStringToURL(scope, exception_state);
+  if (!scope_url)
+    return;
+
+  badge_service_->SetBadge(*scope_url, std::move(value));
 }
 
-void Badge::ClearBadge(WTF::String scope) {
-  // Resolve |scope| against the URL of the current document/worker.
-  badge_service_->ClearBadge(KURL(execution_context_->Url(), scope));
+void Badge::ClearBadge(WTF::String scope, ExceptionState& exception_state) {
+  base::Optional<KURL> scope_url = ScopeStringToURL(scope, exception_state);
+  if (!scope_url)
+    return;
+
+  badge_service_->ClearBadge(*scope_url);
 }
 
 void Badge::Trace(blink::Visitor* visitor) {
@@ -95,4 +106,22 @@ Badge* Badge::BadgeFromState(ScriptState* script_state) {
   return Badge::From(ExecutionContext::From(script_state));
 }
 
+base::Optional<KURL> Badge::ScopeStringToURL(WTF::String& scope,
+                                             ExceptionState& exception_state) {
+  // Resolve |scope| against the URL of the current document/worker.
+  KURL scope_url = KURL(execution_context_->Url(), scope);
+
+  if (!scope_url.IsValid()) {
+    exception_state.ThrowTypeError("Invalid scope URL");
+    return base::nullopt;
+  }
+
+  // TODO(mgiuca): Check that URL is same-origin as the execution context. If
+  // not, fail with SecurityError (https://crbug.com/1001404). (This is not a
+  // security bug, since the same-origin check is currently done on the browser
+  // side, but we still want to report the failure as an exception.)
+
+  return base::Optional<KURL>(scope_url);
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/badging/badge.h

@@ -5,10 +5,12 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_BADGING_BADGE_H_
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_BADGING_BADGE_H_
 
+#include "base/optional.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/badging/badging.mojom-blink.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
 #include "third_party/blink/renderer/platform/supplementable.h"
+#include "third_party/blink/renderer/platform/weborigin/kurl.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
@@ -39,16 +41,22 @@ class Badge final : public ScriptWrappable,
                   ExceptionState&);
   static void set(ScriptState*, ExceptionState&);
   static void set(ScriptState*, uint64_t content, ExceptionState&);
-  static void clear(ScriptState*, const BadgeOptions*);
+  static void clear(ScriptState*, const BadgeOptions*, ExceptionState&);
 
-  void SetBadge(WTF::String scope, mojom::blink::BadgeValuePtr value);
-  void ClearBadge(WTF::String scope);
+  void SetBadge(WTF::String scope,
+                mojom::blink::BadgeValuePtr value,
+                ExceptionState&);
+  void ClearBadge(WTF::String scope, ExceptionState&);
 
   void Trace(blink::Visitor*) override;
 
  private:
   static Badge* BadgeFromState(ScriptState* script_state);
 
+  // If the URL is invalid, sets an exception and returns nullopt, which callers
+  // should check for and stop doing work.
+  base::Optional<KURL> ScopeStringToURL(WTF::String& scope, ExceptionState&);
+
   mojo::Remote<blink::mojom::blink::BadgeService> badge_service_;
   Member<ExecutionContext> execution_context_;
 };

third_party/blink/renderer/modules/badging/experimental_badge.idl

@@ -13,5 +13,5 @@
 ] interface ExperimentalBadge {
   [CallWith=ScriptState, RaisesException, MeasureAs=BadgeSet]
   static void set(optional [EnforceRange] unsigned long long contents, optional BadgeOptions options);
-  [CallWith=ScriptState, MeasureAs=BadgeClear] static void clear(optional BadgeOptions options);
+  [CallWith=ScriptState, RaisesException, MeasureAs=BadgeClear] static void clear(optional BadgeOptions options);
 };

third_party/blink/web_tests/badging/badge-error.html

@@ -41,10 +41,9 @@ badge_test(() => { ExperimentalBadge.set({}); }, undefined, undefined,
 // }, undefined, undefined, 'SecurityError');
 
 // Invalid URL.
-// TODO(mgiuca): Currently succeeds with scope = "". https://crbug.com/1001405
-// badge_test(() => {
-//   ExperimentalBadge.set(1, {scope: 'https://example.com:99999'});
-// }, undefined, undefined, 'TypeError');
+badge_test(() => {
+  ExperimentalBadge.set(1, {scope: 'https://example.com:99999'});
+}, undefined, undefined, 'TypeError');
 
 </script>
 </body>