GNOME Bugzilla – Bug 587907
body-less responses seem to not get content-sniffed/got-chunk/got-body emitted
Last modified: 2009-07-10 17:25:29 UTC
After WebKitGTK+ started using the new content sniffing feature in soup three layout tests started failing, and the cause seems to be that 412 responses are not getting a content-sniffed emission. This is the output of debugging calls I added to got-headers, got-chunk, and content-sniffed handlers in our soup backend: +** (DumpRenderTree:6435): DEBUG: got-headers: 412 +** (DumpRenderTree:6435): DEBUG: got-headers: 304 +** (DumpRenderTree:6435): DEBUG: got-headers: 200 +** (DumpRenderTree:6435): DEBUG: content-sniffed: 200 +** (DumpRenderTree:6435): DEBUG: sniffed +** (DumpRenderTree:6435): DEBUG: got-chunk: 200 As you can see, 412 and 304 do not get content-sniffed, nor got-chunk. This may have been caused by the new content sniffing code, but it is likely that it is just being exposed by the fact that I now rely on content-sniffed to report headers, instead of relying on got-headers.
There is no special handling for status 412 anywhere; if got-sniffed is not emitted for that test, it has something to do with the exact specific response returned in that test, and not anything to do with it being a 412. 304, OTOH, is one of the magic responses that never has a response body. It appears that the check after got_body in io_read() is broken; io->delayed_chunk_data will only be set if it has read some data, so if there is no response body (either because the status code never has a response body or just because the server didn't send a response), then we don't emit got-sniffed. It needs to be checking io->delay_got_chunks first. Presumably the 412 response in this test has "Content-Length: 0", and so it also triggers the bug. Assuming this is the case, we should add a test to tests/sniffing-test for this (0-length response, via both Content-Length and chunked).
Created attachment 138187 [details] [review] fix, not yet ready for pushing I say this is not yet ready to be pushed because I found another problem while working on the tests for this problem, and I believe that needs to go into a commit before this one (which will have a small conflict with the patch).
Created attachment 138190 [details] [review] fix for the hangs on chunked encoding Let me abuse this bug a little bit, posting also this fix here =).
(In reply to comment #2) > Created an attachment (id=138187) [edit] > fix, not yet ready for pushing There are a few problems here, and now that I'm looking more carefully at the existing code, I see a few more problems there too. Among other things, you can't safely refer to io->whatever in between a signal emission and the RETURN_IF_CANCELLED_OR_PAUSED. I'm going to fix up this patch; feel free to commit the chunked fixes.
(In reply to comment #4) > There are a few problems here, and now that I'm looking more carefully at the > existing code, I see a few more problems there too. Among other things, you > can't safely refer to io->whatever in between a signal emission and the > RETURN_IF_CANCELLED_OR_PAUSED. I'm going to fix up this patch; feel free to > commit the chunked fixes. Hmm, OK. Thanks for looking.
should be fixed in master