Skip to content

Commit

Permalink
[4.x] Backport 1XX handling from master (#7634)
Browse files Browse the repository at this point in the history
* Ignore early hints for 4.9.x (#7444)

(cherry picked from commit b565eec)

* Expand 103 handling to other non-specific 1XX messages. (#7629)

(cherry picked from commit 7e4576c)

* Cleanup

* Add a test of 103 handling

* Add copyright

* Add copyright

* Fix cache test for 102/103 (#7633)

(cherry picked from commit 930f138)

* Disable test
  • Loading branch information
yschimke committed Jan 12, 2023
1 parent 81d3411 commit 949262e
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,12 @@ class OkHttpClientTestRule : TestRule {
}
}

fun eventsList(): List<String> {
return synchronized(clientEventsList) {
clientEventsList.toList()
}
}

companion object {
/**
* A network that resolves only one IP address per host. Use this when testing route selection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions okhttp/src/test/java/okhttp3/CacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down
56 changes: 56 additions & 0 deletions okhttp/src/test/java/okhttp3/InformationalResponseCodeTest.kt
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit 949262e

Please sign in to comment.