649338 - Expose mUploadStreamHasHeaders on the HttpBaseChannel to allow removing appended headers to nsIUploadChannel

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

This bug has been originally created for Firebug here:
http://code.google.com/p/fbug/issues/detail?id=4327

Firebug is using (nsIUploadChannel) request.uploadStream to read data
posted with an HTTP request.

STR:
The problem appears if you load following page:
http://getfirebug.com/tests/issues/4327/

Check the action.php requests (note that Flash is used to post that data). When Firebug reads posted data from the upload channel it looks as follows:

Content-type: application/x-www-form-urlencoded
Content-length: 26

data=%7Bfoo%3A%27foo%27%7D

But WireShark shows only:

data=%7Bfoo%3A%27foo%27%7D

===

There is no way how to safely clear away these headers so, Firebug
shows the same as Wireshark.

Boris is proposing:
Firebug could check the value of mUploadStreamHasHeaders on the HttpBaseChannel, but it's not exposed anywhere. 

You can also read related discussion:
http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/1faabb0d0a8d26f7?hl=en#

Honza

Comment 1

[Boris Zbarsky [:bzbarsky]]

Honza (mayhemer), does that make sense?  Want to write a patch?

Comment 2

[Honza Bambas (:mayhemer)]

Yes, it perfectly makes sense for me.  It is a very simple solution.

Comment 3

[Boris Zbarsky [:bzbarsky]]

What about the second question?  ;)

Comment 4

[Honza Bambas (:mayhemer)]

(In reply to comment #3)
> What about the second question?  ;)

I can do that :)

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Can we target Firefox 6? What flag should I set (if any)?

Honza

Comment 6

[Boris Zbarsky [:bzbarsky]]

There is no flag.  If it gets done in time, it gets in.  If if doesn't, it doesn't.

Comment 7

[Honza Bambas (:mayhemer)]

My plan is to put readonly attribute bool uploadStreamHasHeaders to one of nsIUploadChannel or nsIUploadChannel2 or (and it is the question) new nsIUploadChannel3 interface.

What is a desired option?  I believe Honza prefers to modify nsIUploadChannel2.  Me probably too.

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to comment #7)
> I believe Honza prefers to modify nsIUploadChannel2
Yes

Honza

Comment 9

[Honza Bambas (:mayhemer)]

Attachment: v1 [Check in comment 11]

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 532691 [details] [diff] [review]
v1 [Check in comment 11]

Please use "boolean" in the IDL, not PRBool.  r=me with that.

Comment 11

[Honza Bambas (:mayhemer)]

Comment on attachment 532691 [details] [diff] [review]
v1 [Check in comment 11]

http://hg.mozilla.org/mozilla-central/rev/a54c69c20b6c
http://hg.mozilla.org/mozilla-central/rev/9ad384bc560c (comment fix)

Comment 12

[Virgil Dicu [:virgil] [QA]]

Hello. 

Could anyone provide some simple STRs for verifying this issue?

Comment 13

[Jason Duell]

If firebug now uses this feature, I'd consider that verification enough.

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I can see that a new attribute: nsIUploadChannel2.uploadStreamHasHeaders
has been created.

So, how am I suppose to clear the headers so, Firebug shows the same post-data as Wireshark? (see comment #0)

Honza

Comment 15

[Boris Zbarsky [:bzbarsky]]

You read from the stream until you get to a CRLF CRLF.  Everything before then is headers.  Everything after that is data.  Basically, when uploadStreamHasHeaders the stream contains the end of the HTTP request headers and the blank line and then the request body.

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Boris Zbarsky (:bz) from comment #15)
> You read from the stream until you get to a CRLF CRLF.  Everything before
> then is headers.  Everything after that is data.  Basically, when
> uploadStreamHasHeaders the stream contains the end of the HTTP request
> headers and the blank line and then the request body.

Thanks Boris!
Honza

Comment 17

[u279076]

qa-: fix verification unnecessary

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Note that the original Firebug bug
http://code.google.com/p/fbug/issues/detail?id=4327

is fixed and verified by the reporter.

Honza