Please support ArrayBufferView instances as parameters to XMLHttpRequest.send()

RESOLVED FIXED in mozilla20

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Victor Costan, Assigned: bz)

Tracking

17 Branch
mozilla20
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.21 (KHTML, like Gecko) Chrome/25.0.1349.2 Safari/537.21

Steps to reproduce:

arrayBufferView = new Uint8Array(32);
var xhr = new XMLHttpRequest();
xhr.open('http://test_backend.local', 'POST', false);
xhr.send(arrayBufferView);



Actual results:

[Exception... "Illegal value"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: Web Console :: <TOP_LEVEL> :: line 3"  data: no]


Expected results:

The send() call should go through.

Comment 1

5 years ago
The problem isn't the send call; the method and target of the open call are reversed. It should be |xhr.open('POST', 'http://test_backend.local', false);|
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → INVALID
The test code given is buggy, but we do not in fact support passing in an ArrayBufferView in a useful way, and it looks like this has been added to the XHR2 spec.

Fix coming up.  send() of a DataView from a worker won't work until bug 819838, I suspect, but the rest should work.  Hard to tell, since I can't find existing worker tests for sending things other than null and a string.
Assignee: nobody → bzbarsky
Status: RESOLVED → UNCONFIRMED
Component: General → DOM
Resolution: INVALID → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
Created attachment 690270 [details] [diff] [review]
Add support for passing an ArrayBufferView to XHR.send().

Ben, please let me know if there are existing worker tests for sending non-strings that I just totally missed that I should add this to
Attachment #690270 - Flags: review?(jonas)
Attachment #690270 - Flags: review?(bent.mozilla)
Whiteboard: [need review]
Comment on attachment 690270 [details] [diff] [review]
Add support for passing an ArrayBufferView to XHR.send().

Review of attachment 690270 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. We don't really have good xhr-post tests for workers, unfortunately. Some day we need to run the main thread xhr tests in a worker.
Attachment #690270 - Flags: review?(bent.mozilla) → review+
Comment on attachment 690270 [details] [diff] [review]
Add support for passing an ArrayBufferView to XHR.send().

Review of attachment 690270 [details] [diff] [review]:
-----------------------------------------------------------------

r=me assuming the answer to the below question is "bytes" :)

::: content/base/src/nsXMLHttpRequest.cpp
@@ +2668,5 @@
> +    }
> +    case nsXMLHttpRequest::RequestBody::ArrayBufferView:
> +    {
> +      return ::GetRequestBody(value.mArrayBufferView->Data(),
> +                              value.mArrayBufferView->Length(), aResult,

Does Length() return the size in bytes or in units?
Attachment #690270 - Flags: review?(jonas) → review+
> Does Length() return the size in bytes or in units?

For a dom::ArrayBufferView, in bytes.

In general, Length() on the DOM typed array types returns the length in sizeof(*Data()), and Data() for an ArrayBufferView returns uint8_t*, for lack of anything better to do.
(Reporter)

Comment 7

5 years ago
bz: thank you very much, and apologies for the bad test case!
https://hg.mozilla.org/integration/mozilla-inbound/rev/42213ca59024

Victor, you're welcome, and thank you for filing the bug!
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla20

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/42213ca59024
Status: NEW → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.