From 2aa40bef4444012f9afc3c7fe060df44569ad022 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Mon, 17 Oct 2022 06:32:03 +1000 Subject: [PATCH 1/8] Ignore early hints for 4.9.x (#7444) (cherry picked from commit b565eecf167918253145bce2c348b0bb0ef58b0c) --- .../internal/http/CallServerInterceptor.kt | 20 +++++++++--- .../internal/http1/Http1ExchangeCodec.kt | 32 +++++++++++-------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt index 6801154b35c7..f67ff5654c62 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt @@ -19,8 +19,8 @@ import java.io.IOException import java.net.ProtocolException import okhttp3.Interceptor import okhttp3.Response -import okhttp3.internal.EMPTY_RESPONSE import okhttp3.internal.http2.ConnectionShutdownException +import okhttp3.internal.stripBody import okio.buffer /** This is the last interceptor in the chain. It makes a network call to the server. */ @@ -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, exchange)) { responseBuilder = exchange.readResponseHeaders(expectContinue = false)!! if (invokeStartEvent) { exchange.responseHeadersStart() @@ -137,7 +136,7 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { } if ((code == 204 || code == 205) && response.body?.contentLength() ?: -1L > 0L) { throw ProtocolException( - "HTTP $code had non-zero Content-Length: ${response.body?.contentLength()}") + "HTTP $code had non-zero Content-Length: ${response.body.contentLength()}") } return response } catch (e: IOException) { @@ -148,4 +147,15 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { throw e } } + + private fun shouldIgnoreAndWaitForRealResponse(code: Int, exchange: Exchange): 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 + + // Early Hints (103) but not supported yet in OkHttp + code == 103 -> 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..786ae6355434 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt @@ -26,15 +26,14 @@ import okhttp3.Request import okhttp3.Response import okhttp3.internal.EMPTY_HEADERS import okhttp3.internal.checkOffsetAndCount -import okhttp3.internal.connection.RealConnection import okhttp3.internal.discard import okhttp3.internal.headersContentLength import okhttp3.internal.http.ExchangeCodec import okhttp3.internal.http.RequestLine import okhttp3.internal.http.StatusLine -import okhttp3.internal.http.StatusLine.Companion.HTTP_CONTINUE import okhttp3.internal.http.promisesBody import okhttp3.internal.http.receiveHeaders +import okhttp3.internal.http.HTTP_CONTINUE import okhttp3.internal.skipAll import okio.Buffer import okio.BufferedSink @@ -64,8 +63,7 @@ import okio.Timeout class Http1ExchangeCodec( /** The client that configures this stream. May be null for HTTPS proxy tunnels. */ private val client: OkHttpClient?, - /** The connection that carries this stream. */ - override val connection: RealConnection, + override val carrier: ExchangeCodec.Carrier, private val source: BufferedSource, private val sink: BufferedSink ) : ExchangeCodec { @@ -90,7 +88,7 @@ class Http1ExchangeCodec( override fun createRequestBody(request: Request, contentLength: Long): Sink { return when { - request.body != null && request.body.isDuplex() -> throw ProtocolException( + request.body?.isDuplex() == true -> throw ProtocolException( "Duplex connections are not supported for HTTP/1") request.isChunked -> newChunkedSink() // Stream a request body of unknown length. contentLength != -1L -> newKnownLengthSink() // Stream a request body of a known length. @@ -101,7 +99,7 @@ class Http1ExchangeCodec( } override fun cancel() { - connection.cancel() + carrier.cancel() } /** @@ -115,7 +113,7 @@ class Http1ExchangeCodec( * the proper value. */ override fun writeRequestHeaders(request: Request) { - val requestLine = RequestLine.get(request, connection.route().proxy.type()) + val requestLine = RequestLine.get(request, carrier.route.proxy.type()) writeRequest(request.headers, requestLine) } @@ -184,6 +182,7 @@ class Http1ExchangeCodec( .code(statusLine.code) .message(statusLine.message) .headers(headersReader.readHeaders()) + .trailers { error("trailers not available") } return when { expectContinue && statusLine.code == HTTP_CONTINUE -> { @@ -193,6 +192,11 @@ class Http1ExchangeCodec( state = STATE_READ_RESPONSE_HEADERS responseBuilder } + statusLine.code == HTTP_EARLY_HINTS -> { + // Early Hints will mean a second header are coming. + state = STATE_READ_RESPONSE_HEADERS + responseBuilder + } else -> { state = STATE_OPEN_RESPONSE_BODY responseBuilder @@ -200,7 +204,7 @@ class Http1ExchangeCodec( } } catch (e: EOFException) { // Provide more context if the server ends the stream before sending a response. - val address = connection.route().address.url.redact() + val address = carrier.route.address.url.redact() throw IOException("unexpected end of stream on $address", e) } } @@ -232,7 +236,7 @@ class Http1ExchangeCodec( private fun newUnknownLengthSource(): Source { check(state == STATE_OPEN_RESPONSE_BODY) { "state: $state" } state = STATE_READING_RESPONSE_BODY - connection.noNewExchanges() + carrier.noNewExchanges() return UnknownLengthSource() } @@ -332,7 +336,7 @@ class Http1ExchangeCodec( return try { source.read(sink, byteCount) } catch (e: IOException) { - connection.noNewExchanges() + carrier.noNewExchanges() responseBodyComplete() throw e } @@ -369,7 +373,7 @@ class Http1ExchangeCodec( val read = super.read(sink, minOf(bytesRemaining, byteCount)) if (read == -1L) { - connection.noNewExchanges() // The server didn't supply the promised content length. + carrier.noNewExchanges() // The server didn't supply the promised content length. val e = ProtocolException("unexpected end of stream") responseBodyComplete() throw e @@ -387,7 +391,7 @@ class Http1ExchangeCodec( if (bytesRemaining != 0L && !discard(ExchangeCodec.DISCARD_STREAM_TIMEOUT_MILLIS, MILLISECONDS)) { - connection.noNewExchanges() // Unread bytes remain on the stream. + carrier.noNewExchanges() // Unread bytes remain on the stream. responseBodyComplete() } @@ -413,7 +417,7 @@ class Http1ExchangeCodec( val read = super.read(sink, minOf(byteCount, bytesRemainingInChunk)) if (read == -1L) { - connection.noNewExchanges() // The server didn't supply the promised chunk length. + carrier.noNewExchanges() // The server didn't supply the promised chunk length. val e = ProtocolException("unexpected end of stream") responseBodyComplete() throw e @@ -450,7 +454,7 @@ class Http1ExchangeCodec( if (closed) return if (hasMoreChunks && !discard(ExchangeCodec.DISCARD_STREAM_TIMEOUT_MILLIS, MILLISECONDS)) { - connection.noNewExchanges() // Unread bytes remain on the stream. + carrier.noNewExchanges() // Unread bytes remain on the stream. responseBodyComplete() } closed = true From b636618cdf5177c0164248b1d25ed4082baf9398 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Thu, 5 Jan 2023 13:33:39 +1000 Subject: [PATCH 2/8] Expand 103 handling to other non-specific 1XX messages. (#7629) (cherry picked from commit 7e4576ccc2a9a819a7ecb9b38d6f5417f4eb58fb) --- .../kotlin/okhttp3/internal/http/CallServerInterceptor.kt | 6 ++++-- .../kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt index f67ff5654c62..469f5aba3afb 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt @@ -153,8 +153,10 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { // actual response status. code == 100 -> true - // Early Hints (103) but not supported yet in OkHttp - code == 103 -> 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 786ae6355434..70ee1e90fb0f 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt @@ -34,6 +34,7 @@ import okhttp3.internal.http.StatusLine import okhttp3.internal.http.promisesBody import okhttp3.internal.http.receiveHeaders import okhttp3.internal.http.HTTP_CONTINUE +import okhttp3.internal.http.HTTP_EARLY_HINTS import okhttp3.internal.skipAll import okio.Buffer import okio.BufferedSink @@ -192,8 +193,9 @@ class Http1ExchangeCodec( state = STATE_READ_RESPONSE_HEADERS responseBuilder } - statusLine.code == HTTP_EARLY_HINTS -> { - // Early Hints will mean a second header are coming. + 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 } From 5b9c6105387ae3db8d7e6da117243208d342ffcf Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Thu, 5 Jan 2023 22:15:23 +1000 Subject: [PATCH 3/8] Cleanup --- .../internal/http/CallServerInterceptor.kt | 8 +++--- .../internal/http1/Http1ExchangeCodec.kt | 28 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt index 469f5aba3afb..32c21282da01 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http/CallServerInterceptor.kt @@ -19,8 +19,8 @@ import java.io.IOException import java.net.ProtocolException import okhttp3.Interceptor import okhttp3.Response +import okhttp3.internal.EMPTY_RESPONSE import okhttp3.internal.http2.ConnectionShutdownException -import okhttp3.internal.stripBody import okio.buffer /** This is the last interceptor in the chain. It makes a network call to the server. */ @@ -104,7 +104,7 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { .build() var code = response.code - if (shouldIgnoreAndWaitForRealResponse(code, exchange)) { + if (shouldIgnoreAndWaitForRealResponse(code)) { responseBuilder = exchange.readResponseHeaders(expectContinue = false)!! if (invokeStartEvent) { exchange.responseHeadersStart() @@ -136,7 +136,7 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { } if ((code == 204 || code == 205) && response.body?.contentLength() ?: -1L > 0L) { throw ProtocolException( - "HTTP $code had non-zero Content-Length: ${response.body.contentLength()}") + "HTTP $code had non-zero Content-Length: ${response.body?.contentLength()}") } return response } catch (e: IOException) { @@ -148,7 +148,7 @@ class CallServerInterceptor(private val forWebSocket: Boolean) : Interceptor { } } - private fun shouldIgnoreAndWaitForRealResponse(code: Int, exchange: Exchange): Boolean = when { + 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 diff --git a/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt b/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt index 70ee1e90fb0f..ddefea428836 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http1/Http1ExchangeCodec.kt @@ -26,15 +26,15 @@ import okhttp3.Request import okhttp3.Response import okhttp3.internal.EMPTY_HEADERS import okhttp3.internal.checkOffsetAndCount +import okhttp3.internal.connection.RealConnection import okhttp3.internal.discard import okhttp3.internal.headersContentLength import okhttp3.internal.http.ExchangeCodec import okhttp3.internal.http.RequestLine import okhttp3.internal.http.StatusLine +import okhttp3.internal.http.StatusLine.Companion.HTTP_CONTINUE import okhttp3.internal.http.promisesBody import okhttp3.internal.http.receiveHeaders -import okhttp3.internal.http.HTTP_CONTINUE -import okhttp3.internal.http.HTTP_EARLY_HINTS import okhttp3.internal.skipAll import okio.Buffer import okio.BufferedSink @@ -64,7 +64,8 @@ import okio.Timeout class Http1ExchangeCodec( /** The client that configures this stream. May be null for HTTPS proxy tunnels. */ private val client: OkHttpClient?, - override val carrier: ExchangeCodec.Carrier, + /** The connection that carries this stream. */ + override val connection: RealConnection, private val source: BufferedSource, private val sink: BufferedSink ) : ExchangeCodec { @@ -89,7 +90,7 @@ class Http1ExchangeCodec( override fun createRequestBody(request: Request, contentLength: Long): Sink { return when { - request.body?.isDuplex() == true -> throw ProtocolException( + request.body != null && request.body.isDuplex() -> throw ProtocolException( "Duplex connections are not supported for HTTP/1") request.isChunked -> newChunkedSink() // Stream a request body of unknown length. contentLength != -1L -> newKnownLengthSink() // Stream a request body of a known length. @@ -100,7 +101,7 @@ class Http1ExchangeCodec( } override fun cancel() { - carrier.cancel() + connection.cancel() } /** @@ -114,7 +115,7 @@ class Http1ExchangeCodec( * the proper value. */ override fun writeRequestHeaders(request: Request) { - val requestLine = RequestLine.get(request, carrier.route.proxy.type()) + val requestLine = RequestLine.get(request, connection.route().proxy.type()) writeRequest(request.headers, requestLine) } @@ -183,7 +184,6 @@ class Http1ExchangeCodec( .code(statusLine.code) .message(statusLine.message) .headers(headersReader.readHeaders()) - .trailers { error("trailers not available") } return when { expectContinue && statusLine.code == HTTP_CONTINUE -> { @@ -206,7 +206,7 @@ class Http1ExchangeCodec( } } catch (e: EOFException) { // Provide more context if the server ends the stream before sending a response. - val address = carrier.route.address.url.redact() + val address = connection.route().address.url.redact() throw IOException("unexpected end of stream on $address", e) } } @@ -238,7 +238,7 @@ class Http1ExchangeCodec( private fun newUnknownLengthSource(): Source { check(state == STATE_OPEN_RESPONSE_BODY) { "state: $state" } state = STATE_READING_RESPONSE_BODY - carrier.noNewExchanges() + connection.noNewExchanges() return UnknownLengthSource() } @@ -338,7 +338,7 @@ class Http1ExchangeCodec( return try { source.read(sink, byteCount) } catch (e: IOException) { - carrier.noNewExchanges() + connection.noNewExchanges() responseBodyComplete() throw e } @@ -375,7 +375,7 @@ class Http1ExchangeCodec( val read = super.read(sink, minOf(bytesRemaining, byteCount)) if (read == -1L) { - carrier.noNewExchanges() // The server didn't supply the promised content length. + connection.noNewExchanges() // The server didn't supply the promised content length. val e = ProtocolException("unexpected end of stream") responseBodyComplete() throw e @@ -393,7 +393,7 @@ class Http1ExchangeCodec( if (bytesRemaining != 0L && !discard(ExchangeCodec.DISCARD_STREAM_TIMEOUT_MILLIS, MILLISECONDS)) { - carrier.noNewExchanges() // Unread bytes remain on the stream. + connection.noNewExchanges() // Unread bytes remain on the stream. responseBodyComplete() } @@ -419,7 +419,7 @@ class Http1ExchangeCodec( val read = super.read(sink, minOf(byteCount, bytesRemainingInChunk)) if (read == -1L) { - carrier.noNewExchanges() // The server didn't supply the promised chunk length. + connection.noNewExchanges() // The server didn't supply the promised chunk length. val e = ProtocolException("unexpected end of stream") responseBodyComplete() throw e @@ -456,7 +456,7 @@ class Http1ExchangeCodec( if (closed) return if (hasMoreChunks && !discard(ExchangeCodec.DISCARD_STREAM_TIMEOUT_MILLIS, MILLISECONDS)) { - carrier.noNewExchanges() // Unread bytes remain on the stream. + connection.noNewExchanges() // Unread bytes remain on the stream. responseBodyComplete() } closed = true From f0753d9f56e483afd72f686c380e6343fa1a5ab8 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Fri, 6 Jan 2023 17:55:40 +1000 Subject: [PATCH 4/8] Add a test of 103 handling --- .../kotlin/okhttp3/OkHttpClientTestRule.kt | 6 +++ .../okhttp3/InformationalResponseCodeTest.kt | 41 +++++++++++++++++++ 2 files changed, 47 insertions(+) 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/test/java/okhttp3/InformationalResponseCodeTest.kt b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt new file mode 100644 index 000000000000..d7b6f99f05b6 --- /dev/null +++ b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt @@ -0,0 +1,41 @@ +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() { + // Enable 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) + } +} \ No newline at end of file From 519be5147717269fadabfcf786867efa8ba9f11b Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Fri, 6 Jan 2023 17:57:33 +1000 Subject: [PATCH 5/8] Add copyright --- .../okhttp3/InformationalResponseCodeTest.kt | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt index d7b6f99f05b6..0ba9ad9ecc49 100644 --- a/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt +++ b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt @@ -1,3 +1,18 @@ +/* + * 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 @@ -38,4 +53,4 @@ class InformationalResponseCodeTest { val incomingHeaders = "<< 0x00000003\\s+\\d+\\s+HEADERS\\s+END_HEADERS".toRegex() assertThat(clientTestRule.eventsList().filter { it.matches(incomingHeaders) }).hasSize(2) } -} \ No newline at end of file +} From 7790219d8a45ebc7e1c4558cf612348010793015 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Fri, 6 Jan 2023 17:59:22 +1000 Subject: [PATCH 6/8] Add copyright --- okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt index 0ba9ad9ecc49..46bcd26d86d1 100644 --- a/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt +++ b/okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt @@ -34,7 +34,7 @@ class InformationalResponseCodeTest { @Test fun test103() { - // Enable curl so cloudflare will send a 103 + // 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") From cd30b10fb8f3c9822cae85d748eb21914f29df09 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Fri, 6 Jan 2023 18:01:50 +1000 Subject: [PATCH 7/8] Fix cache test for 102/103 (#7633) (cherry picked from commit 930f1381253a157dad0074996c972bb8a95d90fb) --- okhttp/src/test/java/okhttp3/CacheTest.java | 37 +++++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/okhttp/src/test/java/okhttp3/CacheTest.java b/okhttp/src/test/java/okhttp3/CacheTest.java index e95e090522a8..e59d7af9aa30 100644 --- a/okhttp/src/test/java/okhttp3/CacheTest.java +++ b/okhttp/src/test/java/okhttp3/CacheTest.java @@ -104,7 +104,6 @@ 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); assertCached(true, 200); assertCached(false, 201); assertCached(false, 202); @@ -151,7 +150,12 @@ public final class CacheTest { assertCached(false, 506); } - private void assertCached(boolean shouldPut, int responseCode) throws Exception { + @Test public void responseCachingWith1xxInformationalResponse() throws Exception { + assertSubsequentResponseCached( 102, 200); + assertSubsequentResponseCached( 103, 200); + } + + private void assertCached(boolean shouldWriteToCache, int responseCode) throws Exception { int expectedResponseCode = responseCode; server = new MockWebServer(); @@ -193,7 +197,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 { @@ -202,6 +206,33 @@ private void assertCached(boolean shouldPut, int responseCode) throws Exception server.shutdown(); // tearDown() isn't sufficient; this test starts multiple servers } + private void assertSubsequentResponseCached(int initialResponseCode, int finalResponseCode) throws Exception { + server = new MockWebServer(); + MockResponse.Builder builder = new MockResponse.Builder() + .addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS)) + .addHeader("Expires: " + formatDate(1, TimeUnit.HOURS)) + .code(finalResponseCode) + .body("ABCDE") + .addInformationalResponse(new MockResponse(initialResponseCode)); + + server.enqueue(builder.build()); + server.start(); + + Request request = new Request.Builder() + .url(server.url("/")) + .build(); + Response response = client.newCall(request).execute(); + assertThat(response.code()).isEqualTo(finalResponseCode); + + // Exhaust the content stream. + response.body().string(); + + Response cached = cacheGet(cache, request); + assertThat(cached).isNotNull(); + cached.body().close(); + server.shutdown(); // tearDown() isn't sufficient; this test starts multiple servers + } + @Test public void responseCachingAndInputStreamSkipWithFixedLength() throws IOException { testResponseCaching(TransferKind.FIXED_LENGTH); } From 1c8bf3100fed79a879af91da041ff8171634a298 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Fri, 6 Jan 2023 18:26:11 +1000 Subject: [PATCH 8/8] Disable test --- okhttp/src/test/java/okhttp3/CacheTest.java | 36 +++------------------ 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/okhttp/src/test/java/okhttp3/CacheTest.java b/okhttp/src/test/java/okhttp3/CacheTest.java index e59d7af9aa30..6a47623a3f87 100644 --- a/okhttp/src/test/java/okhttp3/CacheTest.java +++ b/okhttp/src/test/java/okhttp3/CacheTest.java @@ -104,6 +104,10 @@ public final class CacheTest { // We can't test 100 because it's not really a response. // assertCached(false, 100); assertCached(false, 101); + // 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); @@ -150,11 +154,6 @@ public final class CacheTest { assertCached(false, 506); } - @Test public void responseCachingWith1xxInformationalResponse() throws Exception { - assertSubsequentResponseCached( 102, 200); - assertSubsequentResponseCached( 103, 200); - } - private void assertCached(boolean shouldWriteToCache, int responseCode) throws Exception { int expectedResponseCode = responseCode; @@ -206,33 +205,6 @@ private void assertCached(boolean shouldWriteToCache, int responseCode) throws E server.shutdown(); // tearDown() isn't sufficient; this test starts multiple servers } - private void assertSubsequentResponseCached(int initialResponseCode, int finalResponseCode) throws Exception { - server = new MockWebServer(); - MockResponse.Builder builder = new MockResponse.Builder() - .addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS)) - .addHeader("Expires: " + formatDate(1, TimeUnit.HOURS)) - .code(finalResponseCode) - .body("ABCDE") - .addInformationalResponse(new MockResponse(initialResponseCode)); - - server.enqueue(builder.build()); - server.start(); - - Request request = new Request.Builder() - .url(server.url("/")) - .build(); - Response response = client.newCall(request).execute(); - assertThat(response.code()).isEqualTo(finalResponseCode); - - // Exhaust the content stream. - response.body().string(); - - Response cached = cacheGet(cache, request); - assertThat(cached).isNotNull(); - cached.body().close(); - server.shutdown(); // tearDown() isn't sufficient; this test starts multiple servers - } - @Test public void responseCachingAndInputStreamSkipWithFixedLength() throws IOException { testResponseCaching(TransferKind.FIXED_LENGTH); }