bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

xhr.send() should not throw on an invalid url string

NEW
Unassigned

Status

()

Core
DOM
P5
normal
5 years ago
18 days ago

People

(Reporter: alta88, Unassigned)

Tracking

({dev-doc-needed})

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
Invalid urls should be flagged and sent to onerror().  Otherwise a client is forced to do a try catch newURI and QI to nsiIFile (if file: protocol is detected when applying a string to an nsIURI instance) to guarantee not crashing on a send().

Invalid (file) urls such as:
file:
file:/
file:foo
Is this actually specific to file:// URIs, and is it really a bug?

For example, this testcase:

<!DOCTYPE html>
<body>
  <script>
    var xhr = new XMLHttpRequest();
    var opened = false;
    try {
      xhr.open('GET', 'abracadabra:something');
      opened = true;
    } catch (e) {
      alert("Failed open: " + e);
    }
    if (opened) {
      try {
        xhr.send();
      } catch (e) {
        alert("Failed send: " + e);
      }
    }
</script>

throws on open() in Firefox and on send() in Chrome and Opera.

Which seems fine, when what you pass in isn't a url at all.
(Reporter)

Comment 2

5 years ago
I think the client legitimately should check for a valid protocol, for its own purposes.  So file would be a legitimate protocol whose url gets passed onto open/send, unlike abracadabra.  What does your test do with file:?

So the question is whether a non url should throw, or should all clients be forced to do the hack check in comment #1.  If you think the latter, just say it.

I do not think, once the protocol check passes (regardless of what it is), that a send() should throw on a non url.  I also don't think what Chrome/Opera do regarding api friendliness is important.

Btw, the fix in bug 282432, while an improvement in going to onerror(), has a status code of 0, readyState of 4 (completed), and no error text at all in the request.  That is just simply unfriendly to a caller concerned about UX and feedback.
> What does your test do with file:?

My test does not throw with file: or file:foo for me (on Mac).  It throws with file:/ because that fails a security check.  The same way that any file:// URI used in web page would throw, due to the same security check.
I have no idea what your hack check is talking about, since nsIURI and nsIFile are not exposed on the web.

> I also don't think what Chrome/Opera do regarding api friendliness is important.

Cross-browser compat on web-exposed behavior is in fact important.
(Reporter)

Comment 5

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #3)
> > What does your test do with file:?
> 
> My test does not throw with file: or file:foo for me (on Mac).  It throws
> with file:/ because that fails a security check.  The same way that any
> file:// URI used in web page would throw, due to the same security check.

Checking on a mac is good.  Also checking an os specific thing like file names on windows is better.
(Reporter)

Comment 6

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #4)
> I have no idea what your hack check is talking about, since nsIURI and
> nsIFile are not exposed on the web.
> 
> > I also don't think what Chrome/Opera do regarding api friendliness is important.
> 
> Cross-browser compat on web-exposed behavior is in fact important.

This is from chrome, not web.
> This is from chrome, not web.

Why should chrome have different behavior than the web?  It shouldn't.

If this is only about XHR from chrome, I frankly don't care about it...

Updated

5 years ago
Keywords: dev-doc-needed
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven't been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.