303376 - [FIX]loadSubscript failed with async protocol (http) which uses NS_ImplementChannelOpen in the Open and js files bigger than 4096 bytes

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Jean-Pierre Dumont]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; .NET CLR 1.0.3705)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050707

the catched error returned is #define LOAD_ERROR_READUNDERFLOW "File Read Error 
(underflow.)"

cf file mozilla/xpconnect/loader/mozJSSubScriptLoader.cpp:298

rv = chan->Open (getter_AddRefs(instream));
    if (NS_FAILED(rv))
    {
        errmsg = JS_NewStringCopyZ (cx, LOAD_ERROR_NOSTREAM);
        goto return_exception;
    }
    
    rv = chan->GetContentLength (&len);
    if (NS_FAILED(rv) || len == -1)
    {
        errmsg = JS_NewStringCopyZ (cx, LOAD_ERROR_NOCONTENT);
        goto return_exception;
    }

    buf = new char[len + 1];
    if (!buf)
        return NS_ERROR_OUT_OF_MEMORY;
    
    rv = instream->Read (buf, len, &readcount);
    if (NS_FAILED(rv))
    {
        errmsg = JS_NewStringCopyZ (cx, LOAD_ERROR_BADREAD);
        goto return_exception;
    }

the problem is that this code compares the chan->GetContentLength (actually 
more than 4096) with the readcount of the instream that is only 4096 at this 
time

I notice that the bug is for asynchronous protocol wich use 
NS_ImplementChannelOpen(this, ...) in their Synchronous Open Channel funtion to 
glue to their OpenAsync...

So perhaps the bug comes from NS_ImplementChannelOpen implementatation

Best Regards,
  

Reproducible: Always

Comment 1

[Jean-Pierre Dumont]

sorry, but I ommit the last fragment of code wich return the error ;-)

    if (NS_STATIC_CAST(PRUint32, len) != readcount)
    {
        errmsg = JS_NewStringCopyZ (cx, LOAD_ERROR_READUNDERFLOW);
        goto return_exception;
    }

Comment 2

[Boris Zbarsky [:bzbarsky]]

That looks like a bug in the subscript loader -- nothing guarantees that all the data is available immediately.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: I believe this is the right thing here....

Comment 4

[Darin Fisher]

Comment on attachment 203093 [details] [diff] [review]
I believe this is the right thing here....

lastReadCount declaration wants to be aligned with its peers ;)

r=darin

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 203093 [details] [diff] [review]
I believe this is the right thing here....

sr=jst

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed.