diff --git a/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java b/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java index 283b43a24..29b7fa401 100644 --- a/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java +++ b/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Deque; import java.util.List; +import okhttp3.internal.DoubleInetAddressDns; import okhttp3.internal.SingleInetAddressDns; import okhttp3.internal.tls.SslClient; import okhttp3.mockwebserver.MockResponse; @@ -34,6 +35,7 @@ import org.junit.Rule; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; @@ -44,12 +46,13 @@ public final class EventListenerTest { @Rule public final MockWebServer server = new MockWebServer(); private OkHttpClient client; + private final SingleInetAddressDns singleDns = new SingleInetAddressDns(); private final RecordingEventListener listener = new RecordingEventListener(); private final SslClient sslClient = SslClient.localhost(); @Before public void setUp() { client = new OkHttpClient.Builder() - .dns(new SingleInetAddressDns()) + .dns(singleDns) .eventListener(listener) .build(); } @@ -95,17 +98,70 @@ public final class EventListenerTest { assertEquals(200, response.code()); response.body().close(); - DnsStart dnsStart = listener.findNextEvent(DnsStart.class); + DnsStart dnsStart = listener.removeUpToEvent(DnsStart.class); assertSame(call, dnsStart.call); assertEquals("localhost", dnsStart.domainName); - DnsEnd dnsEnd = listener.findNextEvent(DnsEnd.class); + DnsEnd dnsEnd = listener.removeUpToEvent(DnsEnd.class); assertSame(call, dnsEnd.call); assertEquals("localhost", dnsEnd.domainName); assertEquals(1, dnsEnd.inetAddressList.size()); assertNull(dnsEnd.throwable); } + @Test public void noDnsLookupOnPooledConnection() throws IOException { + server.enqueue(new MockResponse()); + server.enqueue(new MockResponse()); + + // Seed the pool. + Call call1 = client.newCall(new Request.Builder() + .url(server.url("/")) + .build()); + Response response1 = call1.execute(); + assertEquals(200, response1.code()); + response1.body().close(); + + listener.clearAllEvents(); + + Call call2 = client.newCall(new Request.Builder() + .url(server.url("/")) + .build()); + Response response2 = call2.execute(); + assertEquals(200, response2.code()); + response2.body().close(); + + List> recordedEvents = listener.recordedEventTypes(); + assertFalse(recordedEvents.contains(DnsStart.class)); + assertFalse(recordedEvents.contains(DnsEnd.class)); + } + + @Test public void multipleDnsLookupsForSingleCall() throws IOException { + server.enqueue(new MockResponse() + .setResponseCode(301) + .setHeader("Location", "http://www.fakeurl:" + server.getPort())); + server.enqueue(new MockResponse()); + + FakeDns dns = new FakeDns(); + dns.set("fakeurl", singleDns.lookup(server.getHostName())); + dns.set("www.fakeurl", singleDns.lookup(server.getHostName())); + + client = client.newBuilder() + .dns(dns) + .build(); + + Call call = client.newCall(new Request.Builder() + .url("http://fakeurl:" + server.getPort()) + .build()); + Response response = call.execute(); + assertEquals(200, response.code()); + response.body().close(); + + listener.removeUpToEvent(DnsStart.class); + listener.removeUpToEvent(DnsEnd.class); + listener.removeUpToEvent(DnsStart.class); + listener.removeUpToEvent(DnsEnd.class); + } + @Test public void failedDnsLookup() { client = client.newBuilder() .dns(new FakeDns()) @@ -119,9 +175,9 @@ public final class EventListenerTest { } catch (IOException expected) { } - listener.findNextEvent(DnsStart.class); + listener.removeUpToEvent(DnsStart.class); - DnsEnd dnsEnd = listener.findNextEvent(DnsEnd.class); + DnsEnd dnsEnd = listener.removeUpToEvent(DnsEnd.class); assertSame(call, dnsEnd.call); assertEquals("fakeurl", dnsEnd.domainName); assertNull(dnsEnd.inetAddressList); @@ -147,9 +203,9 @@ public final class EventListenerTest { } catch (IOException expected) { } - listener.findNextEvent(DnsStart.class); + listener.removeUpToEvent(DnsStart.class); - DnsEnd dnsEnd = listener.findNextEvent(DnsEnd.class); + DnsEnd dnsEnd = listener.removeUpToEvent(DnsEnd.class); assertSame(call, dnsEnd.call); assertEquals("fakeurl", dnsEnd.domainName); assertNull(dnsEnd.inetAddressList); @@ -167,10 +223,10 @@ public final class EventListenerTest { assertEquals(200, response.code()); response.body().close(); - SecureConnectStart secureStart = listener.findNextEvent(SecureConnectStart.class); + SecureConnectStart secureStart = listener.removeUpToEvent(SecureConnectStart.class); assertSame(call, secureStart.call); - SecureConnectEnd secureEnd = listener.findNextEvent(SecureConnectEnd.class); + SecureConnectEnd secureEnd = listener.removeUpToEvent(SecureConnectEnd.class); assertSame(call, secureEnd.call); assertNotNull(secureEnd.handshake); assertNull(secureEnd.throwable); @@ -190,10 +246,10 @@ public final class EventListenerTest { } catch (IOException expected) { } - SecureConnectStart secureStart = listener.findNextEvent(SecureConnectStart.class); + SecureConnectStart secureStart = listener.removeUpToEvent(SecureConnectStart.class); assertSame(call, secureStart.call); - SecureConnectEnd secureEnd = listener.findNextEvent(SecureConnectEnd.class); + SecureConnectEnd secureEnd = listener.removeUpToEvent(SecureConnectEnd.class); assertSame(call, secureEnd.call); assertNull(secureEnd.handshake); assertTrue(secureEnd.throwable instanceof IOException); @@ -216,15 +272,70 @@ public final class EventListenerTest { assertEquals(200, response.code()); response.body().close(); - SecureConnectStart secureStart = listener.findNextEvent(SecureConnectStart.class); + SecureConnectStart secureStart = listener.removeUpToEvent(SecureConnectStart.class); assertSame(call, secureStart.call); - SecureConnectEnd secureEnd = listener.findNextEvent(SecureConnectEnd.class); + SecureConnectEnd secureEnd = listener.removeUpToEvent(SecureConnectEnd.class); assertSame(call, secureEnd.call); assertNotNull(secureEnd.handshake); assertNull(secureEnd.throwable); } + @Test public void multipleSecureConnectsForSingleCall() throws IOException { + enableTls(false); + server.enqueue(new MockResponse() + .setSocketPolicy(SocketPolicy.FAIL_HANDSHAKE)); + server.enqueue(new MockResponse()); + + client = client.newBuilder() + .dns(new DoubleInetAddressDns()) + .build(); + + Call call = client.newCall(new Request.Builder() + .url(server.url("/")) + .build()); + Response response = call.execute(); + assertEquals(200, response.code()); + response.body().close(); + + listener.removeUpToEvent(SecureConnectStart.class); + listener.removeUpToEvent(SecureConnectEnd.class); + + listener.removeUpToEvent(SecureConnectStart.class); + listener.removeUpToEvent(SecureConnectEnd.class); + } + + @Test public void noSecureConnectsOnPooledConnection() throws IOException { + enableTls(false); + server.enqueue(new MockResponse()); + server.enqueue(new MockResponse()); + + client = client.newBuilder() + .dns(new DoubleInetAddressDns()) + .build(); + + // Seed the pool. + Call call1 = client.newCall(new Request.Builder() + .url(server.url("/")) + .build()); + Response response1 = call1.execute(); + assertEquals(200, response1.code()); + response1.body().close(); + + listener.clearAllEvents(); + + Call call2 = client.newCall(new Request.Builder() + .url(server.url("/")) + .build()); + Response response2 = call2.execute(); + assertEquals(200, response2.code()); + response2.body().close(); + + List> recordedEvents = listener.recordedEventTypes(); + assertFalse(recordedEvents.contains(SecureConnectStart.class)); + assertFalse(recordedEvents.contains(SecureConnectEnd.class)); + } + private void enableTls(boolean tunnelProxy) { client = client.newBuilder() .sslSocketFactory(sslClient.socketFactory, sslClient.trustManager) @@ -280,14 +391,16 @@ public final class EventListenerTest { static final class RecordingEventListener extends EventListener { final Deque eventSequence = new ArrayDeque<>(); - T findNextEvent(Class eventClass) { + /** + * Removes recorded events up to (and including) an event is found whose class equals + * {@code eventClass} and returns it. + */ + T removeUpToEvent(Class eventClass) { Object event = eventSequence.poll(); while (event != null && !eventClass.isInstance(event)) { event = eventSequence.poll(); } - if (event == null) { - fail("Expected event type: " + eventClass.getName()); - } + if (event == null) throw new AssertionError(); return (T) event; } @@ -299,6 +412,10 @@ public final class EventListenerTest { return eventTypes; } + void clearAllEvents() { + eventSequence.clear(); + } + @Override public void dnsStart(Call call, String domainName) { eventSequence.offer(new DnsStart(call, domainName)); } diff --git a/okhttp/src/main/java/okhttp3/EventListener.java b/okhttp/src/main/java/okhttp3/EventListener.java index 7f8c55392..3c64409d3 100644 --- a/okhttp/src/main/java/okhttp3/EventListener.java +++ b/okhttp/src/main/java/okhttp3/EventListener.java @@ -34,13 +34,23 @@ public abstract class EventListener { public void fetchStart(Call call) { } - /** Invoked just prior to a DNS lookup. See {@link Dns#lookup(String)}. */ + /** + * Invoked just prior to a DNS lookup. See {@link Dns#lookup(String)}. + * + *

This can be invoked more than 1 time for a single {@link Call}. For example, if the response + * to the {@link Call#request()} is a redirect to a different host. + * + *

If the {@link Call} is able to reuse an existing pooled connection, this method will not be + * invoked. See {@link ConnectionPool}. + */ public void dnsStart(Call call, String domainName) { } /** * Invoked immediately after a DNS lookup. * + *

This method is always invoked after {@link #dnsStart(Call, String)}. + * *

{@code inetAddressList} will be non-null and {@code throwable} will be null in the case of a * successful DNS lookup. * @@ -57,7 +67,14 @@ public abstract class EventListener { /** * Invoked just prior to initiating a TLS connection. * - *

If the {@link Call#request()} does not use TLS, this method will not be invoked. + *

This method is invoked if the following conditions are met: + *

    + *
  • The {@link Call#request()} requires TLS.
    • + *
  • No existing connection from the {@link ConnectionPool} can be reused.
    • + *
+ * + *

This can be invoked more than 1 time for a single {@link Call}. For example, if the response + * to the {@link Call#request()} is a redirect to a different address, or a connection is retried. */ public void secureConnectStart(Call call) { } @@ -65,7 +82,7 @@ public abstract class EventListener { /** * Invoked immediately after a TLS connection was attempted. * - *

If the {@link Call#request()} does not use TLS, this method will not be invoked. + *

This method is always invoked after {@link #secureConnectStart(Call)}. * *

{@code handshake} will be non-null and {@code throwable} will be null in the case of a * successful TLS connection.