Bug 1509101. Stop passing around anchor targets as char16_t*; use nsAString instead. r=baku
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 21 Nov 2018 18:53:39 +0000
changeset 447613 4563b979582b601a929eed63ef4e6162973e5c73
parent 447612 834b61114235c7a2f2413d135ad11e5b4148c09b
child 447614 6dc7299787eaa33e81f401d4ef9e8593922ed539
push id35084
push userebalazs@mozilla.com
push dateThu, 22 Nov 2018 09:19:57 +0000
treeherdermozilla-central@111154a7621c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1509101
milestone65.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 1509101. Stop passing around anchor targets as char16_t*; use nsAString instead. r=baku Differential Revision: https://phabricator.services.mozilla.com/D12579
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
docshell/base/nsILinkHandler.h
dom/base/nsContentUtils.cpp
dom/html/HTMLFormElement.cpp
dom/plugins/base/nsPluginInstanceOwner.cpp
testing/web-platform/tests/html/browsers/windows/resources/message-parent.html
testing/web-platform/tests/html/browsers/windows/targeting-with-embedded-null-in-target.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -13066,17 +13066,17 @@ nsDocShell::EnsureCommandHandler()
 
 // link handling
 
 class OnLinkClickEvent : public Runnable
 {
 public:
   OnLinkClickEvent(nsDocShell* aHandler, nsIContent* aContent,
                    nsIURI* aURI,
-                   const char16_t* aTargetSpec,
+                   const nsAString& aTargetSpec,
                    const nsAString& aFileName,
                    nsIInputStream* aPostDataStream,
                    nsIInputStream* aHeadersDataStream,
                    bool aNoOpenerImplied,
                    bool aIsUserTriggered,
                    bool aIsTrusted,
                    nsIPrincipal* aTriggeringPrincipal);
 
@@ -13087,17 +13087,17 @@ public:
     // We need to set up an AutoJSAPI here for the following reason: When we do
     // OnLinkClickSync we'll eventually end up in nsGlobalWindow::OpenInternal
     // which only does popup blocking if !LegacyIsCallerChromeOrNativeCode().
     // So we need to fake things so that we don't look like native code as far
     // as LegacyIsCallerNativeCode() is concerned.
     AutoJSAPI jsapi;
     if (mIsTrusted || jsapi.Init(mContent->OwnerDoc()->GetScopeObject())) {
       mHandler->OnLinkClickSync(mContent, mURI,
-                                mTargetSpec.get(), mFileName,
+                                mTargetSpec, mFileName,
                                 mPostDataStream,
                                 mHeadersDataStream, mNoOpenerImplied,
                                 nullptr, nullptr, mIsUserTriggered,
                                 mTriggeringPrincipal);
     }
     return NS_OK;
   }
 
@@ -13114,17 +13114,17 @@ private:
   bool mIsUserTriggered;
   bool mIsTrusted;
   nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
 };
 
 OnLinkClickEvent::OnLinkClickEvent(nsDocShell* aHandler,
                                    nsIContent* aContent,
                                    nsIURI* aURI,
-                                   const char16_t* aTargetSpec,
+                                   const nsAString& aTargetSpec,
                                    const nsAString& aFileName,
                                    nsIInputStream* aPostDataStream,
                                    nsIInputStream* aHeadersDataStream,
                                    bool aNoOpenerImplied,
                                    bool aIsUserTriggered,
                                    bool aIsTrusted,
                                    nsIPrincipal* aTriggeringPrincipal)
   : mozilla::Runnable("OnLinkClickEvent")
