XMLHttpRequest shouldn't use a pipe as an upload stream

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Biesinger, Assigned: Biesinger)

Tracking

Trunk
Points:
---
Bug Flags:
blocking1.9 -
wanted1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Flags: blocking1.9?
testcases from bug 395581 might come in handy to test this bug
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.
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Created attachment 308999 [details] [diff] [review]
patch
Attachment #308999 - Flags: superreview?(bzbarsky)
Attachment #308999 - Flags: review?(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....
Attachment #308999 - Flags: superreview?(bzbarsky)
Attachment #308999 - Flags: superreview+
Attachment #308999 - Flags: review?(bzbarsky)
Attachment #308999 - Flags: review+
Comment on attachment 308999 [details] [diff] [review]
patch

hm ok... I can look into adding tests tomorrow
Attachment #308999 - Flags: approval1.9?
Flags: in-testsuite?
Version: unspecified → Trunk
Comment on attachment 308999 [details] [diff] [review]
patch

Clearing as biesi says he'll add tests, first.
Attachment #308999 - Flags: approval1.9?
Comment on attachment 308999 [details] [diff] [review]
patch

re-requesting approval, tests exist now
Attachment #308999 - Flags: approval1.9?
...but I'll comment out the variant part, because that currently fails. filed Bug 423219.
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 on attachment 308999 [details] [diff] [review]
patch

Yay, tests! a=beltzner
Attachment #308999 - Flags: approval1.9? → approval1.9+
Created attachment 310312 [details] [diff] [review]
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
Attachment #308999 - Attachment is obsolete: true
Attachment #309700 - Attachment is obsolete: true
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Depends on: 483985
You need to log in before you can comment on or make changes to this bug.