RFR: 8314978: Multiple server call from connection failing with expect100 in getOutputStream [v2]

[Vyom Tewari]

On Sun, 3 Sep 2023 09:16:24 GMT, Vyom Tewari <vtewari at openjdk.org> wrote:

>> With the current implementation of HttpURLConnection  if server rejects the  “Expect 100-continue” then there will be ‘java.net.ProtocolException’ will be thrown from 'expect100Continue()' method. 
>> 
>> After the exception thrown, If we call any other method on the same instance (ex getHeaderField(), or getHeaderFields()). They will internally call getOuputStream() which invokes writeRequests(), which make the actual server call.
>> 
>> The code change will sets the existing variable ‘rememberedException’ when there is exception and getOutputStream0() will re-throw ‘rememberedException’  if the ‘rememberedException’ is not null. 
>> 
>> Note: getOutputStream0() also call’s  ‘expect100Continue()’  if ‘expectContinue’ is true.
>
> Vyom Tewari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   modified the junit tests names

> > With the current implementation of HttpURLConnection if server rejects the “Expect 100-continue” then there will be ‘java.net.ProtocolException’ will be thrown from 'expect100Continue()' method.
> 
> Looking at the current implementation in `sun/net/www/protocol/http/HttpURLConnection.java`, I'm unsure if `expect100Continue()` is doing the right thing when compared with what the HTTP/1.1 RFC-9110 section 10.1.1 expects https://www.rfc-editor.org/rfc/rfc9110#field.expect:
> 
> > Upon receiving an HTTP/1.1 (or later) request that has a method, target URI, and complete header section that contains a 100-continue expectation and an indication that request content will follow, an origin server MUST send either:
> > an immediate response with a final status code, if that status can be determined by examining just the method, target URI, and header fields, or
> > an immediate 100 (Continue) response to encourage the client to send the request content.
> 
> which means the server is allowed to send a (final) response code like 2xx. However, the current implementation in `expect100Continue()` appears to consider it a protocol violation and throws an exception https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java#L1362
> 
> ```java
> if (responseCode != 100) {
>     // responseCode will be returned to caller
>     throw new ProtocolException("Server rejected operation");
> }
> ```

After going through RFC(https://www.rfc-editor.org/rfc/rfc9110#field.expect:) it is clear that server  can do the followings.

####################
Requirements for servers:
A server that receives a 100-continue expectation in an HTTP/1.0 request MUST ignore that expectation.

A server MAY omit sending a 100 (Continue) response if it has already received some or all of the content for the corresponding request, or if the framing indicates that there is no content. 

A server that sends a 100 (Continue) response MUST ultimately send a final status code, once it receives and processes the request content, unless the connection is closed prematurely. 

A server that responds with a final status code before reading the entire request content SHOULD indicate whether it intends to close the connection (e.g., see Section 9.6 of [HTTP/1.1]) or continue reading the request content.

#####################

So you are correct, throwing “ProtocolException("Server rejected operation”);” is not the right way to handle ‘100-continue’. I tested the other httpclient(curl, Java new HttpClient) and both does not throw any exception in case, if server either reject(417)  or send final 200 response.  HttpURLConnection have to behave the similar way . 

As I am not sure why the existing implementation throws “ProtocolException("Server rejected operation”);” . Let see what other’s have to say about not throwing “ProtocolException("Server rejected operation”);” if server response is not 100 ?.

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

PR Comment: https://git.openjdk.org/jdk/pull/15483#issuecomment-1705185838