343971 - XMLHTTPRequest post fails in file upload if given a stream that does not implement ReadSegments

Status: RESOLVED WORKSFORME

(Core :: Networking, defect, P3)

Description

[/\/\arcio Galli]

The above testcase is not too simplified yet, but it shows probably a _regression_ with POST/XMLHTTPRequest between 06-26 and 06-28 builds. 

Testcase note: 
---
* Set your signed.applets.code_principal_support to true
* Expect an instant "hello world:" message when it fails. 
* Expect a delayed ( upload time ) "hello world:" + valid error XML message when it wors.

Comment 1

[/\/\arcio Galli]

Test summary: 
--
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060628 BonEcho/2.0a3

Fails. After I pick an image for upload ( a PNG was used ) it fails, req.onreadystatechange is called quickly with a readyState = 4 but with req.responseText empty. 

Comment 2

[/\/\arcio Galli]

Test summary: version that works ( 06/26 )
---
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060626 BonEcho/2.0a3

Works. After I pick an image for upload ( PNG used ), req.onreadystatechange is called with a readyState = 4 and qeq.responseText shows. Yes, it works when it shows an XML error message from Flickr.com. 

Comment 3

[Ria Klaassen (not reading all bugmail)]

In trunk that would mean: between 1.9a1_2006062214 and 1.9a1_2006062215 (ID's)

Comment 4

[Ria Klaassen (not reading all bugmail)]

Bug 334142 seems a possible culprit.

Comment 5

[/\/\arcio Galli]

Trace from version with problem. You may notice the image bits are not attached with the post. 
-
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060711 BonEcho/2.0b1

=====

POST /services/upload/ HTTP/1.1
Host: www.flickr.com
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060711 BonEcho/2.0b1
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive
Content-Type: multipart/form-data; boundary=123456789
Content-Length: 2227
Pragma: no-cache
Cache-Control: no-cache

--123456789
Content-Disposition: form-data; name="api_key"

some_random_value
--123456789
Content-Disposition: form-data; name="auth_token"

some_random_value
--123456789
Content-Disposition: form-data; name="api_sig"

b06ab2cf90144e6ca739db849092474e
--123456789
Content-Disposition: form-data; name="photo"; filename="minimo.png"
Content-Type: image/png

HTTP/1.1 200 OK
Date: Fri, 14 Jul 2006 17:43:07 GMT
Server: Apache/2.0.52
Connection: close
Content-Length: 109
Content-Type: text/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8" ?>
<rsp stat="fail">
.<err code="98" msg="Invalid auth token" />
</rsp>

Comment 6

[/\/\arcio Galli]

=====
Trace using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060626 BonEcho/2.0a3; 

The image bits are attached in the post. 
=====
POST /services/upload/ HTTP/1.1
Host: www.flickr.com
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060626 BonEcho/2.0a3
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive
Content-Type: multipart/form-data; boundary=123456789
Content-Length: 2227
Pragma: no-cache
Cache-Control: no-cache

--123456789
Content-Disposition: form-data; name="api_key"

some_random_value
--123456789
Content-Disposition: form-data; name="auth_token"

some_random_value
--123456789
Content-Disposition: form-data; name="api_sig"

b06ab2cf90144e6ca739db849092474e
--123456789
Content-Disposition: form-data; name="photo"; filename="minimo.png"
Content-Type: image/png

.PNG
.
...
IHDR... ... .............gAMA..X..G..... cHRM..z%..............u...._..:....oi..+....IDATx.b...-.-.@.1.....oo..#I.@..f..-....H...@......?-.]......EI1q"u....>8x....W....t.r.u....>....m....W..Xx8..m?.%!I.F.."..{..8{...].{..._>....H...D......................h.... ...`...G/.Q3...P...t..y..........@....|.@...n./........o..d...r...uZ....v................X
L<....Zjo.~.g...!h._....i.g=}........e.@..|.4...?.>}x...../.<.r...[...|p......~..~...r.._~....`a...E....??.98..E5.T....e.e$.%..%D.....XY998...d..+.Cr...z...o....,.\..br|2....f>._...d....g.ef.rh..
+><.....o_^.....m.fv..\.k:..C...d.04.o..\........-........bg...........?...~.0..`..../.0..0.7_.3}.....W...|....5W.v..i..(..ggc.. P.021.y.m.........LL..zj||.......{...._..01~da~...D.Y..03~...(....FV.~..M-eI....W.
./i..+...d..."..\.6L^.....*.#..ss........?......01|cfx.........of..?F......88Xo^.........e-...LLP....%.y..o.....mJ.....yx8...!..A...........@..`A..77..3'.2.~....uJ^j&......L.l...Y...:..{......@.Y...............x...........<......*r........a.h......v......O_.$5o......W...L.?.@.b.........BL,wN.........k...7.Q....=.#:r...X.....;...22...._.......~.........?...../]...gmF.....!0c........~!~.s....U.2a...../.3...L.`.b........,...`M..(.
5.H......$. .?<bN]<.$e.......4.......e.....o........_.i%FF..7...9....................3.., *...?..............~HII.....?`
...-..#q........1......`.. ..@>..Y(...?|.t..].!e.......f...o..5.....*s.Mz...+...?,.l......?|..=.!...P6@.!.!.g/_.[(....|._..].|ea....P~N&Ei.m...".fO......7........>..i......(..C..=z....Dd
..?T...iz....J3...9rf....VA^.......V/..'.....woc..D......W0...&6w..0.....cIF....a. .O....i....+...9Eh.@......w...........I..3../.:..z.
.....>{.*..X.3....o.@.....1..........K..~..uw......4.7D$`FT.,U..s...}...7.]...A...........(B..?...q....63.34.CS2..h.2\.rYAN.....lF ..@....BQ.0....h....g.. xN.j..T..[0.. -...........%....IEND.B`.
--123456789HTTP/1.1 200 OK
Date: Fri, 14 Jul 2006 17:41:40 GMT
Server: Apache/2.0.52
Connection: close
Content-Length: 109
Content-Type: text/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8" ?>
<rsp stat="fail">
.<err code="98" msg="Invalid auth token" />
</rsp>

Comment 7

[/\/\arcio Galli]

If I replace the nsIFileInputStream with a string, then the testcase works. The nsIFileInputStream object is passed to the nsIMultiplexInputStream used in the POST. 
 

Comment 8

[Darin Fisher]

Marcio, try wrapping the nsIFileInputStream with a nsIBufferedInputStream.  Then, the bug should go away.  The problem here (I suspect) is that nsFileInputStream does not implement ReadSegments.  I think this bug may be invalid.

Comment 9

[/\/\arcio Galli]

Darin,

The workaround works nice. Works with all versions of Firefox. 

Do you think ReadSegments broke? ( considering it was working before ). So I leave  to you to decide if it s invalid. 

Comment 10

[Darin Fisher]

No, it didn't break.  This bug only looks like a regression because we added code to always inject that buffered input stream into the mix, but it had the downside of causing some odd bug that we couldn't track down.

Fixing summary and confirming.  XMLHttpRequest should be able to detect this condition using NS_InputStreamIsBuffered and then performing the requisite wrapping if that returns false.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Note: steps in comment 0 should say signed.applets.codebase_principal_support for the preference name...

Comment 12

[Boris Zbarsky [:bzbarsky]]

So the thing is, this testcase passes an nsMultiplexInputStream to XMLHttpRequest.  That stream _is_ buffered (or rather supports ReadSegments).  But one of its constituent streams is not.

Note that actual file input submission wraps the file stream in a buffered stream before sticking it in the multiplex stream.

I suppose we could also fix the multiplex stream to wrap streams in buffered streams...

Comment 13

[Boris Zbarsky [:bzbarsky]]

Ah... so the multiplex stream lives in xpcom.  Which means it can't depend on the buffered stream, which lives in necko.

I think we should simply document that consumers of nsIMultiplexInputStream should only pass it buffered streams (and make it throw if an unbuffered one is passed in)...

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This seems like WONTFIX and just fix in the documentation.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Or we could move buffered streams into XPCOM...  that would make more sense to me anyway.  biesi, bsmedberg, darin, what do you think?

Comment 16

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

yes, I think we should do that. of course that alone won't fix this bug.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Per earlier comments, this is a necko/xpcom bug, not a DOM one.

Comment 18

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 19

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 20

[Valentin Gosu [:valentin] (he/him)]

I think this is fixed, judging by this code:
https://searchfox.org/mozilla-central/rev/cb5faf5dd5176494302068c553da97b4d08aa339/xpcom/io/nsMultiplexInputStream.cpp#76-82

if (!NS_InputStreamIsBuffered(mBufferedStream)) {
  nsCOMPtr<nsIInputStream> bufferedStream;
  nsresult rv = NS_NewBufferedInputStream(getter_AddRefs(bufferedStream),
                                          mBufferedStream.forget(), 4096);
  NS_ENSURE_SUCCESS(rv, rv);
  mBufferedStream = bufferedStream;
}