com.nimbusds.oauth2.sdk.http.HTTPRequest#closeStreams generates unnecessary java.net.ProtocolException "Cannot write output after reading input."

In Dynatrace, we are seeing a lot of supposedly “failed” requests in production.

These failed requests pertain to a java.net.ProtocolException with message "Cannot write output after reading input.".

This happens in sun.net.www.protocol.http.HttpURLConnection#getOutputStream0

in

// if there's already an input stream open, throw an exception
if (inputStream != null) {
    throw new ProtocolException("Cannot write output after reading input.");
}

which comes from client applications using the com.nimbusds.oauth2.sdk.http.HTTPRequest#send()

which at the end always closes the streams with com.nimbusds.oauth2.sdk.http.HTTPRequest#closeStreams

private static void closeStreams(final HttpURLConnection conn) {

    if (conn == null) {
       return;
    }

    try {
       if (conn.getInputStream() != null) {
          conn.getInputStream().close();
       }
    } catch (Exception e) {
       // ignore
    }

    try {
       if (conn.getOutputStream() != null) {
          conn.getOutputStream().close();
       }
    } catch (Exception e) {
       // ignore
    }

    try {
       if (conn.getErrorStream() != null) {
          conn.getOutputStream().close();
       }
    } catch (Exception e) {
       // ignore
    }
}

The issue with this strategy is that when you want to close the outputStream, you did not keep a reference to it and call again conn.getOutputStream() (at line 16 above) and this fails in sun.net.www.protocol.http.HttpURLConnection#getOutputStream0 because it refuses to give the reference to the outputStream after the inputStream has been initialized…

‌

An improved way, which would not throw unnecessary exceptions, would be to keep the reference to the outputStream so that getOutputStream() does not have to be called again to close it.

‌

Another way which can reduce the number of exception thrown would be to only close the outputStream when conn.getDoOutput() is true. This way would only throw the exception in case of “POST” and “PUT”.

‌

Comments (7)