422537 - XMLHttpRequest shouldn't use a pipe as an upload stream

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

bug 361934 made XMLHttpRequest use a pipe as the http channel's upload stream.

That has the major disadvantage that the stream is not seekable. That makes it impossible for an on-modify-request observer to read the stream without side-effects. I suspect an enabled Firebug will now break POST XMLHttpRequests, so requesting blocking.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

testcases from bug 395581 might come in handy to test this bug

Comment 2

[Johnny Stenback (:jst)]

Not going to block on this, but we'd definitely consider taking a fix for this if one shows up. Worst case Firebug simply needs to stop looking at XHR data until this does get fixed.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 308999 [details] [diff] [review]
patch

Good catch.  Thanks for fixing.

Add some tests?  Shouldn't be hard to mochitest, I would think....

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 308999 [details] [diff] [review]
patch

hm ok... I can look into adding tests tomorrow

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 308999 [details] [diff] [review]
patch

Clearing as biesi says he'll add tests, first.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: mochitests

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 308999 [details] [diff] [review]
patch

re-requesting approval, tests exist now

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

...but I'll comment out the variant part, because that currently fails. filed Bug 423219.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

or rather, I'll just remove that part. xpconnect doesn't really do what I expected there, and the "foo" array entry achieves the same purpose.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 308999 [details] [diff] [review]
patch

Yay, tests! a=beltzner

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch as checked in

Checking in content/base/src/nsXMLHttpRequest.cpp;
/cvsroot/mozilla/content/base/src/nsXMLHttpRequest.cpp,v  <--  nsXMLHttpRequest.cpp
new revision: 1.228; previous revision: 1.227
done
Checking in content/base/test/Makefile.in;
/cvsroot/mozilla/content/base/test/Makefile.in,v  <--  Makefile.in
new revision: 1.62; previous revision: 1.61
done
RCS file: /cvsroot/mozilla/content/base/test/test_bug422537.html,v
done
Checking in content/base/test/test_bug422537.html;
/cvsroot/mozilla/content/base/test/test_bug422537.html,v  <--  test_bug422537.html
initial revision: 1.1
done