Bug 1299373. Scripts with an empty src attribute should not execute. r=bkelly
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 07 Sep 2016 14:21:36 -0400
changeset 313070 8edc64f3a6e40c5f8aa67ed2255c49a4a1a2a386
parent 313069 ff96cb069ef2ca380d679b9b9a7dd2f7d4919d2e
child 313071 0e851843cf5a65f8570dbc3f99ab16243b2bb307
push id30669
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 00:56:12 +0000
treeherdermozilla-central@77940cbf0c2a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1299373
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1299373. Scripts with an empty src attribute should not execute. r=bkelly
dom/html/HTMLScriptElement.cpp
dom/svg/SVGScriptElement.cpp
testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty-with-base.html.ini
testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty.html.ini
testing/web-platform/meta/old-tests/submission/Opera/script_scheduling/137.html.ini
--- a/dom/html/HTMLScriptElement.cpp
+++ b/dom/html/HTMLScriptElement.cpp
@@ -134,16 +134,20 @@ void
 HTMLScriptElement::SetText(const nsAString& aValue, ErrorResult& rv)
 {
   rv = nsContentUtils::SetNodeTextContent(this, aValue, true);
 }
 
 
 NS_IMPL_STRING_ATTR(HTMLScriptElement, Charset, charset)
 NS_IMPL_BOOL_ATTR(HTMLScriptElement, Defer, defer)
+// If this ever gets changed to return "" if the attr value is "" (see
+// https://github.com/whatwg/html/issues/1739 for why it might not get changed),
+// it may be worth it to use GetSrc instead of GetAttr and manual
+// NewURIWithDocumentCharset in FreezeUriAsyncDefer.
 NS_IMPL_URI_ATTR(HTMLScriptElement, Src, src)
 NS_IMPL_STRING_ATTR(HTMLScriptElement, Type, type)
 NS_IMPL_STRING_ATTR(HTMLScriptElement, HtmlFor, _for)
 NS_IMPL_STRING_ATTR(HTMLScriptElement, Event, event)
 
 void
 HTMLScriptElement::SetCharset(const nsAString& aCharset, ErrorResult& rv)
 {
@@ -265,21 +269,27 @@ HTMLScriptElement::GetScriptCharset(nsAS
 void
 HTMLScriptElement::FreezeUriAsyncDefer()
 {
   if (mFrozen) {
     return;
   }
 
   // variation of this code in nsSVGScriptElement - check if changes
-  // need to be transfered when modifying
-  if (HasAttr(kNameSpaceID_None, nsGkAtoms::src)) {
-    nsAutoString src;
-    GetSrc(src);
-    NS_NewURI(getter_AddRefs(mUri), src);
+  // need to be transfered when modifying.  Note that we don't use GetSrc here
+  // because it will return the base URL when the attr value is "".
+  nsAutoString src;
+  if (GetAttr(kNameSpaceID_None, nsGkAtoms::src, src)) {
+    // Empty src should be treated as invalid URL.
+    if (!src.IsEmpty()) {
+      nsCOMPtr<nsIURI> baseURI = GetBaseURI();
+      nsContentUtils::NewURIWithDocumentCharset(getter_AddRefs(mUri),
+                                                src, OwnerDoc(), baseURI);
+    }
+
     // At this point mUri will be null for invalid URLs.
     mExternal = true;
 
     bool defer, async;
     GetAsync(&async);
     GetDefer(&defer);
 
     mDefer = !async && defer;
--- a/dom/svg/SVGScriptElement.cpp
+++ b/dom/svg/SVGScriptElement.cpp
@@ -149,18 +149,22 @@ SVGScriptElement::FreezeUriAsyncDefer()
     // need to be transfered when modifying
     nsAutoString src;
     if (mStringAttributes[HREF].IsExplicitlySet()) {
       mStringAttributes[HREF].GetAnimValue(src, this);
     } else {
       mStringAttributes[XLINK_HREF].GetAnimValue(src, this);
     }
 
-    nsCOMPtr<nsIURI> baseURI = GetBaseURI();
-    NS_NewURI(getter_AddRefs(mUri), src, nullptr, baseURI);
+    // Empty src should be treated as invalid URL.
+    if (!src.IsEmpty()) {
+      nsCOMPtr<nsIURI> baseURI = GetBaseURI();
+      NS_NewURI(getter_AddRefs(mUri), src, nullptr, baseURI);
+    }
+
     // At this point mUri will be null for invalid URLs.
     mExternal = true;
   }
 
   mFrozen = true;
 }
 
 //----------------------------------------------------------------------
deleted file mode 100644
--- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty-with-base.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[empty-with-base.html]
-  type: testharness
-  [Script src with an empty URL]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[empty.html]
-  type: testharness
-  [Script src with an empty URL]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/old-tests/submission/Opera/script_scheduling/137.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[137.html]
-  type: testharness
-  expected: ERROR