@@ -13141,17 +13141,17 @@ OnLinkClickEvent::OnLinkClickEvent(nsDoc
   , mIsTrusted(aIsTrusted)
   , mTriggeringPrincipal(aTriggeringPrincipal)
 {
 }
 
 NS_IMETHODIMP
 nsDocShell::OnLinkClick(nsIContent* aContent,
                         nsIURI* aURI,
-                        const char16_t* aTargetSpec,
+                        const nsAString& aTargetSpec,
                         const nsAString& aFileName,
                         nsIInputStream* aPostDataStream,
                         nsIInputStream* aHeadersDataStream,
                         bool aIsUserTriggered,
                         bool aIsTrusted,
                         nsIPrincipal* aTriggeringPrincipal)
 {
 #ifndef ANDROID
@@ -13178,47 +13178,46 @@ nsDocShell::OnLinkClick(nsIContent* aCon
   }
 
   nsresult rv = NS_ERROR_FAILURE;
   nsAutoString target;
 
   nsCOMPtr<nsIWebBrowserChrome3> browserChrome3 = do_GetInterface(mTreeOwner);
   bool noOpenerImplied = false;
   if (browserChrome3) {
-    nsAutoString oldTarget(aTargetSpec);
-    rv = browserChrome3->OnBeforeLinkTraversal(oldTarget, aURI,
+    rv = browserChrome3->OnBeforeLinkTraversal(aTargetSpec, aURI,
                                                aContent, mIsAppTab, target);
-    if (!oldTarget.Equals(target)) {
+    if (!aTargetSpec.Equals(target)) {
       noOpenerImplied = true;
     }
   }
 
   if (NS_FAILED(rv)) {
     target = aTargetSpec;
   }
 
   nsCOMPtr<nsIRunnable> ev =
-    new OnLinkClickEvent(this, aContent, aURI, target.get(), aFileName,
+    new OnLinkClickEvent(this, aContent, aURI, target, aFileName,
                          aPostDataStream, aHeadersDataStream, noOpenerImplied,
                          aIsUserTriggered, aIsTrusted, aTriggeringPrincipal);
   return DispatchToTabGroup(TaskCategory::UI, ev.forget());
 }
 
 static bool
 IsElementAnchorOrArea(nsIContent* aContent)
 {
   // Make sure we are dealing with either an <A> or <AREA> element in the HTML
   // or XHTML namespace.
   return aContent->IsAnyOfHTMLElements(nsGkAtoms::a, nsGkAtoms::area);
 }
 
 NS_IMETHODIMP
 nsDocShell::OnLinkClickSync(nsIContent* aContent,
                             nsIURI* aURI,
-                            const char16_t* aTargetSpec,
+                            const nsAString& aTargetSpec,
                             const nsAString& aFileName,
                             nsIInputStream* aPostDataStream,
                             nsIInputStream* aHeadersDataStream,
                             bool aNoOpenerImplied,
                             nsIDocShell** aDocShell,
                             nsIRequest** aRequest,
                             bool aIsUserTriggered,
                             nsIPrincipal* aTriggeringPrincipal)
@@ -13321,18 +13320,16 @@ nsDocShell::OnLinkClickSync(nsIContent* 
     if (refPolEnum != RP_Unset) {
       refererPolicy = refPolEnum;
     }
   }
 
   // referer could be null here in some odd cases, but that's ok,
   // we'll just load the link w/o sending a referer in those cases.
 
-  nsAutoString target(aTargetSpec);
-
   // If this is an anchor element, grab its type property to use as a hint
   nsAutoString typeHint;
   RefPtr<HTMLAnchorElement> anchor = HTMLAnchorElement::FromNode(aContent);
   if (anchor) {
     anchor->GetType(typeHint);
     NS_ConvertUTF16toUTF8 utf8Hint(typeHint);
     nsAutoCString type, dummy;
     NS_ParseRequestContentType(utf8Hint, type, dummy);
@@ -13361,17 +13358,17 @@ nsDocShell::OnLinkClickSync(nsIContent* 
                              false,
                              false,                     // LoadReplace
                              false,                     // IsFromProcessingFrameAttributes
                              referer,                   // Referer URI
                              refererPolicy,             // Referer policy
                              triggeringPrincipal,
                              aContent->NodePrincipal(),
                              flags,
-                             target,                    // Window target
+                             aTargetSpec,               // Window target
                              NS_LossyConvertUTF16toASCII(typeHint),
                              aFileName,                 // Download as file
                              aPostDataStream,           // Post data stream
                              aHeadersDataStream,        // Headers stream
                              loadType,                  // Load type
                              nullptr,                   // No SHEntry
                              true,                      // first party site
                              VoidString(),              // No srcdoc
@@ -13383,17 +13380,17 @@ nsDocShell::OnLinkClickSync(nsIContent* 
     nsPingListener::DispatchPings(this, aContent, aURI, referer, refererPolicy);
   }
   return rv;
 }
 
 NS_IMETHODIMP
 nsDocShell::OnOverLink(nsIContent* aContent,
                        nsIURI* aURI,
-                       const char16_t* aTargetSpec)
+                       const nsAString& aTargetSpec)
 {
   if (aContent->IsEditable()) {
     return NS_OK;
   }
 
   nsCOMPtr<nsIWebBrowserChrome2> browserChrome2 = do_GetInterface(mTreeOwner);
   nsresult rv = NS_ERROR_FAILURE;
 
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -191,37 +191,37 @@ public:
     // Need this here because otherwise nsIWebNavigation::Stop
     // overrides the docloader's Stop()
     return nsDocLoader::Stop();
   }
 
   // nsILinkHandler
   NS_IMETHOD OnLinkClick(nsIContent* aContent,
                          nsIURI* aURI,
-                         const char16_t* aTargetSpec,
+                         const nsAString& aTargetSpec,
                          const nsAString& aFileName,
                          nsIInputStream* aPostDataStream,
                          nsIInputStream* aHeadersDataStream,
                          bool aIsUserTriggered,
                          bool aIsTrusted,
                          nsIPrincipal* aTriggeringPrincipal) override;
   NS_IMETHOD OnLinkClickSync(nsIContent* aContent,
                              nsIURI* aURI,
-                             const char16_t* aTargetSpec,
+                             const nsAString& aTargetSpec,
                              const nsAString& aFileName,
                              nsIInputStream* aPostDataStream = 0,
                              nsIInputStream* aHeadersDataStream = 0,
                              bool aNoOpenerImplied = false,
                              nsIDocShell** aDocShell = 0,
                              nsIRequest** aRequest = 0,
                              bool aIsUserTriggered = false,
                              nsIPrincipal* aTriggeringPrincipal = nullptr) override;
   NS_IMETHOD OnOverLink(nsIContent* aContent,
                         nsIURI* aURI,
-                        const char16_t* aTargetSpec) override;
+                        const nsAString& aTargetSpec) override;
   NS_IMETHOD OnLeaveLink() override;
 
   // Don't use NS_DECL_NSILOADCONTEXT because some of nsILoadContext's methods
   // are shared with nsIDocShell (appID, etc.) and can't be declared twice.
   NS_IMETHOD GetAssociatedWindow(mozIDOMWindowProxy**) override;
   NS_IMETHOD GetTopWindow(mozIDOMWindowProxy**) override;
   NS_IMETHOD GetTopFrameElement(mozilla::dom::Element**) override;
   NS_IMETHOD GetNestedFrameId(uint64_t*) override;
--- a/docshell/base/nsILinkHandler.h
+++ b/docshell/base/nsILinkHandler.h
@@ -37,17 +37,17 @@ public:
    * @param aPostDataStream the POST data to send
    * @param aHeadersDataStream ???
    * @param aIsTrusted false if the triggerer is an untrusted DOM event.
    * @param aTriggeringPrincipal, if not passed explicitly we fall back to
    *        the document's principal.
    */
   NS_IMETHOD OnLinkClick(nsIContent* aContent,
                          nsIURI* aURI,
-                         const char16_t* aTargetSpec,
+                         const nsAString& aTargetSpec,
                          const nsAString& aFileName,
                          nsIInputStream* aPostDataStream,
                          nsIInputStream* aHeadersDataStream,
                          bool aIsUserTriggered,
                          bool aIsTrusted,
                          nsIPrincipal* aTriggeringPrincipal) = 0;
 
   /**
@@ -66,17 +66,17 @@ public:
    * @param aNoOpenerImplied if the link implies "noopener"
    * @param aDocShell (out-param) the DocShell that the request was opened on
    * @param aRequest the request that was opened
    * @param aTriggeringPrincipal, if not passed explicitly we fall back to
    *        the document's principal.
    */
   NS_IMETHOD OnLinkClickSync(nsIContent* aContent,
                              nsIURI* aURI,
-                             const char16_t* aTargetSpec,
+                             const nsAString& aTargetSpec,
                              const nsAString& aFileName,
                              nsIInputStream* aPostDataStream = 0,
                              nsIInputStream* aHeadersDataStream = 0,
                              bool aNoOpenerImplied = false,
                              nsIDocShell** aDocShell = 0,
                              nsIRequest** aRequest = 0,
                              bool aIsUserTriggered = false,
                              nsIPrincipal* aTriggeringPrincipal = nullptr) = 0;
@@ -86,17 +86,17 @@ public:
    *
    * @param aContent the linked content.
    * @param aURI an URI object that defines the destination for the link
    * @param aTargetSpec indicates where the link is targeted (it may be an empty
    *        string)
    */
   NS_IMETHOD OnOverLink(nsIContent* aContent,
                         nsIURI* aURLSpec,
-                        const char16_t* aTargetSpec) = 0;
+                        const nsAString& aTargetSpec) = 0;
 
   /**
    * Process the mouse leaving a link.
    */
   NS_IMETHOD OnLeaveLink() = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsILinkHandler, NS_ILINKHANDLER_IID)
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -5538,17 +5538,17 @@ nsContentUtils::TriggerLink(nsIContent *
   }
 
   nsILinkHandler *handler = aPresContext->GetLinkHandler();
   if (!handler) {
     return;
   }
 
   if (!aClick) {
-    handler->OnOverLink(aContent, aLinkURI, aTargetSpec.get());
+    handler->OnOverLink(aContent, aLinkURI, aTargetSpec);
     return;
   }
 
   // Check that this page is allowed to load this URI.
   nsresult proceed = NS_OK;
 
   if (sSecurityManager) {
     uint32_t flag = static_cast<uint32_t>(nsIScriptSecurityManager::STANDARD);
@@ -5571,17 +5571,17 @@ nsContentUtils::TriggerLink(nsIContent *
          !aContent->IsHTMLElement(nsGkAtoms::area) &&
          !aContent->IsSVGElement(nsGkAtoms::a)) ||
         !aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::download, fileName) ||
         NS_FAILED(aContent->NodePrincipal()->CheckMayLoad(aLinkURI, false, true))) {
       fileName.SetIsVoid(true); // No actionable download attribute was found.
     }
 
     handler->OnLinkClick(aContent, aLinkURI,
-                         fileName.IsVoid() ? aTargetSpec.get() : EmptyString().get(),
+                         fileName.IsVoid() ? aTargetSpec : EmptyString(),
                          fileName, nullptr, nullptr, EventStateManager::IsHandlingUserInput(),
                          aIsTrusted, aContent->NodePrincipal());
   }
 }
 
 /* static */
 void
 nsContentUtils::GetLinkLocation(Element* aElement, nsString& aLocationString)
