510528 - nsInputStreamPump.cpp should not throw an error if no data is consumed

Status: RESOLVED INVALID

(Core :: Networking, defect)

Description

[Bobby Holley (:bholley)]

nsInputStreamPump throws an error if a ReadSegments implementation reads 0 bytes in OnDataAvailable.
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsInputStreamPump.cpp#529

The interface docs seem to suggest that a consumer is allowed to consume no bytes
http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsIInputStream.idl#159

In bug 435296, I've taken care to consistently set bytesRead to 0 when the input is erroneous, since it seemed more logical than to claim we read everything. As such, any time imagelib detects and handles erroneous image data, the input stream pump throws an error in the console. This is harmless, but I don't think we should be doing it.

Joe suggested I CC bz to make sure he considers setting bytesRead to 0 to be reasonable semantics.

Comment 1

[Bobby Holley (:bholley)]

biesi pointed out that there are other interface docs that supersede this:

http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsIStreamListener.idl#51

resolving as invalid.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsIStreamListener.idl#51
 52      * [...]  The onDataAvailable impl
 53      * must read exactly |aCount| bytes of data before returning.

You have to read data _somehow_ in onDataAvailable. The issue is not in readSegments here.

The other option is returning an error from onDataAvailable, which seems perhaps more correct when an error happens, but I don't really know your code enough to say.