Skip to content

Commit 2bb7da7

Browse files
renechoirenechoi
committed
cap negative Content-Length in GoogleHttpClient response conversion
`convertResponse` narrowed the response `Content-Length` to `Integer` while only guarding the upper bound, so a negative header value was passed straight through to `Response.Body#length()`. That field is documented as "length in bytes, if known. Null if unknown or greater than Integer.MAX_VALUE", and every other client already rejects negative values before narrowing: - `OkHttpClient` (`contentLength() >= 0 && <= Integer.MAX_VALUE`) - `ApacheHttp5Client` and `ApacheHttpClient` (same guard on the entity) - `Http2Client` (same guard, added in #3431) `GoogleHttpClient` was the only remaining client without it, so a malformed `Content-Length: -1` surfaced as `length() == -1` instead of `null`. `InvocationContext` compares that value against `MAX_RESPONSE_BUFFER_SIZE` to decide whether a response is bufferable, and a negative length silently passes the check. Adds the missing lower-bound guard and reads the header once instead of twice. Covered by tests mirroring `Http2ClientContentLengthTest`.
1 parent 293e69e commit 2bb7da7

2 files changed

Lines changed: 68 additions & 2 deletions

File tree

‎googlehttpclient/src/main/java/feign/googlehttpclient/GoogleHttpClient.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,10 @@ private final HttpRequest convertRequest(
111111
private final Response convertResponse(
112112
final Request inputRequest, final HttpResponse inputResponse) throws IOException {
113113
final HttpHeaders headers = inputResponse.getHeaders();
114+
final Long length = headers.getContentLength();
114115
Integer contentLength = null;
115-
if (headers.getContentLength() != null && headers.getContentLength() <= Integer.MAX_VALUE) {
116-
contentLength = inputResponse.getHeaders().getContentLength().intValue();
116+
if (length != null && length >= 0 && length <= Integer.MAX_VALUE) {
117+
contentLength = length.intValue();
117118
}
118119
return Response.builder()
119120
.body(inputResponse.getContent(), contentLength)

‎googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientContentLengthTest.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright © 2012 The Feign Authors ([email protected])
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package feign.googlehttpclient;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import com.google.api.client.testing.http.MockHttpTransport;
21+
import com.google.api.client.testing.http.MockLowLevelHttpResponse;
22+
import feign.Request;
23+
import feign.Request.HttpMethod;
24+
import feign.Request.Options;
25+
import feign.Response;
26+
import java.io.IOException;
27+
import java.nio.charset.StandardCharsets;
28+
import java.util.Collections;
29+
import org.junit.jupiter.api.Test;
30+
31+
class GoogleHttpClientContentLengthTest {
32+
33+
private static Response decode(String contentLength) throws IOException {
34+
final MockLowLevelHttpResponse lowLevelResponse =
35+
new MockLowLevelHttpResponse().setStatusCode(200).setContent("");
36+
lowLevelResponse.addHeader("Content-Length", contentLength);
37+
final MockHttpTransport transport =
38+
new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build();
39+
final Request request =
40+
Request.create(
41+
HttpMethod.GET,
42+
"http://localhost",
43+
Collections.emptyMap(),
44+
null,
45+
StandardCharsets.UTF_8,
46+
null);
47+
return new GoogleHttpClient(transport).execute(request, new Options());
48+
}
49+
50+
@Test
51+
void contentLengthAboveIntMaxIsReportedAsUnknown() throws IOException {
52+
// 2^31, a valid Content-Length larger than Integer.MAX_VALUE
53+
assertThat(decode("2147483648").body().length()).isNull();
54+
}
55+
56+
@Test
57+
void negativeContentLengthIsReportedAsUnknown() throws IOException {
58+
assertThat(decode("-1").body().length()).isNull();
59+
}
60+
61+
@Test
62+
void contentLengthWithinIntRangeIsPreserved() throws IOException {
63+
assertThat(decode("1024").body().length()).isEqualTo(1024);
64+
}
65+
}

0 commit comments

Comments
 (0)