27022 - Files read through nsBufferedInputStream are truncated

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Michael Bruun Petersen]

Files read through nsBufferedInputStream are truncated to mBufferSize.
If I try to read a large (text, html or other) file into the browser, I only get
the first 64K.
This happens with the latest (feb 8) source, but not with M13.

The problem seems to be a bug in nsBufferedInputStram::Fill() in the file
mozilla/netwerk/base/src/nsBufferedStreams.cpp
If the buffer is empty when Fill() is called, mFillPoint is not moved back to
zero. And under some circumstances this means that nothing is read into the
buffer. 
To fix it I changed 

    if (rem > 0) {
        // slide the remainder down to the start of the buffer
        // |<------------->|<--rem-->|<--->|
        // b               c         f     s
        nsCRT::memcpy(mBuffer, mBuffer + mCursor, rem);
        mBufferStartOffset += mCursor;    
        mFillPoint = rem;
        mCursor = 0;
    }

to the following:

    if (rem > 0) {
        // slide the remainder down to the start of the buffer
        // |<------------->|<--rem-->|<--->|
        // b               c         f     s
        nsCRT::memcpy(mBuffer, mBuffer + mCursor, rem);
        mBufferStartOffset += mCursor;
    }
    mFillPoint = rem;
    mCursor = 0;

This seems to work well here, at least.

Comment 1

[Warren Harris]

I'll take this. 

Doug: Can you review this change too?

Jud: Could this be related to the data transfer problem you were seeing in the 
socket transport?

Comment 2

[Judson Valeski]

quite possibly (rick has that bug now). let me know when you've got something 
and I'll test it.

Comment 3

[Doug Turner (:dougt)]

nice find [e-mail].  warren, r=dougt.

Comment 4

[Warren Harris]

Jud: Give this patch a try and see if it fixes the other problem you were 
seeing.

Comment 5

[Judson Valeski]

it may have helped, but we still bogg out. I'm noticing some event processing 
issues. If I give other windows attention while uploading, we seem to fall out 
of select and write some data :-/

Comment 6

[Judson Valeski]

scratch the event processing association comment, I'm tired. and I take back the 
"maybe" comment too. If I try to upload a 150k file, we get up to about 145k 
then stall. If I try to upload a 50k file, we get up to about 40k then stall.

Comment 7

[Warren Harris]

Sounds like it's not flushing the last chunk in the buffer. If this is for 
input streams, then maybe we should just turn buffering off for now. We 
primarily determined the need for buffering for output streams. Input streams 
where a "bonus."

Comment 8

[Warren Harris]

I think we need to pull mBufferStartOffset += mCursor; out of the if statement 
too, right? Otherwise the seek stuff gets thrown off.

Comment 9

[Michael Bruun Petersen]

I think you're right. "mBufferStartOffset += mCursor;" should also be moved. 

Comment 10

[Warren Harris]

mbp -- muchos gracias for spotting this, and for your suggested fix!

Comment 11

[Tom Everingham]

verified:
Linux 2000021510