8000 http: fix bug that response headers aren't cleared on multiple redirects · robccan/google-http-java-client@6665f76 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6665f76

Browse files
author
Yaniv Inbar
committed
http: fix bug that response headers aren't cleared on multiple redirects
https://codereview.appspot.com/7368047/
1 parent a33c0ed commit 6665f76

File tree

4 files changed

+42
-14
lines changed

4 files changed

+42
-14
lines changed

‎google-http-client/src/main/java/com/google/api/client/http/HttpHeaders.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -974,8 +974,8 @@ static void serializeHeaders(HttpHeaders headers,
974974
* Puts all headers of the {@link LowLevelHttpResponse} into this {@link HttpHeaders} object.
975975
*
976976
* <p>
977-
* Upgrade warning: this method now throws an {@link IOException}. In prior version 1.11 it did
978-
* not throw an exception.
977+
* Upgrade warning: in prior version 1.13 it did not clear the headers before parsing the
978+
* response, but starting in version 1.14 it does clear the headers first.
979979
* </p>
980980
*
981981
* @param response Response from which the headers are copied
@@ -985,6 +985,7 @@ static void serializeHeaders(HttpHeaders headers,
985985
*/
986986
public final void fromHttpResponse(LowLevelHttpResponse response, StringBuilder logger)
987987
throws IOException {
988+
clear();
988989
ParseHeaderState state = new ParseHeaderState(this, logger);
989990
int headerCount = response.getHeaderCount();
990991
for (int i = 0; i < headerCount; i++) {

‎google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,6 @@ public final class HttpResponse {
7171
/** Parsed content-type/media type or {@code null} if content-type is null. */
7272
private final HttpMediaType mediaType;
7373

74-
/** HTTP headers. */
75-
private final HttpHeaders headers;
76-
7774
/** Low-level HTTP response. */
7875
LowLevelHttpResponse response;
7976

@@ -83,9 +80,6 @@ public final class HttpResponse {
8380
/** Status message or {@code null}. */
8481
private final String statusMessage;
8582

86-
/** HTTP transport. */
87-
private final HttpTransport transport;
88-
8983
/** HTTP request. */
9084
private final HttpRequest request;
9185

@@ -125,8 +119,6 @@ public final class HttpResponse {
125119

126120
HttpResponse(HttpRequest request, LowLevelHttpResponse response) throws IOException {
127121
this.request = request;
128-
transport = request.getTransport();
129-
headers = request.getResponseHeaders();
130122
contentLoggingLimit = request.getContentLoggingLimit();
131123
loggingEnabled = request.isLoggingEnabled();
132124
this.response = response;
@@ -154,13 +146,13 @@ public final class HttpResponse {
154146
}
155147

156148
// headers
157-
headers.fromHttpResponse(response, loggable ? logbuf : null);
149+
request.getResponseHeaders().fromHttpResponse(response, loggable ? logbuf : null);
158150

159151
// Retrieve the content-type directly from the headers as response.getContentType() is outdated
160152
// and e.g. not set by BatchUnparsedResponse.FakeLowLevelHttpResponse
161153
String contentType = response.getContentType();
162154
if (contentType == null) {
163-
contentType = headers.getContentType();
155+
contentType = request.getResponseHeaders().getContentType();
164156
}
165157
this.contentType = contentType;
166158
mediaType = contentType == null ? null : new HttpMediaType(contentType);
@@ -287,7 +279,7 @@ public HttpMediaType getMediaType() {
287279
* @since 1.5
288280
*/
289281
public HttpHeaders getHeaders() {
290-
return headers;
282+
return request.getResponseHeaders();
291283
}
292284

293285
/**
@@ -324,7 +316,7 @@ public String getStatusMessage() {
324316
* @since 1.5
325317
*/
326318
public HttpTransport getTransport() {
327-
return transport;
319+
return request.getTransport();
328320
}
329321

330322
/**

‎google-http-client/src/test/java/com/google/api/client/http/HttpHeadersTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,4 +279,12 @@ public void testFromHttpResponse_doubleConvert() throws Exception {
279279
assertEquals("foo/bar", slugHeaders2.getContentType());
280280
assertEquals("123456789", slugHeaders2.slug);
281281
}
282+
283+
public void testFromHttpResponse_clearOldValue() throws Exception {
284+
HttpHeaders headers = new HttpHeaders();
285+
headers.put("Foo", "oldValue");
286+
headers.fromHttpResponse(new MockLowLevelHttpResponse().setHeaderNames(Arrays.asList("Foo"))
287+
.setHeaderValues(Arrays.asList("newvalue")), null);
288+
assertEquals(Arrays.asList("newvalue"), headers.get("Foo"));
289+
}
282290
}

‎google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,4 +962,31 @@ public void testExecuteAsync()
962962
assertTrue(futureResponse.isDone());
963963
assertNotNull(futureResponse.get(10, TimeUnit.MILLISECONDS));
964964
}
965+
966+
public void testExecute_redirects() throws Exception {
967+
class MyTransport extends MockHttpTransport {
968+
int count = 1;
969+
970+
@Override
971+
public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
972+
// expect that it redirected to new URL every time using the count
973+
assertEquals(HttpTesting.SIMPLE_URL + "_" + count, url);
974+
count++;
975+
return new MockLowLevelHttpRequest().setResponse(
976+
new MockLowLevelHttpResponse().setStatusCode(
977+
HttpStatusCodes.STATUS_CODE_MOVED_PERMANENTLY)
978+
.setHeaderNames(Arrays.asList("Location"))
979+
.setHeaderValues(Arrays.asList(HttpTesting.SIMPLE_URL + "_" + count)));
980+
}
981+
}
982+
MyTransport transport = new MyTransport();
983+
HttpRequest request = transport.createRequestFactory()
984+
.buildGetRequest(new GenericUrl(HttpTesting.SIMPLE_URL + "_" + transport.count));
985+
try {
986+
request.execute();
987+
fail("expected " + HttpResponseException.class);
988+
} catch (HttpResponseException e) {
989+
assertEquals(HttpStatusCodes.STATUS_CODE_MOVED_PERMANENTLY, e.getStatusCode());
990+
}
991+
}
965992
}

0 commit comments

Comments
 (0)
0