Don't log a protocol when it is unknown. (#3558)

Closes: #3395
square · Aug 30, 2017 · 16f43b6 · 16f43b6
1 parent 1801d0c
commit 16f43b6
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 21 deletions.
diff --git a/okhttp-logging-interceptor/src/main/java/okhttp3/logging/HttpLoggingInterceptor.java b/okhttp-logging-interceptor/src/main/java/okhttp3/logging/HttpLoggingInterceptor.java
@@ -24,7 +24,6 @@
 import okhttp3.Interceptor;
 import okhttp3.MediaType;
 import okhttp3.OkHttpClient;
-import okhttp3.Protocol;
 import okhttp3.Request;
 import okhttp3.RequestBody;
 import okhttp3.Response;
@@ -151,8 +150,10 @@ public Level getLevel() {
     boolean hasRequestBody = requestBody != null;
 
     Connection connection = chain.connection();
-    Protocol protocol = connection != null ? connection.protocol() : Protocol.HTTP_1_1;
-    String requestStartMessage = "--> " + request.method() + ' ' + request.url() + ' ' + protocol;
+    String requestStartMessage = "--> "
+        + request.method()
+        + ' ' + request.url()
+        + (connection != null ? " " + connection.protocol() : "");
     if (!logHeaders && hasRequestBody) {
       requestStartMessage += " (" + requestBody.contentLength() + "-byte body)";
     }

diff --git a/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.java b/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.java
@@ -21,13 +21,17 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.regex.Pattern;
+import javax.net.ssl.HostnameVerifier;
 import okhttp3.Dns;
 import okhttp3.HttpUrl;
 import okhttp3.MediaType;
 import okhttp3.OkHttpClient;
+import okhttp3.Protocol;
+import okhttp3.RecordingHostnameVerifier;
 import okhttp3.Request;
 import okhttp3.RequestBody;
 import okhttp3.Response;
+import okhttp3.internal.tls.SslClient;
 import okhttp3.logging.HttpLoggingInterceptor.Level;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
@@ -50,6 +54,8 @@ public final class HttpLoggingInterceptorTest {
 
   @Rule public final MockWebServer server = new MockWebServer();
 
+  private SslClient sslClient = SslClient.localhost();
+  private HostnameVerifier hostnameVerifier = new RecordingHostnameVerifier();
   private OkHttpClient client;
   private String host;
   private HttpUrl url;
@@ -71,6 +77,8 @@ private void setLevel(Level level) {
     client = new OkHttpClient.Builder()
         .addNetworkInterceptor(networkInterceptor)
         .addInterceptor(applicationInterceptor)
+        .sslSocketFactory(sslClient.socketFactory, sslClient.trustManager)
+        .hostnameVerifier(hostnameVerifier)
         .build();
 
     host = server.getHostName() + ":" + server.getPort();
@@ -117,7 +125,7 @@ private void setLevel(Level level) {
     client.newCall(request().build()).execute();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
         .assertNoMoreLogs();
 
@@ -134,7 +142,7 @@ private void setLevel(Level level) {
     client.newCall(request().post(RequestBody.create(PLAIN, "Hi?")).build()).execute();
 
     applicationLogs
-        .assertLogEqual("--> POST " + url + " http/1.1 (3-byte body)")
+        .assertLogEqual("--> POST " + url + " (3-byte body)")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
         .assertNoMoreLogs();
 
@@ -154,7 +162,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 6-byte body\\)")
         .assertNoMoreLogs();
 
@@ -174,7 +182,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, unknown-length body\\)")
         .assertNoMoreLogs();
 
@@ -192,7 +200,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Length: 0")
@@ -221,7 +229,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> POST " + url + " http/1.1")
+        .assertLogEqual("--> POST " + url)
         .assertLogEqual("Content-Type: text/plain; charset=utf-8")
         .assertLogEqual("Content-Length: 3")
         .assertLogEqual("--> END POST")
@@ -254,7 +262,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> POST " + url + " http/1.1")
+        .assertLogEqual("--> POST " + url)
         .assertLogEqual("Content-Length: 3")
         .assertLogEqual("--> END POST")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
@@ -293,7 +301,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> POST " + url + " http/1.1")
+        .assertLogEqual("--> POST " + url)
         .assertLogEqual("Content-Type: text/plain; charset=utf-8")
         .assertLogEqual("--> END POST")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
@@ -326,7 +334,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Length: 6")
@@ -356,7 +364,7 @@ private void setLevel(Level level) {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Length: 0")
@@ -393,7 +401,7 @@ private void bodyGetNoBody(int code) throws IOException {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- " + code + " No Content " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Length: 0")
@@ -422,7 +430,7 @@ private void bodyGetNoBody(int code) throws IOException {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> POST " + url + " http/1.1")
+        .assertLogEqual("--> POST " + url)
         .assertLogEqual("Content-Type: text/plain; charset=utf-8")
         .assertLogEqual("Content-Length: 3")
         .assertLogEqual("")
@@ -460,7 +468,7 @@ private void bodyGetNoBody(int code) throws IOException {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Length: 6")
@@ -496,7 +504,7 @@ private void bodyGetNoBody(int code) throws IOException {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Transfer-encoding: chunked")
@@ -548,7 +556,7 @@ private void bodyGetNoBody(int code) throws IOException {
         .assertNoMoreLogs();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Type: text/plain; charset=utf-8")
@@ -583,7 +591,7 @@ private void bodyGetNoBody(int code) throws IOException {
         .assertNoMoreLogs();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Type: text/html; charset=0")
@@ -622,7 +630,7 @@ private void bodyGetNoBody(int code) throws IOException {
     response.body().close();
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("--> END GET")
         .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
         .assertLogEqual("Content-Length: 9")
@@ -664,11 +672,32 @@ private void bodyGetNoBody(int code) throws IOException {
     }
 
     applicationLogs
-        .assertLogEqual("--> GET " + url + " http/1.1")
+        .assertLogEqual("--> GET " + url)
         .assertLogEqual("<-- HTTP FAILED: java.net.UnknownHostException: reason")
         .assertNoMoreLogs();
   }
 
+  @Test public void http2() throws Exception {
+    server.useHttps(sslClient.socketFactory, false);
+    url = server.url("/");
+
+    setLevel(Level.BASIC);
+
+    server.enqueue(new MockResponse());
+    Response response = client.newCall(request().build()).execute();
+    assertEquals(Protocol.HTTP_2, response.protocol());
+
+    applicationLogs
+        .assertLogEqual("--> GET " + url)
+        .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
+        .assertNoMoreLogs();
+
+    networkLogs
+        .assertLogEqual("--> GET " + url + " h2")
+        .assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
+        .assertNoMoreLogs();
+  }
+
   private Request.Builder request() {
     return new Request.Builder().url(url);
   }