diff options
7 files changed, 118 insertions, 18 deletions
@@ -32,6 +32,16 @@ license { ], } +java_defaults { + name: "okhttp_errorprone_defaults", + errorprone: { + javacflags: [ + "-Xep:InvalidTimeZoneID:WARN", + "-Xep:TryFailThrowable:WARN", + ], + }, +} + // The source files that contribute to Android's core library APIs. filegroup { name: "okhttp_api_files", @@ -49,6 +59,7 @@ nojarjar_visibility = [ // non-jarjar'd version of okhttp to compile the tests against java_library { name: "okhttp-nojarjar", + defaults: ["okhttp_errorprone_defaults"], visibility: nojarjar_visibility, srcs: [ "android/src/main/java/**/*.java", @@ -91,12 +102,14 @@ filegroup { java_library { name: "okhttp", + defaults: ["okhttp_errorprone_defaults"], visibility: [ "//art/build/apex", "//art/build/sdk", "//external/grpc-grpc-java/okhttp", "//external/robolectric-shadows", - "//libcore", + "//external/robolectric", + "//libcore:__subpackages__", "//packages/modules/ArtPrebuilt", ], srcs: [ @@ -125,6 +138,7 @@ java_library { visibility: [ "//art/build/sdk", "//external/robolectric-shadows", + "//external/robolectric", ], static_libs: [ "okhttp", @@ -139,8 +153,9 @@ java_library { // third-party or unbundled applications or libraries that require OkHttp. java_library { name: "okhttp-norepackage", + defaults: ["okhttp_errorprone_defaults"], host_supported: true, - + apex_available: ["com.android.adservices"], visibility: [ "//art/build/sdk", "//external/grpc-grpc-java/okhttp", @@ -164,6 +179,8 @@ java_library { // Build against a "core_current" as it cannot use "current" as it has to // build in manifests without frameworks/base. sdk_version: "core_current", + // Make sure that this will be added to the sdk snapshot for S. + min_sdk_version: "S", } // Generate Version.java based on the version number from pom.xml. @@ -192,6 +209,7 @@ java_library { java_library { name: "okhttp-tests-nojarjar", + defaults: ["okhttp_errorprone_defaults"], visibility: nojarjar_visibility, srcs: [ "android/test/java/**/*.java", diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockResponse.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockResponse.java index db68595..c46d96d 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockResponse.java +++ b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockResponse.java @@ -21,6 +21,7 @@ import com.squareup.okhttp.internal.framed.Settings; import com.squareup.okhttp.ws.WebSocketListener; import java.util.ArrayList; import java.util.List; +import java.util.function.Consumer; import java.util.concurrent.TimeUnit; import okio.Buffer; @@ -38,6 +39,7 @@ public final class MockResponse implements Cloneable { private TimeUnit throttlePeriodUnit = TimeUnit.SECONDS; private SocketPolicy socketPolicy = SocketPolicy.KEEP_OPEN; + private Consumer<SocketPolicy> socketShutdownListener = null; private long bodyDelayAmount = 0; private TimeUnit bodyDelayUnit = TimeUnit.MILLISECONDS; @@ -195,6 +197,28 @@ public final class MockResponse implements Cloneable { } /** + * Sets a listener that can wait until a socket is closed. This is important if the + * {@link SocketPolicy} gets set to something that shuts down the socket after a transaction and + * that socket may get reused in subsequent calls if they happen too fast. + * + * @param listener The listener that will be notified when a socket is closed. This could be an + * instance of {@link SocketShutdownListener}. + * + * @see SocketPolicy + * @see SocketShutdownListener + */ + public MockResponse setSocketShutdownListener(Consumer<SocketPolicy> listener) { + this.socketShutdownListener = listener; + return this; + } + + void notifyShutdown(SocketPolicy reason) { + if (socketShutdownListener != null) { + socketShutdownListener.accept(reason); + } + } + + /** * Throttles the response body writer to sleep for the given period after each * series of {@code bytesPerPeriod} bytes are written. Use this to simulate * network behavior. diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java index b15e92f..7a3deee 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java +++ b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java @@ -554,11 +554,14 @@ public final class MockWebServer implements TestRule { // See warnings associated with these socket policies in SocketPolicy. if (response.getSocketPolicy() == SocketPolicy.DISCONNECT_AT_END) { socket.close(); + response.notifyShutdown(SocketPolicy.DISCONNECT_AT_END); return false; } else if (response.getSocketPolicy() == SocketPolicy.SHUTDOWN_INPUT_AT_END) { socket.shutdownInput(); + response.notifyShutdown(SocketPolicy.SHUTDOWN_INPUT_AT_END); } else if (response.getSocketPolicy() == SocketPolicy.SHUTDOWN_OUTPUT_AT_END) { socket.shutdownOutput(); + response.notifyShutdown(SocketPolicy.SHUTDOWN_OUTPUT_AT_END); } sequenceNumber++; diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java index cdbbf72..6f4602f 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java +++ b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java @@ -26,8 +26,9 @@ package com.squareup.okhttp.mockwebserver; * server may not have had time to close the socket. The socket will be closed at an indeterminate * point before or during the second request. It may be closed after client has started sending the * request body. If a request body is not retryable then the client may fail the request, making - * client behavior non-deterministic. Add delays in the client to improve the chances that the - * server has closed the socket before follow up requests are made. + * client behavior non-deterministic. In order to prevent that {@link + * MockResponse#setSocketShutdownListener} can be used. Otherwise, add delays in the client to + * improve the chances that the server has closed the socket before follow up requests are made. */ public enum SocketPolicy { diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketShutdownListener.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketShutdownListener.java new file mode 100644 index 0000000..b83ce4a --- /dev/null +++ b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketShutdownListener.java @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2023 Google 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 com.squareup.okhttp.mockwebserver; + +import java.util.concurrent.CountDownLatch; +import java.util.function.Consumer; + +/** + * Utility class for registering as a listener to {@link MockResponse#setSocketShutdownListener}. + * + * An instance will wait for a single shutdown callback. Once triggered, the listener will stay in a + * "shutdown occurred" state. + * + * @see SocketPolicy + */ +public class SocketShutdownListener implements Consumer<SocketPolicy> { + + final CountDownLatch shutdownLatch = new CountDownLatch(1); + + @Override + public void accept(SocketPolicy reason) { + shutdownLatch.countDown(); + } + + public void waitForSocketShutdown() { + try { + shutdownLatch.await(); + } catch (InterruptedException e) { + } + } +} diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/ConnectionReuseTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/ConnectionReuseTest.java index f445dac..93219bf 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/ConnectionReuseTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/ConnectionReuseTest.java @@ -19,6 +19,7 @@ import com.squareup.okhttp.internal.SslContextBuilder; import com.squareup.okhttp.mockwebserver.MockResponse; import com.squareup.okhttp.mockwebserver.MockWebServer; import com.squareup.okhttp.mockwebserver.SocketPolicy; +import com.squareup.okhttp.mockwebserver.SocketShutdownListener; import com.squareup.okhttp.testing.RecordingHostnameVerifier; import java.util.Arrays; import java.util.concurrent.TimeUnit; @@ -166,8 +167,10 @@ public final class ConnectionReuseTest { } @Test public void staleConnectionNotReusedForNonIdempotentRequest() throws Exception { + SocketShutdownListener shutdownListener = new SocketShutdownListener(); server.enqueue(new MockResponse().setBody("a") - .setSocketPolicy(SocketPolicy.SHUTDOWN_OUTPUT_AT_END)); + .setSocketPolicy(SocketPolicy.SHUTDOWN_OUTPUT_AT_END) + .setSocketShutdownListener(shutdownListener)); server.enqueue(new MockResponse().setBody("b")); Request requestA = new Request.Builder() @@ -177,6 +180,8 @@ public final class ConnectionReuseTest { assertEquals("a", responseA.body().string()); assertEquals(0, server.takeRequest().getSequenceNumber()); + shutdownListener.waitForSocketShutdown(); + Request requestB = new Request.Builder() .url(server.url("/")) .post(RequestBody.create(MediaType.parse("text/plain"), "b")) diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java index a3e1450..d20ab9a 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/URLConnectionTest.java @@ -28,6 +28,7 @@ import com.squareup.okhttp.mockwebserver.MockResponse; import com.squareup.okhttp.mockwebserver.MockWebServer; import com.squareup.okhttp.mockwebserver.RecordedRequest; import com.squareup.okhttp.mockwebserver.SocketPolicy; +import com.squareup.okhttp.mockwebserver.SocketShutdownListener; import com.squareup.okhttp.testing.RecordingHostnameVerifier; import java.io.IOException; import java.io.InputStream; @@ -409,8 +410,10 @@ public final class URLConnectionTest { } private void testServerClosesOutput(SocketPolicy socketPolicy) throws Exception { + SocketShutdownListener shutdownListener = new SocketShutdownListener(); server.enqueue(new MockResponse().setBody("This connection won't pool properly") - .setSocketPolicy(socketPolicy)); + .setSocketPolicy(socketPolicy) + .setSocketShutdownListener(shutdownListener)); MockResponse responseAfter = new MockResponse().setBody("This comes after a busted connection"); server.enqueue(responseAfter); server.enqueue(responseAfter); // Enqueue 2x because the broken connection may be reused. @@ -420,9 +423,7 @@ public final class URLConnectionTest { assertContent("This connection won't pool properly", connection1); assertEquals(0, server.takeRequest().getSequenceNumber()); - // Give the server time to enact the socket policy if it's one that could happen after the - // client has received the response. - Thread.sleep(500); + shutdownListener.waitForSocketShutdown(); HttpURLConnection connection2 = client.open(server.getUrl("/b")); connection2.setReadTimeout(100); @@ -641,10 +642,12 @@ public final class URLConnectionTest { * https://github.com/square/okhttp/issues/515 */ @Test public void sslFallbackNotUsedWhenRecycledConnectionFails() throws Exception { + SocketShutdownListener shutdownListener = new SocketShutdownListener(); server.useHttps(sslContext.getSocketFactory(), false); server.enqueue(new MockResponse() .setBody("abc") - .setSocketPolicy(SocketPolicy.DISCONNECT_AT_END)); + .setSocketPolicy(SocketPolicy.DISCONNECT_AT_END) + .setSocketShutdownListener(shutdownListener)); server.enqueue(new MockResponse().setBody("def")); suppressTlsFallbackScsv(client.client()); @@ -652,8 +655,7 @@ public final class URLConnectionTest { assertContent("abc", client.open(server.getUrl("/"))); - // Give the server time to disconnect. - Thread.sleep(500); + shutdownListener.waitForSocketShutdown(); assertContent("def", client.open(server.getUrl("/"))); @@ -1273,9 +1275,11 @@ public final class URLConnectionTest { } @Test public void transparentGzipWorksAfterExceptionRecovery() throws Exception { + SocketShutdownListener shutdownListener = new SocketShutdownListener(); server.enqueue(new MockResponse() .setBody("a") - .setSocketPolicy(SHUTDOWN_INPUT_AT_END)); + .setSocketPolicy(SHUTDOWN_INPUT_AT_END) + .setSocketShutdownListener(shutdownListener)); server.enqueue(new MockResponse() .addHeader("Content-Encoding: gzip") .setBody(gzip("b"))); @@ -1283,8 +1287,7 @@ public final class URLConnectionTest { // Seed the pool with a bad connection. assertContent("a", client.open(server.getUrl("/"))); - // Give the server time to disconnect. - Thread.sleep(500); + shutdownListener.waitForSocketShutdown(); // This connection will need to be recovered. When it is, transparent gzip should still work! assertContent("b", client.open(server.getUrl("/"))); @@ -2674,14 +2677,16 @@ public final class URLConnectionTest { private void reusedConnectionFailsWithPost(TransferKind transferKind, int requestSize) throws Exception { - server.enqueue(new MockResponse().setBody("A").setSocketPolicy(DISCONNECT_AT_END)); + SocketShutdownListener shutdownListener = new SocketShutdownListener(); + server.enqueue(new MockResponse().setBody("A") + .setSocketPolicy(DISCONNECT_AT_END) + .setSocketShutdownListener(shutdownListener)); server.enqueue(new MockResponse().setBody("B")); server.enqueue(new MockResponse().setBody("C")); assertContent("A", client.open(server.getUrl("/a"))); - // Give the server time to disconnect. - Thread.sleep(500); + shutdownListener.waitForSocketShutdown(); // If the request body is larger than OkHttp's replay buffer, the failure may still occur. byte[] requestBody = new byte[requestSize]; |