Okhttp: Handling of early 403 FORBIDDEN with Connection: close whilst streaming

[fakraemer]

We are using okhttp against an HTTP server (Finagle) which will cut off the connection early if the client is not authorised for a particular resource.

HTTP/1.1 403 Forbidden\r\n
[Expert Info (Chat/Sequence): HTTP/1.1 403 Forbidden\r\n]
Request Version: HTTP/1.1
Status Code: 403
Response Phrase: Forbidden
Content-Length: 0\r\n
Connection: close\r\n
\r\n

The client does not handle this behaviour well, as in it escapes with

java.net.SocketException: Connection reset
    at java.net.SocketOutputStream.socketWrite(SocketOutputStream.java:118)
    at java.net.SocketOutputStream.write(SocketOutputStream.java:159)
    at okio.Okio$1.write(Okio.java:78)
    at okio.AsyncTimeout$1.write(AsyncTimeout.java:155)
    at okio.RealBufferedSink.emitCompleteSegments(RealBufferedSink.java:133)
    at okio.RealBufferedSink.write(RealBufferedSink.java:45)
    at com.squareup.okhttp.internal.http.HttpConnection$FixedLengthSink.write(HttpConnection.java:302)
    at okio.RealBufferedSink.emitCompleteSegments(RealBufferedSink.java:133)
    at okio.RealBufferedSink.writeAll(RealBufferedSink.java:83)
    at com.squareup.okhttp.internal.http.URLConnectionTest$2.writeTo(URLConnectionTest.java:2235)
    at com.squareup.okhttp.Call.getResponse(Call.java:202)
    at com.squareup.okhttp.Call.execute(Call.java:80) 

I'm not an expert on the HTTP/1.1 spec, but given from what I've read so far, I believe the server is in the right and I'd expect the client to parse the response headers and expose them. The sections I'm referring to are

8.1.2 Overall Operation
... Persistent connections provide a mechanism by which a client and a server can signal the close of a TCP connection. This signaling takes > place using the Connection header field (section 14.10). Once a close has been signaled, the client MUST NOT send any more requests on that connection.

and further

8.1.2.1 Negotiation
... If the server chooses to close the connection immediately after sending the
response, it SHOULD send a Connection header including the connection-token close.
An HTTP/1.1 client MAY expect a connection to remain open, but would decide to keep it open based on whether the response from a server contains a Connection header with the connection-token close.

I tried to recreate the problem in your test infrastructure, please see URLConnectionTest#connectionCloseEarlyInResponseWhilstStreaming (based on 7f763c1).

[swankjesse]

Comnection reuse is a red herring here. Your situation has two different results:

• The request body was truncated by the server. That's an error.
• The response of 403.

OkHttp gives up as soon as the error is encountered. It's unlikely that we can do anything better.

That said, I'm curious about what other user agents do here. Does curl return the 403? What about Chrome?

[fakraemer]

Hi Jesse,

Thanks for your reply. I'm not sure if I've done this correctly, but it seems curl can cope with the server responding early with a 403.

dd if=/dev/zero bs=10485760 count=1|curl -v -H "Content-Type: application/octet-stream" -H "Content-Length: 10485760" -H "Connection: Keep-Alive" -H "Expect:" -H "Transfer-Encoding:" -T - http://localhost:8080/context
* About to connect() to localhost port 8080 (#0)
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> PUT /context HTTP/1.1
> User-Agent: curl/7.29.0
> Host: localhost:8080
> Accept: */*
> Content-Type: application/octet-stream
> Content-Length: 10485760
> Connection: Keep-Alive
> 
< HTTP/1.1 403 Forbidden
< Content-Length: 0
< Connection: close
< 
* we are done reading and this is set to close, stop send
* Closing connection 0

And this is what I got from the tcpdump

PUT /context HTTP/1.1
User-Agent: curl/7.29.0
Host: localhost:8080
Accept: */*
Content-Type: application/octet-stream
Content-Length: 10485760
Connection: Keep-Alive

...

HTTP/1.1 403 Forbidden
Content-Length: 0
Connection: close

Note that I changed the URLs and truncated the request body shown here, ... standing for the zeros I generated. The entire conversation is 147770 bytes long, so since the input is 10485760 bytes I can assume that we got truncated early and could not stream out the full body.

Cheers,
Fabs

[ricellis]

There are some other cases where the spec allows a server to close a connection before sending a request body is complete, such as 413 Request Entity Too Large. It would be nice to read those responses and stop sending data rather than throw a SocketTimeoutException. This is particularly true because we also can't avoid starting the send by using Expect: 100-Continue due to #675.

I think the relevant part of the spec relating to this is actually 8.2.2.

For the 413 case I was looking at OkHttp (3.4.2) does:

Exception in thread "main" java.net.SocketTimeoutException: timeout
    at okio.Okio$3.newTimeoutException(Okio.java:210)
    at okio.AsyncTimeout.exit(AsyncTimeout.java:288)
    at okio.AsyncTimeout$1.write(AsyncTimeout.java:184)
    at okio.RealBufferedSink.emitCompleteSegments(RealBufferedSink.java:171)
    at okio.RealBufferedSink.write(RealBufferedSink.java:41)
    at okhttp3.internal.http.Http1xStream$FixedLengthSink.write(Http1xStream.java:282)
    at okio.RealBufferedSink.emitCompleteSegments(RealBufferedSink.java:171)
    at okio.RealBufferedSink.write(RealBufferedSink.java:91)
    at okhttp3.RequestBody$2.writeTo(RequestBody.java:96)
    at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:47)
... interceptor chain omitted...
    at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:170)
    at okhttp3.RealCall.execute(RealCall.java:60)
...
Caused by: java.net.SocketException: Broken pipe (Write failed)
    at java.net.SocketOutputStream.socketWrite0(Native Method)
    at java.net.SocketOutputStream.socketWrite(SocketOutputStream.java:109)
    at java.net.SocketOutputStream.write(SocketOutputStream.java:153)
    at sun.security.ssl.OutputRecord.writeBuffer(OutputRecord.java:431)
    at sun.security.ssl.OutputRecord.write(OutputRecord.java:417)
    at sun.security.ssl.SSLSocketImpl.writeRecordInternal(SSLSocketImpl.java:876)
    at sun.security.ssl.SSLSocketImpl.writeRecord(SSLSocketImpl.java:847)
    at sun.security.ssl.AppOutputStream.write(AppOutputStream.java:123)
    at okio.Okio$1.write(Okio.java:78)
    at okio.AsyncTimeout$1.write(AsyncTimeout.java:180)
    ... 27 more

whereas cURL does manage to read the response status and body and stop the send:

< HTTP/1.1 413 Request Entity Too Large
* HTTP error before end of send, stop sending
< 
{"error":"too_large","reason":"the request entity is too large"}
* Closing connection 0

[frannoriega]

Today (6 years after this issue was opened), I experienced the same problem.
I managed to create the following network interceptor which allowed me to work around the issue, using okhttp 4.6.0.

import okhttp3.Connection;
import okhttp3.Interceptor;
import okhttp3.MediaType;
import okhttp3.Protocol;
import okhttp3.Response;
import okhttp3.ResponseBody;
import okhttp3.internal.http1.HeadersReader;
import okio.Buffer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.InputStream;
import java.net.ProtocolException;
import java.net.Socket;
import java.net.SocketException;
import java.util.Objects;

import javax.annotation.Nonnull;

public class SocketClosedRecoveryInterceptor implements Interceptor {

    @Nonnull
    @Override
    public Response intercept(@Nonnull Chain chain) throws IOException {
        try {
            return chain.proceed(chain.request());
        } catch (SocketException e) {
            Connection connection = Objects.requireNonNull(
                    chain,
                    "The interceptor chain should have a non-null connection").connection();
            Socket socket = Objects.requireNonNull(
                    connection,
                    "The interceptor connection should have a non-null socket").socket();
            InputStream inputStream = Objects.requireNonNull(
                    socket,
                    "The interceptor socket should have a non-null input stream").getInputStream();
            try (Buffer buffer = new Buffer()) {
                buffer.write(inputStream.readNBytes(inputStream.available()));
                HeadersReader headersReader = new HeadersReader(buffer);
                StatusLine statusLine = StatusLine.parse(headersReader.readLine());
                Response.Builder builder = new Response.Builder()
                        .request(chain.request())
                        .protocol(statusLine.protocol)
                        .code(statusLine.code)
                        .message(statusLine.message)
                        .headers(headersReader.readHeaders())
                        .body(ResponseBody.create(buffer, MediaType.get("text/plain"), buffer.size()));
                return builder.build();
            }
        }
    }

