sendBeacon should throw a TypeError on bad URLs

RESOLVED FIXED in Firefox 52

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: igrigorik, Assigned: wisniewskit)

Tracking

Trunk
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36




Expected results:

Discussion: https://github.com/w3c/web-platform-tests/pull/4024#discussion_r84792665
Spec update: https://github.com/w3c/beacon/commit/ff26009247cc3c76eb9c7b33cd9185082cff0a01

What is the expected output?

navigator.sendBeacon("bad://url") # should throw a TypeError, same as Fetch API.

Updated

3 years ago
Component: Untriaged → DOM
Product: Firefox → Core
Priority: -- → P3
Comment on attachment 8805740 [details] [diff] [review]
1313533-have_sendBeacon_throw_TypeErrors_on_bad_URIs.diff


>   nsCOMPtr<nsIURI> uri;
>   nsresult rv = nsContentUtils::NewURIWithDocumentCharset(
>                   getter_AddRefs(uri),
>                   aUrl,
>                   doc,
>                   doc->GetDocBaseURI());
>   if (NS_FAILED(rv)) {
>-    aRv.Throw(NS_ERROR_DOM_URL_MISMATCH_ERR);
>+    aRv.ThrowTypeError<MSG_INVALID_URL>(aUrl);
>     return false;
>   }
> 
>-  // Explicitly disallow loading data: URIs
>-  bool isDataScheme = false;
>-  rv = uri->SchemeIs("data", &isDataScheme);
>-  if (NS_FAILED(rv) || isDataScheme) {
>-    aRv.Throw(NS_ERROR_CONTENT_BLOCKED);
>+  // Spec disallows any schemes save for HTTP/HTTPs
>+  bool isValidScheme;
>+  if (!(NS_SUCCEEDED(uri->SchemeIs("http", &isValidScheme)) && isValidScheme) &&
>+      !(NS_SUCCEEDED(uri->SchemeIs("https", &isValidScheme)) && isValidScheme)) {
>+    aRv.ThrowTypeError<MSG_INVALID_URL_SCHEME>( NS_LITERAL_STRING("Beacon"), aUrl);
So we have http(s) check also below by using checking nsIHttpChannel. But I guess this is easier to understand, so ok.
Attachment #8805740 - Flags: review?(bugs) → review+
Assignee

Comment 3

3 years ago
Okay, thanks smaug. Requesting check-in.
Keywords: checkin-needed
Assignee: nobody → wisniewskit

Comment 4

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4079746a3f7
Have sendBeacon throw TypeErrors on invalid URLs to match a spec change. r=smaug
Keywords: checkin-needed

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a4079746a3f7
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.