Trailers in responses with "Transfer-Encoding: chunked" are treated as chunks (if their name begins with [A-F]) or lead to an error (otherwise). Note that some trailers are allowed in HTTP/1.1 even if we don't send "TE: trailers". I'll attach a patch, but it is probably not complete. It does not add support for the "Trailer" header and I do not know if problems are caused by adding headers after the page itself has arrived. While writing the patch I noticed a related problem with nsHttpTransaction::Read(): I am not sure, but I think we loose data if pipelining is enabled and data of different responses is contained within a single chunk arriving over the network.

lack of support for trailers is a regression from the http branch landing. pipelining is currently not implemented, but that being said the problem you have described will need to be solved once pipelining is activated. my approach to that problem is simply to have a PushBack method on nsHttpConnection, so data can be in a sense given back to the connection (and later given out to the next transaction in the pipeline). so, in short, this "bug" is really just a prerequisit for pipelining in the new world.

clarence: i'm not completely sold on the design put forth by your patch. i think the changes could be more isolated to either just the chunked decoder or just the http transaction code. in the old code, we let the chunked decoder eat the trailing headers and only report EOF after they had been consumed. this is probably the right thing now as well, but for some time i have considered doing it in nsHttpTransaction (in much the same manner as regular headers are parsed). this may just add clutter to nsHttpTransaction, so for that reason i'm leaning toward the former solution. at any rate, i've gone ahead and attached a very simple work around for this bug just in case.

I am assuming you made the DontReuse function so you could call it from elsewhere as well (in future) r=gagan

how hard would be it to consume the last bytes on the socket. Note that ftp is going to want something that takes a nsSocketTransport and consumes any extra data on the socket. If it is too hard to do now, that is fine. (s)r=d

yeah.. i just didn't want to slap something together, and trailers are a VERY rare occurance. until getting the right patch, i just wanted to land something to make us at least not fail to load the next page.

a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)

minimal patch checked in... futuring this bug.

-> moz 0.9.5 since this will need to be fixed before the pipelining code can land.

Comment on attachment 50655 [details] [diff] [review] v1.0 patch >+ else { >+ // save the partial line; wait for more data >+ mLineBuf.Append(buf, count); >+ *bytesConsumed = count; >+ } this is no good... it actually needs to skip a trailing CR.

Comment on attachment 50656 [details] [diff] [review] v1.1 revised to better handled CRLF line delimiters r=gagan

Comment on attachment 50656 [details] [diff] [review] v1.1 revised to better handled CRLF line delimiters sr=mscott

fixed-on-trunk

Verified per darin's comment.