On Sun, 3 Sep 2023 09:16:24 GMT, Vyom Tewari <[email protected]> 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 I am concerned that you are reusing the `rememberedException` field, which is used by getInputStream(). A server doesn't need to read the request body if it doesn't need it to send the response. So it could close the socket input and write the response body. The interaction between getOutputStream and getInputStream makes me uneasy here. I believe you may need to add more tests where the server doesn't read the request body before replying, and check that you can still read the response - for instance, it could be 404 with some text, and you shouldn't get an exception when reading the response body. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15483#issuecomment-1721468032
