Closed Bug 875783 Opened 11 years ago Closed 5 years ago

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

Categories

(Core :: DOM: Core & HTML, defect, P5)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: alta88, Unassigned)

Details

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.
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.
(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.
(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...
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
Component: DOM → DOM: Core & HTML

Is this bug still a bug? I just tried the example in comment #1 and I didn't see any exception alerts...

(And I did a little more testing over in https://bugzilla.mozilla.org/show_bug.cgi?id=1569797#c20)

The example in comment 1 is not terribly related to the original bug, which is specifically about malformed file: URLs and specifically for XHR in a privileged context, as far as I can tell.

That said, current behavior of XHR in Firefox is as follows:

  1. If a string is passed to open() that can't be parsed into an nsIURI at all, open() throws. This is https://xhr.spec.whatwg.org/#the-open()-method step 7.
  2. If a security check fails during send() an exception will be thrown from send(). There isn't a corresponding spec item, because the spec doesn't really have a concept of loading security checks in this sense.... Per spec those should maybe become error responses, async.

If a security check fails during send() an exception will be thrown from send()

Er, only for sync XHR. For async we will catch that exception inside send() and convert it into an async error report, as observed in bug 1569797 comment 20.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.