bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

image decoders' WriteFrom() shouldn't ignore errors from ReadSegments() / callback




10 years ago
10 years ago


(Reporter: Dolske, Unassigned)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)




10 years ago
Found this issue when fixing bug 413512 and writing a testcase. That bug covered a crash caused by trying to reencode the result of decoding an invalid image file.

The image decoders are invoked by feeding a stream to imgIDecoder::WriteFrom(), which then calls stream->ReadSegments() with a callback. The callback handles the actual decoding. The interesting observation is that the callback was returning an error, but it was getting lost somewhere -- WriteFrom() returned NS_OK.

Bug 191227 added code to ignore errors from the callbacks. I'm not clear on what the actual problem in 191227 was, but the wholesale suppression of errors seems like a bad idea. In the bug I was working on, it causes image decoding to always (?) succeed, and forces error handling away from where the error actually occurred.

If there are non-critical errors that should be ignored in some cases, seems like a better approach would be for the caller/callee to drop them, or *maybe* have ReadSegments() just ignore a subset of errors.
Note that the documentation for the writer function in nsIInputStream.idl explicitly says:

   Errors are never passed to the caller of ReadSegments.

That is, the ReadSegments return value indicates whether the part the stream is doing worked correctly.  The right way to signal errors from the callback to the caller of ReadSegments, if desired, is to use the closure argument.


10 years ago
Blocks: 414185

Comment 2

10 years ago
Hmm, sounds like the error handling in the decoders needs to tweaked so that errors are not discarded.
Component: Networking → ImageLib
QA Contact: networking → imagelib
Summary: A buffered stream's ReadSegments() shouldn't ignore errors from the callback → image decoders' WriteFrom() shouldn't ignore errors from ReadSegments() / callback

Comment 3

10 years ago
Ah... The decoders are using |this| as the closure, and setting a member variable to indicate error status. So whatever the problem is in 414185 is probably an isolated bug or edge case, not a general failure.
Last Resolved: 10 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.