369787 - calling nsHttpChannel::SetContentType on a closed channel doesn't work as expected.

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Sylvain Pasche]

After the channel was dispatched and has no listener any more, calling nsHttpChannel::SetContentType() will set mContentCharsetHint, meaning the content type returned by GetContentType() is not changed.

mContentCharsetHint is used before opening the channel, but should not be used in case the channel is closed.

Comment 1

[Boris Zbarsky [:bzbarsky]]

One problem is telling apart the "already closed" state and the "already closed, about to be reopened" state.  I don't really see anything in either the API or the code that prevents someone from reopening a channel after it's done...

Comment 2

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

we don't support reopening, even though this code might not enforce that people don't do that.

Comment 3

[Boris Zbarsky [:bzbarsky]]

We should at least document that in the API then.  And maybe enforce it.  I'd be pretty happy with us adding a "was ever opened" flag or something at that point.

The reason I brought it up is that people are doing it, based on newsgroup posts I've seen.

Comment 4

[Sylvain Pasche]

Attachment: v1

This needs bug 372486 to be checked in order to compile.

Comment 5

[Sylvain Pasche]

Regarding your question about an empty http server handler, I guess it's ok doing nothing in it, as it will be dealt in a specific way:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/test/httpserver/httpd.js&rev=1.4&mark=1175-1192#1175

Comment 6

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

Comment on attachment 258960 [details] [diff] [review]
v1

please make BUGID and newType const

also, please use x-foo/x-bar

+function after_channel_closed() {
+  change_content_type();

put this in a try..catch (or try..finally) so that stop is called even if the test fails

Comment 7

[Sylvain Pasche]

Attachment: v2

Comment 8

[Darin Fisher]

Comment on attachment 259482 [details] [diff] [review]
v2

sr=darin (sorry for not reviewing this earlier)

Comment 9

[Nickolay_Ponomarev]

mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp  1.308
mozilla/netwerk/test/unit/test_bug331825.js          1.5
mozilla/netwerk/test/unit/test_bug369787.js          1.1

Comment 10

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

(In reply to comment #6)
> put this in a try..catch (or try..finally) so that stop is called even if the
> test fails

I should've pointed this out in onStopRequest as well :/ See bug 614717