Skip to content

Commit 16f43b6

Browse files
swankjesseswankjesse
authored
Don't log a protocol when it is unknown. (square#3558)
Closes: square#3395
1 parent 1801d0c commit 16f43b6

File tree

2 files changed

+51
-21
lines changed

2 files changed

+51
-21
lines changed

‎okhttp-logging-interceptor/src/main/java/okhttp3/logging/HttpLoggingInterceptor.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import okhttp3.Interceptor;
2525
import okhttp3.MediaType;
2626
import okhttp3.OkHttpClient;
27-
import okhttp3.Protocol;
2827
import okhttp3.Request;
2928
import okhttp3.RequestBody;
3029
import okhttp3.Response;
@@ -151,8 +150,10 @@ public Level getLevel() {
151150
boolean hasRequestBody = requestBody != null;
152151

153152
Connection connection = chain.connection();
154-
Protocol protocol = connection != null ? connection.protocol() : Protocol.HTTP_1_1;
155-
String requestStartMessage = "--> " + request.method() + ' ' + request.url() + ' ' + protocol;
153+
String requestStartMessage = "--> "
154+
+ request.method()
155+
+ ' ' + request.url()
156+
+ (connection != null ? " " + connection.protocol() : "");
156157
if (!logHeaders && hasRequestBody) {
157158
requestStartMessage += " (" + requestBody.contentLength() + "-byte body)";
158159
}

‎okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.java

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@
2121
import java.util.ArrayList;
2222
import java.util.List;
2323
import java.util.regex.Pattern;
24+
import javax.net.ssl.HostnameVerifier;
2425
import okhttp3.Dns;
2526
import okhttp3.HttpUrl;
2627
import okhttp3.MediaType;
2728
import okhttp3.OkHttpClient;
29+
import okhttp3.Protocol;
30+
import okhttp3.RecordingHostnameVerifier;
2831
import okhttp3.Request;
2932
import okhttp3.RequestBody;
3033
import okhttp3.Response;
34+
import okhttp3.internal.tls.SslClient;
3135
import okhttp3.logging.HttpLoggingInterceptor.Level;
3236
import okhttp3.mockwebserver.MockResponse;
3337
import okhttp3.mockwebserver.MockWebServer;
@@ -50,6 +54,8 @@ public final class HttpLoggingInterceptorTest {
5054

5155
@Rule public final MockWebServer server = new MockWebServer();
5256

57+
private SslClient sslClient = SslClient.localhost();
58+
private HostnameVerifier hostnameVerifier = new RecordingHostnameVerifier();
5359
private OkHttpClient client;
5460
private String host;
5561
private HttpUrl url;
@@ -71,6 +77,8 @@ private void setLevel(Level level) {
7177
client = new OkHttpClient.Builder()
7278
.addNetworkInterceptor(networkInterceptor)
7379
.addInterceptor(applicationInterceptor)
80+
.sslSocketFactory(sslClient.socketFactory, sslClient.trustManager)
81+
.hostnameVerifier(hostnameVerifier)
7482
.build();
7583

7684
host = server.getHostName() + ":" + server.getPort();
@@ -117,7 +125,7 @@ private void setLevel(Level level) {
117125
client.newCall(request().build()).execute();
118126

119127
applicationLogs
120-
.assertLogEqual("--> GET " + url + " http/1.1")
128+
.assertLogEqual("--> GET " + url)
121129
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
122130
.assertNoMoreLogs();
123131

@@ -134,7 +142,7 @@ private void setLevel(Level level) {
134142
client.newCall(request().post(RequestBody.create(PLAIN, "Hi?")).build()).execute();
135143

136144
applicationLogs
137-
.assertLogEqual("--> POST " + url + " http/1.1 (3-byte body)")
145+
.assertLogEqual("--> POST " + url + " (3-byte body)")
138146
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
139147
.assertNoMoreLogs();
140148

@@ -154,7 +162,7 @@ private void setLevel(Level level) {
154162
response.body().close();
155163

156164
applicationLogs
157-
.assertLogEqual("--> GET " + url + " http/1.1")
165+
.assertLogEqual("--> GET " + url)
158166
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 6-byte body\\)")
159167
.assertNoMoreLogs();
160168

@@ -174,7 +182,7 @@ private void setLevel(Level level) {
174182
response.body().close();
175183

176184
applicationLogs
177-
.assertLogEqual("--> GET " + url + " http/1.1")
185+
.assertLogEqual("--> GET " + url)
178186
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, unknown-length body\\)")
179187
.assertNoMoreLogs();
180188

@@ -192,7 +200,7 @@ private void setLevel(Level level) {
192200
response.body().close();
193201

194202
applicationLogs
195-
.assertLogEqual("--> GET " + url + " http/1.1")
203+
.assertLogEqual("--> GET " + url)
196204
.assertLogEqual("--> END GET")
197205
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
198206
.assertLogEqual("Content-Length: 0")
@@ -221,7 +229,7 @@ private void setLevel(Level level) {
221229
response.body().close();
222230

223231
applicationLogs
224-
.assertLogEqual("--> POST " + url + " http/1.1")
232+
.assertLogEqual("--> POST " + url)
225233
.assertLogEqual("Content-Type: text/plain; charset=utf-8")
226234
.assertLogEqual("Content-Length: 3")
227235
.assertLogEqual("--> END POST")
@@ -254,7 +262,7 @@ private void setLevel(Level level) {
254262
response.body().close();
255263

256264
applicationLogs
257-
.assertLogEqual("--> POST " + url + " http/1.1")
265+
.assertLogEqual("--> POST " + url)
258266
.assertLogEqual("Content-Length: 3")
259267
.assertLogEqual("--> END POST")
260268
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
@@ -293,7 +301,7 @@ private void setLevel(Level level) {
293301
response.body().close();
294302

295303
applicationLogs
296-
.assertLogEqual("--> POST " + url + " http/1.1")
304+
.assertLogEqual("--> POST " + url)
297305
.assertLogEqual("Content-Type: text/plain; charset=utf-8")
298306
.assertLogEqual("--> END POST")
299307
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
@@ -326,7 +334,7 @@ private void setLevel(Level level) {
326334
response.body().close();
327335

328336
applicationLogs
329-
.assertLogEqual("--> GET " + url + " http/1.1")
337+
.assertLogEqual("--> GET " + url)
330338
.assertLogEqual("--> END GET")
331339
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
332340
.assertLogEqual("Content-Length: 6")
@@ -356,7 +364,7 @@ private void setLevel(Level level) {
356364
response.body().close();
357365

358366
applicationLogs
359-
.assertLogEqual("--> GET " + url + " http/1.1")
367+
.assertLogEqual("--> GET " + url)
360368
.assertLogEqual("--> END GET")
361369
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
362370
.assertLogEqual("Content-Length: 0")
@@ -393,7 +401,7 @@ private void bodyGetNoBody(int code) throws IOException {
393401
response.body().close();
394402

395403
applicationLogs
396-
.assertLogEqual("--> GET " + url + " http/1.1")
404+
.assertLogEqual("--> GET " + url)
397405
.assertLogEqual("--> END GET")
398406
.assertLogMatch("<-- " + code + " No Content " + url + " \\(\\d+ms\\)")
399407
.assertLogEqual("Content-Length: 0")
@@ -422,7 +430,7 @@ private void bodyGetNoBody(int code) throws IOException {
422430
response.body().close();
423431

424432
applicationLogs
425-
.assertLogEqual("--> POST " + url + " http/1.1")
433+
.assertLogEqual("--> POST " + url)
426434
.assertLogEqual("Content-Type: text/plain; charset=utf-8")
427435
.assertLogEqual("Content-Length: 3")
428436
.assertLogEqual("")
@@ -460,7 +468,7 @@ private void bodyGetNoBody(int code) throws IOException {
460468
response.body().close();
461469

462470
applicationLogs
463-
.assertLogEqual("--> GET " + url + " http/1.1")
471+
.assertLogEqual("--> GET " + url)
464472
.assertLogEqual("--> END GET")
465473
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
466474
.assertLogEqual("Content-Length: 6")
@@ -496,7 +504,7 @@ private void bodyGetNoBody(int code) throws IOException {
496504
response.body().close();
497505

498506
applicationLogs
499-
.assertLogEqual("--> GET " + url + " http/1.1")
507+
.assertLogEqual("--> GET " + url)
500508
.assertLogEqual("--> END GET")
501509
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
502510
.assertLogEqual("Transfer-encoding: chunked")
@@ -548,7 +556,7 @@ private void bodyGetNoBody(int code) throws IOException {
548556
.assertNoMoreLogs();
549557

550558
applicationLogs
551-
.assertLogEqual("--> GET " + url + " http/1.1")
559+
.assertLogEqual("--> GET " + url)
552560
.assertLogEqual("--> END GET")
553561
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
554562
.assertLogEqual("Content-Type: text/plain; charset=utf-8")
@@ -583,7 +591,7 @@ private void bodyGetNoBody(int code) throws IOException {
583591
.assertNoMoreLogs();
584592

585593
applicationLogs
586-
.assertLogEqual("--> GET " + url + " http/1.1")
594+
.assertLogEqual("--> GET " + url)
587595
.assertLogEqual("--> END GET")
588596
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
589597
.assertLogEqual("Content-Type: text/html; charset=0")
@@ -622,7 +630,7 @@ private void bodyGetNoBody(int code) throws IOException {
622630
response.body().close();
623631

624632
applicationLogs
625-
.assertLogEqual("--> GET " + url + " http/1.1")
633+
.assertLogEqual("--> GET " + url)
626634
.assertLogEqual("--> END GET")
627635
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms\\)")
628636
.assertLogEqual("Content-Length: 9")
@@ -664,11 +672,32 @@ private void bodyGetNoBody(int code) throws IOException {
664672
}
665673

666674
applicationLogs
667-
.assertLogEqual("--> GET " + url + " http/1.1")
675+
.assertLogEqual("--> GET " + url)
668676
.assertLogEqual("<-- HTTP FAILED: java.net.UnknownHostException: reason")
669677
.assertNoMoreLogs();
670678
}
671679

680+
@Test public void http2() throws Exception {
681+
server.useHttps(sslClient.socketFactory, false);
682+
url = server.url("/");
683+
684+
setLevel(Level.BASIC);
685+
686+
server.enqueue(new MockResponse());
687+
Response response = client.newCall(request().build()).execute();
688+
assertEquals(Protocol.HTTP_2, response.protocol());
689+
690+
applicationLogs
691+
.assertLogEqual("--> GET " + url)
692+
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
693+
.assertNoMoreLogs();
694+
695+
networkLogs
696+
.assertLogEqual("--> GET " + url + " h2")
697+
.assertLogMatch("<-- 200 OK " + url + " \\(\\d+ms, 0-byte body\\)")
698+
.assertNoMoreLogs();
699+
}
700+
672701
private Request.Builder request() {
673702
return new Request.Builder().url(url);
674703
}

0 commit comments

Comments
 (0)