On Fri, 19 Apr 2024 18:28:19 GMT, robert engels <d...@openjdk.org> wrote:

>> src/jdk.httpserver/share/classes/sun/net/httpserver/FixedLengthOutputStream.java
>>  line 68:
>> 
>>> 66:         remaining --;
>>> 67:         if(remaining==0) {
>>> 68:             close();
>> 
>> `close()` has the side effect of closing the input stream. This again may 
>> break other code.
>
> We can remove this. As I stated, it is a bad bug and will leave the 
> connection in a corrupted state if the handler does not call close() on the 
> outstream or the exchange, so I don't think existing code "works".
> 
> Also, I think issuing the close() here is benign, because any further writes 
> would throw an exception (too much data), and a subsequent close (or multiple 
> closes) are ignored.
> 
> Essentially, once you have written all of the data you stated you would send, 
> it is a critical error if you do anything on the output stream other than 
> flush() or close(). I added a check that a flush() on a closed stream is 
> ignored.

as to the input stream being closed, that already would have happened. If the 
handler closed the output stream, then tried to read the input stream, it would 
fail.

in retrospect, I think this should be removed, because if the handler was not 
closing the output stream before, the connection would have been dead, so it 
seems doubtful this was the case in the field. and then we can fix the broken 
tests to not not have a bug in not properly closing the output stream.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1572786639

Reply via email to