From 949262e9fbafc6c957b6b7215da03ae2247a08eb Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Thu, 12 Jan 2023 22:10:39 +1000 Subject: [PATCH] [4.x] Backport 1XX handling from master (#7634) * Ignore early hints for 4.9.x (#7444) (cherry picked from commit b565eecf167918253145bce2c348b0bb0ef58b0c) * Expand 103 handling to other non-specific 1XX messages. (#7629) (cherry picked from commit 7e4576ccc2a9a819a7ecb9b38d6f5417f4eb58fb) * Cleanup * Add a test of 103 handling * Add copyright * Add copyright * Fix cache test for 102/103 (#7633) (cherry picked from commit 930f1381253a157dad0074996c972bb8a95d90fb) * Disable test --- .../kotlin/okhttp3/OkHttpClientTestRule.kt | 6 ++ .../internal/http/CallServerInterceptor.kt | 18 +++++- .../internal/http1/Http1ExchangeCodec.kt | 6 ++ okhttp/src/test/java/okhttp3/CacheTest.java | 9 ++- .../okhttp3/InformationalResponseCodeTest.kt | 56 +++++++++++++++++++ 5 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt diff --git a/okhttp-testing-support/src/main/kotlin/okhttp3/OkHttpClientTestRule.kt b/okhttp-testing-support/src/main/kotlin/okhttp3/OkHttpClientTestRule.kt index e0174acb4caa..1c4c5d2dd945 100644 --- a/okhttp-testing-support/src/main/kotlin/okhttp3/OkHttpClientTestRule.kt +++ b/okhttp-testing-support/src/main/kotlin/okhttp3/OkHttpClientTestRule.kt @@ -264,6 +264,12 @@ class OkHttpClientTestRule : TestRule { } } + fun eventsList(): List { + return synchronized(clientEventsList) { + clientEventsList.toList() + } + } + companion object { /** * A network that resolves only one IP address per host. Use this when testing route selection diff --git a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt index 6801154b35c7..32c21282da01 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt @@ -103,9 +103,8 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { .receivedResponseAtMillis(System.currentTimeMillis()) .build() var code = response.code - if (code == 100) { - // Server sent a 100-continue even though we did not request one. Try again to read the - // actual response status. + + if (shouldIgnoreAndWaitForRealResponse(code)) { responseBuilder = exchange.readResponseHeaders(expectContinue = false)!! if (invokeStartEvent) { exchange.responseHeadersStart() @@ -148,4 +147,17 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { throw e } } + + private fun shouldIgnoreAndWaitForRealResponse(code: Int): Boolean = when { + // Server sent a 100-continue even though we did not request one. Try again to read the + // actual response status. + code == 100 -> true + + // Handle Processing (102) & Early Hints (103) and any new codes without failing + // 100 and 101 are the exceptions with different meanings + // But Early Hints not currently exposed + code in (102 until 200) -> true + + else -> false + } } diff --git a/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt b/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt index 87f516a69a75..ddefea428836 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt @@ -193,6 +193,12 @@ class Http1ExchangeCodec( state = STATE_READ_RESPONSE_HEADERS responseBuilder } + statusLine.code in (102 until 200) -> { + // Processing and Early Hints will mean a second headers are coming. + // Treat others the same for now + state = STATE_READ_RESPONSE_HEADERS + responseBuilder + } else -> { state = STATE_OPEN_RESPONSE_BODY responseBuilder diff --git a/okhttp/src/test/java/okhttp3/CacheTest.java b/okhttp/src/test/java/okhttp3/CacheTest.java index e95e090522a8..6a47623a3f87 100644 --- a/okhttp/src/test/java/okhttp3/CacheTest.java +++ b/okhttp/src/test/java/okhttp3/CacheTest.java @@ -104,7 +104,10 @@ public final class CacheTest { // We can't test 100 because it's not really a response. // assertCached(false, 100); assertCached(false, 101); - assertCached(false, 102); + // We don't test 102 or 103 because it will break on the informational response with + // mockwebserver. +// assertSubsequentResponseCached( 102, 200); +// assertSubsequentResponseCached( 103, 200); assertCached(true, 200); assertCached(false, 201); assertCached(false, 202); @@ -151,7 +154,7 @@ public final class CacheTest { assertCached(false, 506); } - private void assertCached(boolean shouldPut, int responseCode) throws Exception { + private void assertCached(boolean shouldWriteToCache, int responseCode) throws Exception { int expectedResponseCode = responseCode; server = new MockWebServer(); @@ -193,7 +196,7 @@ private void assertCached(boolean shouldPut, int responseCode) throws Exception response.body().string(); Response cached = cacheGet(cache, request); - if (shouldPut) { + if (shouldWriteToCache) { assertThat(cached).isNotNull(); cached.body().close(); } else { diff --git a/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt new file mode 100644 index 000000000000..46bcd26d86d1 --- /dev/null +++ b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2023 Block, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package okhttp3 + +import okhttp3.testing.PlatformRule +import org.assertj.core.api.Assertions.assertThat +import org.junit.Ignore +import org.junit.Rule +import org.junit.Test + +@Ignore("For manual testing only") +class InformationalResponseCodeTest { + @JvmField @Rule + val platform = PlatformRule() + + @JvmField @Rule val clientTestRule = OkHttpClientTestRule().apply { + recordFrames = true + } + + private var client = clientTestRule.newClient() + + @Test + fun test103() { + // Pretend we are curl so cloudflare will send a 103 + val request = Request.Builder() + .url("https://tradingstrategy.ai") + .header("user-agent", "curl/7.85.0") + .build() + + val response = client.newCall(request).execute() + + assertThat(response.code).isEqualTo(200) + assertThat(response.protocol).isEqualTo(Protocol.HTTP_2) + response.close() + + val outgoingHeaders = ">> 0x00000003\\s+\\d+\\s+HEADERS\\s+END_STREAM\\|END_HEADERS".toRegex() + assertThat(clientTestRule.eventsList().filter { it.matches(outgoingHeaders) }).hasSize(1) + + // Confirm we get the informational response and final response headers. + val incomingHeaders = "<< 0x00000003\\s+\\d+\\s+HEADERS\\s+END_HEADERS".toRegex() + assertThat(clientTestRule.eventsList().filter { it.matches(incomingHeaders) }).hasSize(2) + } +}