--- a/dom/html/HTMLFormElement.cpp
+++ b/dom/html/HTMLFormElement.cpp
@@ -746,17 +746,17 @@ HTMLFormElement::SubmitSubmission(HTMLFo
     rv = aFormSubmission->GetEncodedSubmission(actionURI,
                                                getter_AddRefs(postDataStream),
                                                actionURI);
     NS_ENSURE_SUBMIT_SUCCESS(rv);
 
     nsAutoString target;
     aFormSubmission->GetTarget(target);
     rv = linkHandler->OnLinkClickSync(this, actionURI,
-                                      target.get(),
+                                      target,
                                       VoidString(),
                                       postDataStream,
                                       nullptr, false,
                                       getter_AddRefs(docShell),
                                       getter_AddRefs(mSubmittingRequest),
                                       EventStateManager::IsHandlingUserInput());
     NS_ENSURE_SUBMIT_SUCCESS(rv);
   }
--- a/dom/plugins/base/nsPluginInstanceOwner.cpp
+++ b/dom/plugins/base/nsPluginInstanceOwner.cpp
@@ -479,17 +479,17 @@ NS_IMETHODIMP nsPluginInstanceOwner::Get
   if (!aDoCheckLoadURIChecks) {
     mozilla::OriginAttributes attrs =
       BasePrincipal::Cast(content->NodePrincipal())->OriginAttributesRef();
     triggeringPrincipal = BasePrincipal::CreateCodebasePrincipal(uri, attrs);
   } else {
     triggeringPrincipal = NullPrincipal::CreateWithInheritedAttributes(content->NodePrincipal());
   }
 
