square / okhttp

Square’s meticulous HTTP client for the JVM, Android, and GraalVM.
https://square.github.io/okhttp/
Apache License 2.0
45.75k stars 9.15k forks source link

Response protocol is not returning HTTP 2 for http 2 endpoint #2236

Closed lexer closed 8 years ago

lexer commented 8 years ago
public class Http2Test {

    @Test
    public void isHttp2Transport() throws IOException {
        HttpLoggingInterceptor loggingInterceptor = new HttpLoggingInterceptor();
        loggingInterceptor.setLevel(HttpLoggingInterceptor.Level.BODY);

        OkHttpClient okHttpClient = new OkHttpClient.Builder()
                .addInterceptor(loggingInterceptor)
                .build();

        Request request = new Request.Builder()
                .url("http://twitter.com")
                .build();

        Response response = okHttpClient.newCall(request).execute();

        Protocol protocol = response.protocol();

        assertEquals(Protocol.HTTP_2, protocol);
    }
}

junit.framework.AssertionFailedError: Expected :h2 Actual :http/1.1

JakeWharton commented 8 years ago

Do you have the ALPN jars added to your classpath? Or are you running the test on Android 5+?

On Wed, Jan 13, 2016, 4:15 PM Alexey Zakharov notifications@github.com wrote:

public class Http2Test {

@Test
public void isHttp2Transport() throws IOException {
    HttpLoggingInterceptor loggingInterceptor = new HttpLoggingInterceptor();
    loggingInterceptor.setLevel(HttpLoggingInterceptor.Level.BODY);

    OkHttpClient okHttpClient = new OkHttpClient.Builder()
            .addInterceptor(loggingInterceptor)
            .build();

    Request request = new Request.Builder()
            .url("http://twitter.com")
            .build();

    Response response = okHttpClient.newCall(request).execute();

    Protocol protocol = response.protocol();

    assertEquals(Protocol.HTTP_2, protocol);
}

}


—
Reply to this email directly or view it on GitHub
<https://github.com/square/okhttp/issues/2236>.
lexer commented 8 years ago

@JakeWharton Im running this as pure junit test. But I run into the issue when I was debugging Android requests on Android 5+ device.

lexer commented 8 years ago

@JakeWharton ok this pure unit test is not correct I guess. I will try to rewrite it to Android test and rerun

lexer commented 8 years ago

@JakeWharton I was probably wrong. When I run my test as Android Instrumentation it succeeded. Will do more research.

lexer commented 8 years ago

cc: @JakeWharton

Cool I think HTTP 2 protocol is set correctly for response. I run my test as integration test and everything is green.

I think something is probably wrong with how HttpLoggingInterceptor log things.

In all my tests: https://github.com/square/okhttp/blob/master/okhttp-logging-interceptor/src/main/java/okhttp3/logging/HttpLoggingInterceptor.java#L151

connection is always null. So this code block:

    Protocol protocol = connection != null ? connection.protocol() : Protocol.HTTP_1_1;
    String requestStartMessage =
        "--> " + request.method() + ' ' + request.url() + ' ' + protocol(protocol);

Will always return HTTP 1.1

01-14 17:16:06.100 18812-18838/com.github.lexer.http2test D/OkHttp: --> GET https://twitter.com/ HTTP/1.1
01-14 17:16:06.721 18812-18838/com.github.lexer.http2test D/OkHttp: <-- 200  https://twitter.com/ (620ms, -1-byte body)
01-14 17:16:06.722 18812-18838/com.github.lexer.http2test D/OkHttp: --> GET https://twitter.com/download HTTP/1.1
01-14 17:16:06.768 18812-18838/com.github.lexer.http2test D/OkHttp: <-- 200  https://twitter.com/download (45ms, -1-byte body)
lexer commented 8 years ago

@JakeWharton I will close this story and will open separate ticket for http logger.

lexer commented 8 years ago

2247 Response protocol is correct. This issue is invalid. Protocol is not correctly logged by HttpLoggingInterceptor.