    /**
     * An HTTP response status line like "HTTP/1.1 200 OK". Copied from
     * {@link okhttp3.internal.http.StatusLine}, since it was package-private.
     */
    private static final class StatusLine {
        /** Numeric status code, 307: Temporary Redirect. */
        public static final int HTTP_TEMP_REDIRECT = 307;
        public static final int HTTP_PERM_REDIRECT = 308;
        public static final int HTTP_CONTINUE = 100;

        public final Protocol protocol;
        public final int code;
        public final String message;

        public StatusLine(Protocol protocol, int code, String message) {
            this.protocol = protocol;
            this.code = code;
            this.message = message;
        }

        public static StatusLine get(Response response) {
            return new StatusLine(response.protocol(), response.code(), response.message());
        }

        public static StatusLine parse(String statusLine) throws IOException {
            // H T T P / 1 . 1   2 0 0   T e m p o r a r y   R e d i r e c t
            // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0

            // Parse protocol like "HTTP/1.1" followed by a space.
            int codeStart;
            Protocol protocol;
            if (statusLine.startsWith("HTTP/1.")) {
                if (statusLine.length() < 9 || statusLine.charAt(8) != ' ') {
                    throw new ProtocolException("Unexpected status line: " + statusLine);
                }
                int httpMinorVersion = statusLine.charAt(7) - '0';
                codeStart = 9;
                if (httpMinorVersion == 0) {
                    protocol = Protocol.HTTP_1_0;
                } else if (httpMinorVersion == 1) {
                    protocol = Protocol.HTTP_1_1;
                } else {
                    throw new ProtocolException("Unexpected status line: " + statusLine);
                }
            } else if (statusLine.startsWith("ICY ")) {
                // Shoutcast uses ICY instead of "HTTP/1.0".
                protocol = Protocol.HTTP_1_0;
                codeStart = 4;
            } else {
                throw new ProtocolException("Unexpected status line: " + statusLine);
            }

            // Parse response code like "200". Always 3 digits.
            if (statusLine.length() < codeStart + 3) {
                throw new ProtocolException("Unexpected status line: " + statusLine);
            }
            int code;
            try {
                code = Integer.parseInt(statusLine.substring(codeStart, codeStart + 3));
            } catch (NumberFormatException e) {
                throw new ProtocolException("Unexpected status line: " + statusLine);
            }

            // Parse an optional response message like "OK" or "Not Modified". If it
            // exists, it is separated from the response code by a space.
            String message = "";
            if (statusLine.length() > codeStart + 3) {
                if (statusLine.charAt(codeStart + 3) != ' ') {
                    throw new ProtocolException("Unexpected status line: " + statusLine);
                }
                message = statusLine.substring(codeStart + 4);
            }

            return new StatusLine(protocol, code, message);
        }

        @Override
        public String toString() {
            StringBuilder result = new StringBuilder();
            result.append(protocol == Protocol.HTTP_1_0 ? "HTTP/1.0" : "HTTP/1.1");
            result.append(' ').append(code);
            if (message != null) {
                result.append(' ').append(message);
            }
            return result.toString();
        }
    }
}

My hypothesis is that the underlying SocketInputStream that OkHTTP is using, throws the SocketException: connection reset whenever there is an attempt to read more bytes than what are actually available.

What I did was basically read only those available bytes (inputStream.readNBytes(inputStream.available())).
The problem is that by doing that, I was working with the raw bytes from the response (outside of OkHTTP types), so I had to parse to create the Response (I copied the code that OkHTTP was using to parse what I needed, the StatusLine class, which is package-private) .

I was mostly interested in getting the status code and message, but I went the extra mile to try to get a text/plain response body.

So far, I tested this with the troublesome requests and it is working. I will keep testing this and see if the hypothesis is correct.
I wanted to share what I've found so far (it could be a useful workaround for others having the same issue) and restart a discussion to see if this can be handled by OkHTTP.