-  rv = lh->OnLinkClick(content, uri, unitarget.get(), VoidString(),
+  rv = lh->OnLinkClick(content, uri, unitarget, VoidString(),
                        aPostStream, headersDataStream,
                        /* isUserTriggered */ false,
                        /* isTrusted */ true, triggeringPrincipal);
 
   return rv;
 }
 
 NS_IMETHODIMP nsPluginInstanceOwner::GetDocument(nsIDocument* *aDocument)
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/browsers/windows/resources/message-parent.html
@@ -0,0 +1,5 @@
+<script>
+  window.parent.postMessage({
+      "name": window.name,
+  }, "*");
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/browsers/windows/targeting-with-embedded-null-in-target.html
@@ -0,0 +1,34 @@
+<!doctype html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Targeting with embedded null in target</title>
+  <script src="/resources/testharness.js"></script>
+  <script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+  <iframe name="abc">
+  </iframe>
+  <a href="resources/message-parent.html" target="abc">Click me</a>
+  <script>
+    var t = async_test();
+    var target_name = "abc\u0000def";
+
+    onmessage = t.step_func_done(function (e) {
+        assert_equals(e.data.name, target_name,
+                      "Should come from a window with the right name");
+        assert_equals(e.source, frames[1],
+                      "Should come frome the right window");
+    });
+
+    t.step(function() {
+        var iframe = document.createElement("iframe");
+        iframe.setAttribute("name", target_name);
+        document.body.appendChild(iframe);
+        var a = document.querySelector("a");
+        a.target = target_name;
+        a.click();
+    });
+  </script>
+</body>
+</html>