Bryan Rockwood Reporter
	Description • 13 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0) Gecko/20100101 Firefox/6.0
Build ID: 20110811165603

Steps to reproduce:

Attempting to reuse a XMLHttpRequest object where the responseType is set to "arraybuffer".


Actual results:

The first request runs without issue.  Subsequent requests do not update the response property so the first requested array buffer is always returned.


Expected results:

Each new request should update the response property with the data retrieved from the request.  Verified this works in Chrome (v13.0.782.112 m) and in Safari (5.1).

	Bryan Rockwood Reporter
	Comment 4 • 13 years ago

Recreating the XMLHttpRequest object before the next request allows the script to retrieve all the data properly so there is a workaround.

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 13 years ago

Jonas, can you take a look into this, please?  This sounds pretty bad....  Possibly bad enough that we need to fix this on 7.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 6 • 13 years ago

We're nulling out mResultArrayBuffer in nsXMLHttpRequest::Abort, which is the obvious bug ...

Digging in here.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 7 • 13 years ago

We need to clear mResultArrayBuffer even if we're not aborting an existing XHR send call.

/me makes a note to investigate FileReader for a similar bug.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 8 • 13 years ago

Comment on attachment 555226 [details] [diff] [review]
Patch

we reset the other similar members here:

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsXMLHttpRequest.cpp#1728

Might be worth being consistent even though your patch releases the buffer slightly sooner.

(for what it's worth, state management in XHR is currently overly complicated. I'm slowly working on a set of patches to improve the situation).

r=me either way.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 9 • 13 years ago

For those playing along at home, FileReader does not have the same bug, because FileReader calls abort unconditionally.

I'll make the change suggested.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 10 • 13 years ago

http://hg.mozilla.org/mozilla-central/rev/bcadab745173
http://hg.mozilla.org/mozilla-central/rev/3a474d3aaa9a

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 12 • 13 years ago

Comment on attachment 555721 [details] [diff] [review]
Patch as landed

I'm kind of undecided on what branches we should fix this on, but it's a pretty trivial patch, and bz seems to think it's important.

	Christopher Blizzard (:blizzard)
	Comment 13 • 13 years ago

Comment on attachment 555721 [details] [diff] [review]
Patch as landed

Approved for Beta (Update 7) and Aurora (Update 8).  Please land as soon as possible.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 14 • 13 years ago

http://hg.mozilla.org/releases/mozilla-aurora/rev/a233176c3910
http://hg.mozilla.org/releases/mozilla-aurora/rev/82e79db5a5fc

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 15 • 13 years ago

http://hg.mozilla.org/releases/mozilla-beta/rev/ede728bc03d2
http://hg.mozilla.org/releases/mozilla-beta/rev/bf3712567861

	Ioana (away)
	Comment 16 • 13 years ago

Verified Fixed on:
->Windows 7 32-bit
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; rv:8.0a2) Gecko/20110825 Firefox/8.0a2
Mozilla/5.0 (Windows NT 6.1; rv:9.0a1) Gecko/20110825 Firefox/9.0a1
->Windows 7 64-bit         
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a2) Gecko/20110825 Firefox/8.0a2
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:9.0a1) Gecko/20110825 Firefox/9.0a1

I used the example from comment #1 to verify this fix:
 1. Open the link provided in comment #1.
 2. Tap no the "do request" button.
Two alerts with the same number are displayed.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 17 • 13 years ago

(In reply to Ioana Budnar from comment #16)
> Two alerts with the same number are displayed.

That's the *bug*.  The numbers are supposed to be different.

	Ioana (away)
	Comment 18 • 13 years ago

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #17)
> (In reply to Ioana Budnar from comment #16)
> > Two alerts with the same number are displayed.
> 
> That's the *bug*.  The numbers are supposed to be different.

I ran the steps from my previous comment on Chrome and Safari 5.1, on which the bug description says the page works fine. The same two alerts with the same number were given on these browsers.

In this case, could you please give me another test case and/or some STRs?

Thank you

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 19 • 13 years ago

I didn't test Safari, but this works in Chrome.  You have to download all three test files though.  For convenience, I've put these files up at http://people.mozilla.org/~khuey/bugs/xhrarraybufferreuse.html.  If you click on that, you should see a dialog with '99998' and another dialog with '1'.  The bug is seeing two dialogs with '99998'.  If you see two dialogs with '3290512384' that means that it didn't find the files.

	Ioana (away)
	Comment 20 • 13 years ago

I have verified this issue on the following builds:
->Windows 7 32-bit
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; rv:8.0a2) Gecko/20110825 Firefox/8.0a2
Mozilla/5.0 (Windows NT 6.1; rv:9.0a1) Gecko/20110825 Firefox/9.0a1
->Windows 7 64-bit         
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a2) Gecko/20110825 Firefox/8.0a2
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:9.0a1) Gecko/20110825 Firefox/9.0a1

The bug is still reproducing: two alerts with '99998' are displayed on all the builds. Should I reopen the bug in this case?

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 21 • 13 years ago

Testing builds made after the fix was checked in would help.

	Ioana (away)
	Comment 22 • 13 years ago

Verified fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0
Build ID: 20110830100616

STR: 
1. Open http://people.mozilla.org/~khuey/bugs/xhrarraybufferreuse.html.
2. Click the "do request" button.
Two alerts are displayed, the first one with "99998" and the second one with "1".

Will check on Fx8 and 9 when new builds appear.

	Ioana (away)
	Comment 23 • 13 years ago

Verified fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a2) Gecko/20110911 Firefox/8.0a2 
Build ID: 20110911042006
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:9.0a1) Gecko/20110911 Firefox/9.0a1
Build ID: 20110911030845

STR: 
1. Open http://people.mozilla.org/~khuey/bugs/xhrarraybufferreuse.html.
2. Click the "do request" button.
Two alerts are displayed, the first one with "99998" and the second one with "1".

	Alex Vincent [:WeirdAl]
	Comment 24 • 13 years ago

I think we may want to back this out - the XHR2 specification says this it should not work.

From http://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#the-open-method, step 11:

"Let async be the value of the async argument or true if it was omitted.

If async is false, there is an associated XMLHttpRequest document and either the timeout attribute value is not zero, the withCredentials attribute value is true, or the responseType attribute value is not the empty string, throw an "InvalidAccessError" exception and terminate these steps."

	Masatoshi Kimura [:emk]
	Comment 25 • 13 years ago

Why backout? It's perfectly valid reusing XHR object with async=true.
You should rather rewrite the test so that it makes sure that .responseType is not settable for sync XHR.

	Masatoshi Kimura [:emk]
	Comment 26 • 13 years ago

See this interdiff about my attemot in bug 701787 (and separated out to the other bug later).
https://bugzilla.mozilla.org/attachment.cgi?oldid=578886&action=interdiff&newid=578887&headers=1

	Boris Zbarsky [:bzbarsky]
	Comment 27 • 13 years ago

> I think we may want to back this out

That should be a separate bug then.  Having the code in the tree but this bug open is just confusing.

	Alex Vincent [:WeirdAl]
	Comment 28 • 13 years ago

Yeah, you're right.  Sorry for the bugspam, everyone.