Open Bug 549444 Opened 15 years ago Updated 8 months ago

Move nsUnknownDecoder logic into nsHttpChannel

Categories

(Core :: Networking: HTTP, defect, P5)

Product:
Core
Component:
Networking: HTTP
Type:
defect

Tracking

()

People

(Reporter: dwitte, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-would-take])

The title of this bug may be disingenuous, but the problem is like so: If one registers a listener with nsHttpChannel, but no Content-Type hint is provided, nsUnknownDecoder will insert itself as a listener in between the channel and the provided listener. If less than 512 bytes of data is provided in the stream, nsUnknownDecoder will buffer it, waiting for a Content-Type response header -- without calling the provided listeners' ODA. nsHttpChannel::OnStopRequest will then call nsUnknownDecoder::OnStopRequest, which will belatedly call the provided listeners' ODA. By this time, the channel has changed its state, and things like nsHttpChannel::IsPending will return false. Which can be confusing for the listener. One suggestion, if I understand bz and jduell correctly, was to move some of the logic of nsUnknownDecoder into the channel itself, such that it can call the listeners' OnStartRequest and ODA before the channel has stopped and changed its state. Obviously, this bug is somewhat of an edge case, but if we're reworking this stuff at some point then we'd want to fix it.
This could be done somewhat like the binary detector in bug 389677 if HTTP channel could guarantee us that it has at least 512 bytes hanging out ready to pass to CallTypeSniffers when we enter nsHttpChannel::CallOnStartRequest.
Of course the difference is that you want to do this even if the LOAD_CALL_CONTENT_SNIFFERS flag isn't set.
Sure. We could even have a content sniffer category that's always called for HTTP if we want or something. The question is how easy it is to provide the guarantee of comment 1. I guess the text-or-binary sniffer more or less already relies on this working, right?
The text-or-binary sniffer doesn't really care so much, right? I don't think we provide that guarantee right now (if the server is slow, etc)...
> The text-or-binary sniffer doesn't really care so much, right? It does if we're going to follow the abarth mime sniff spec. > I don't think we provide that guarantee right now What's the simplest way to do so? Just buffer in nsHttpChannel itself?
I think you'd basically have to do it in nsInputStreamPump, i.e. delay the notification until you have that many bytes. That may be relatively easy to do actually.
Would that work for the case when we read the first 200 bytes from cache and the rest from the network or something?
No. That case will be a little hard to handle, considering the data comes from two different input streams then :/
What would be wrong with buffering 512 bytes of data in nsHttpChannel itself?
It's a lot of code to get it all right, I think.
Whiteboard: [necko-would-take]
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P5
Severity: normal → S3
Blocks: 1851982
You need to log in before you can comment on or make changes to this bug.