Handling of early 403 FORBIDDEN with Connection: close whilst streaming #1001
Comments
|
Comnection reuse is a red herring here. Your situation has two different results:
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? |
|
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. And this is what I got from the tcpdump 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, |
|
There are some other cases where the spec allows a server to close a connection before sending a request body is complete, such as I think the relevant part of the spec relating to this is actually 8.2.2. For the whereas cURL does manage to read the response status and body and stop the send: |
As per https://tools.ietf.org/html/rfc2616#section-8.2.2 An HTTP/1.1 (or later) client sending a message-body SHOULD monitor the network connection for an error status while it is transmitting the request. Currently, okhttp implements a send request, then read response if/when request was sent successfully. This hides early responses such as 413 Payload Too Large errors from clients. This commit fixes that by adding a response read in case of an exception happening while the request is being sent. This closes square#1001 .
|
Today (6 years after this issue was opened), I experienced the same problem. 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 What I did was basically read only those available bytes ( I was mostly interested in getting the status code and message, but I went the extra mile to try to get a 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. |
As per https://tools.ietf.org/html/rfc2616#section-8.2.2 An HTTP/1.1 (or later) client sending a message-body SHOULD monitor the network connection for an error status while it is transmitting the request. Currently, okhttp implements a send request, then read response if/when request was sent successfully. This hides early responses such as 413 Payload Too Large errors from clients. This commit fixes that by adding a response read in case of an exception happening while the request is being sent. This closes square#1001 .
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.
The client does not handle this behaviour well, as in it escapes with
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
and further
I tried to recreate the problem in your test infrastructure, please see URLConnectionTest#connectionCloseEarlyInResponseWhilstStreaming (based on 7f763c1).
The text was updated successfully, but these errors were encountered: