Closed Bug 227976 Opened 21 years ago Closed 20 years ago

nsHttpCompressConv can fail to read any data in OnDataAvailable and return success

Categories

(Core :: Networking: HTTP, defect)

x86
Windows XP
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla1.8alpha2

People

(Reporter: bc, Assigned: darin.moz)

References

()

Details

Attachments

(1 file)

This bug is spun off of bug 223302

The symptom of the problem is an assert

###!!! ASSERTION: OnDataAvailable implementation consumed no data: 'Error', 
file
c:/work/mozilla_source/trunk/mozilla/netwerk/base/src/nsInputStreamPump.cpp,
line 451

This occurs for me on the above URL using a satellite broadband connection which
is characterised by fast download speads, slow upload speeds and high latency
and potentially effects due to the satellite NOC.  Note that this error did not
occur on a fresh SuSe install with 2003120809 build even on satellite, so this
is WinXP or Windows only I guess.

It does not occur when testing against a local web server. 

With bz's help it turns out that nsHTTPCompressConv::OnDataAvailable can have

aCount > 0 with mStreamEnded == 1 at line 137. 
<http://lxr.mozilla.org/seamonkey/source/netwerk/streamconv/converters/nsHTTPCompressConv.cpp#137>
Just to clarify further, bc's printf at that point showed:

aCount == 2459, mStreamEnded == 0
aCount == 3, mStreamEnded == 1

(so we may be partway through reading the gzip footer or whatever at that point....)
I am not seeing this with the most recent version of the dom ts. I will close it
out if I can't reproduce it anywhere.
Blocks: 245608
Note that this is showing up on a real-world web page in bug 245608...
Severity: normal → major
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha2
only reading no data triggers this assertion; clarifying summary
Summary: nsHttpCompressConv can fail to read all data in OnDataAvailable and return success → nsHttpCompressConv can fail to read any data in OnDataAvailable and return success
should this code just read the data and throw it away if mStreamEnded (basically
a ReadSegments that sets *aReadCount to aCount)? I don't know what this variable
does...
I'm not quite sure either, but it may just indicate that the data stream is done
and that what's left is either metadata or padding of some sort....  I suspect
that throwing it out is in fact the safe thing to do.
*** Bug 245608 has been marked as a duplicate of this bug. ***
Attached patch v1 patchSplinter Review
Implements biesi's suggested fix.  Verified with URL in duplicate bug.	This
seems to do the trick.
Attachment #150898 - Flags: review?(cbiesinger)
Attachment #150898 - Flags: review?(cbiesinger) → review+
fixed-on-trunk

cvs checkin comment attributed patch suggestion to boris.. sorry biesi! ;-)
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
*** Bug 222457 has been marked as a duplicate of this bug. ***
would this be a good thing for 1.7.6?
*** Bug 292413 has been marked as a duplicate of this bug. ***
fyi, i added basic auth to the dom-ts site to keep search engine bots from
eating my bandwidth. the user/password are domts/domts.
*** Bug 297545 has been marked as a duplicate of this bug. ***
*** Bug 225708 has been marked as a duplicate of this bug. ***
*** Bug 302541 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: