Closed Bug 1004313 Opened 10 years ago Closed 10 years ago

nsBufferedOutputStream::WriteSegments doesn't handle non-blocking streams well

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jimb, Assigned: jimb)

References

Details

Attachments

(1 file)

nsBufferedOutputStream::WriteSegments can return NS_ERROR_FAILURE even after having transferred some data to the underlying stream, causing the partial write count to be unavailable to JS callers, which makes it extremely difficult to use correctly from JS.

The general convention for non-blocking I/O is that, if the operation was able to transfer any bytes at all, even if not as many as was requested, it returns the count and NS_OK. Only if the operation was unable to transfer any bytes whatsoever does it return NS_BASE_STREAM_WOULD_BLOCK.

We can see this rule implemented correctly in nsBufferedOutputStream::Write: if a call to Flush fails, then we reach this exit path:

    *result = written;
    return (written > 0) ? NS_OK : rv;

However, nsBOS::WriteSegments isn't so civil:

            rv = Flush();
            if (NS_FAILED(rv))
              return rv;

As a consequence, if we go around the WriteSegments loop successfully once, and then a call to Flush fails, WriteSegments will return an error code, and the caller has no idea how many bytes were written. (I guess they can compare 'tell' results before and after, but that's horrible.)
Fix, including tests. (This patch also fixes a memcpy vs. memmove issue.)
Assignee: nobody → jimb
Status: NEW → ASSIGNED
Attachment #8415679 - Flags: review?(mcmanus)
Blocks: 797639
Comment on attachment 8415679 [details] [diff] [review]
Handle partial flushes correctly in nsBufferedOutputStream::WriteSegments.

Review of attachment 8415679 [details] [diff] [review]:
-----------------------------------------------------------------

thanks
Attachment #8415679 - Flags: review?(mcmanus) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e0c131fc557
Flags: in-testsuite+
Target Milestone: --- → mozilla32
https://hg.mozilla.org/mozilla-central/rev/4e0c131fc557
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.