8000 Patch NetHttpResponse to detect abrupt termination · amergey/google-http-java-client@8ae9cef · GitHub
[go: up one dir, main page]
Skip to content

Commit 8ae9cef

Browse files
wonderflywonderfly
committed
Patch NetHttpResponse to detect abrupt termination
 JDK's HttpUrlConnection implementation fails to notify clients when the connection is lost when reading a response, e.g., the server gets restarted. This changelist patches the NetHttpResponse to detect abrupt connection termination when a fixed content length is available. In the case where responses are chunk encoded, connection lost is well handled by JDK. Also extend MockHttpUrlConnection for more testing functionalities.
1 parent 3e0769d commit 8ae9cef

File tree

5 files changed

+320
-9
lines changed

5 files changed

+320
-9
lines changed

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

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.google.api.client.http.LowLevelHttpResponse;
1818

19+
import java.io.FilterInputStream;
1920
import java.io.IOException;
2021
import java.io.InputStream;
2122
import java.net.HttpURLConnection;
@@ -70,14 +71,27 @@ public int getStatusCode() {
7071
* with version 1.17 it returns {@link HttpURLConnection#getInputStream} when it doesn't throw
7172
* {@link IOException}, otherwise it returns {@link HttpURLConnection#getErrorStream}
7273
* </p>
74+
*
75+
* <p>
76+
* Upgrade warning: in versions prior to 1.20 {@link #getContent()} returned
77+
* {@link HttpURLConnection#getInputStream()} or {@link HttpURLConnection#getErrorStream()}, both
78+
* of which silently returned -1 for read() calls when the connection got closed in the middle
79+
* of receiving a response. This is highly likely a bug from JDK's {@link HttpURLConnection}.
80+
* Since version 1.20, the bytes read off the wire will be checked and an {@link IOException} will
81+
* be thrown if the response is not fully delivered when the connection is closed by server for
82+
* whatever reason, e.g., server restarts. Note though that this is a best-effort check: when the
83+
* response is chunk encoded, we have to rely on the underlying HTTP library to behave correctly.
84+
* </p>
7385
*/
7486
@Override
7587
public InputStream getContent() throws IOException {
88+
InputStream in = null;
7689
try {
77-
return connection.getInputStream();
90+
in = connection.getInputStream();
7891
} catch (IOException ioe) {
79-
return connection.getErrorStream();
92+
in = connection.getErrorStream();
8093
}
94+
return in == null ? null : new SizeValidatingInputStream(in);
8195
}
8296

8397
@Override
@@ -131,4 +145,63 @@ public String getHeaderValue(int index) {
131145
public void disconnect() {
132146
connection.disconnect();
133147
}
148+
149+
/**
150+
* A wrapper arround the base {@link InputStream} that validates EOF returned by the read calls.
151+
*
152+
* @since 1.20
153+
*/
154+
private final class SizeValidatingInputStream extends FilterInputStream {
155+
156+
private long bytesRead = 0;
157+
158+
public SizeValidatingInputStream(InputStream in) {
159+
super(in);
160+
}
161+
162+
/**
163+
* java.io.InputStream#read(byte[], int, int) swallows IOException thrown from read() so we have
164+
* to override it.
165+
* @see "http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8-b132/java/io/InputStream.java#185"
166+
*/
167+
@Override
168+
public int read(byte[] b, int off, int len) throws IOException {
169+
int n = in.read(b, off, len);
170+
if (n == -1) {
171+
throwIfFalseEOF();
172+
} else {
173+
bytesRead += n;
174+
}
175+
return n;
176+
}
177+
178+
@Override
179+
public int read() throws IOException {
180+
int n = in.read();
181+
if (n == -1) {
182+
throwIfFalseEOF();
183+
} else {
184+
bytesRead++;
185+
}
186+
return n;
187+
}
188+
189+
// Throws an IOException if gets an EOF in the middle of a response.
190+
private void throwIfFalseEOF() throws IOException {
191+
long contentLength = getContentLength();
192+
if (contentLength == -1) {
193+
// If a Content-Length header is missing, there's nothing we can do.
194+
return;
195+
}
196+
// According to RFC2616, message-body is prohibited in responses to certain requests, e.g.,
197+
// HEAD. Nevertheless an entity-header (possibly with non-zero Content-Length) may be present.
198+
// Thus we exclude the case where bytesRead == 0.
199+
//
200+
// See http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4 for details.
201+
if (bytesRead != 0 && bytesRead < contentLength) {
202+
throw new IOException("Connection closed prematurely: bytesRead = " + bytesRead
203+
+ ", Content-Length = " + contentLength);
204+
}
205+
}
206+
}
134207
}

‎google-http-client/src/main/java/com/google/api/client/testing/http/javanet/MockHttpURLConnection.java‎

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,17 @@
1717
import com.google.api.client.util.Beta;
1818
import com.google.api.client.util.Preconditions;
1919

20-
import java.io.ByteArrayInputStream;
2120
import java.io.ByteArrayOutputStream;
2221
import java.io.IOException;
2322
import java.io.InputStream;
2423
import java.io.OutputStream;
2524
import java.net.HttpURLConnection;
2625
import java.net.URL;
2726
import java.net.UnknownServiceException;
27+
import java.util.ArrayList;
28+
import java.util.LinkedHashMap;
29+
import java.util.List;
30+
import java.util.Map;
2831

2932
/**
3033
* {@link Beta} <br/>
@@ -52,20 +55,28 @@ public class MockHttpURLConnection extends HttpURLConnection {
5255
/**
5356
* The input byte array which represents the content when the status code is less then
5457
* {@code 400}.
58+
*
59+
* @deprecated As of 1.20. Use {@link #setInputStream(InputStream)} instead.
5560
*/
61+
@Deprecated
5662
public static final byte[] INPUT_BUF = new byte[1];
5763

5864
/**
5965
* The error byte array which represents the content when the status code is greater or equal to
6066
* {@code 400}.
67+
*
68+
* @deprecated As of 1.20. Use {@link #setErrorStream(InputStream)} instead.
6169
*/
70+
@Deprecated
6271
public static final byte[] ERROR_BUF = new byte[5];
6372

6473
/** The input stream. */
65-
private InputStream inputStream = new ByteArrayInputStream(INPUT_BUF);
74+
private InputStream inputStream = null;
6675

6776
/** The error stream. */
68-
private InputStream errorStream = new ByteArrayInputStream(ERROR_BUF);
77+
private InputStream errorStream = null;
78+
79+
private Map<String, List<String>> headers = new LinkedHashMap<String, List<String>>();
6980

7081
/**
7182
8090 * @param u the URL or {@code null} for none
@@ -130,6 +141,54 @@ public MockHttpURLConnection setResponseCode(int responseCode) {
130141
return this;
131142
}
132143

144+
/**
145+
* Sets a custom response header.
146+
*
147+
* @since 1.20
148+
*/
149+
public MockHttpURLConnection addHeader(String name, String value) {
150+
Preconditions.checkNotNull(name);
151+
Preconditions.checkNotNull(value);
152+
if (headers.containsKey(name)) {
153+
headers.get(name).add(value);
154+
} else {
155+
List<String> values = new ArrayList<String>();
156+
values.add(value);
157+
headers.put(name, values);
158+
}
159+
return this;
160+
}
161+
162+
/**
163+
* Sets the input stream.
164+
*
165+
* <p>To prevent incidental overwrite, only the first non-null assignment is honored.
166+
*
167+
* @since 1.20
168+
*/
169+
public MockHttpURLConnection setInputStream(InputStream is) {
170+
Preconditions.checkNotNull(is);
171+
if (inputStream == null) {
172+
inputStream = is;
173+
}
174+
return this;
175+
}
176+
177+
/**
178+
* Sets the error stream.
179+
*
180+
* <p>To prevent incidental overwrite, only the first non-null assignment is honored.
181+
*
182+
* @since 1.20
183+
*/
184+
public MockHttpURLConnection setErrorStream(InputStream is) {
185+
Preconditions.checkNotNull(is);
186+
if (errorStream == null) {
187+
errorStream = is;
188+
}
189+
return this;
190+
}
191+
133192
@Override
134193
public InputStream getInputStream() throws IOException {
135194
if (responseCode < 400) {
@@ -142,4 +201,15 @@ public InputStream getInputStream() throws IOException {
142201
public InputStream getErrorStream() {
143202
return errorStream;
144203
}
204+
205+
@Override
206+
public Map<String, List<String>> getHeaderFields() {
207+
return headers;
208+
}
209+
210+
@Override
211+
public String getHeaderField(String name) {
212+
List<String> values = headers.get(name);
213+
return values == null ? null : values.get(0);
214+
}
145215
}

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

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,14 @@
1515
package com.google.api.client.http.javanet;
1616

1717
import com.google.api.client.testing.http.javanet.MockHttpURLConnection;
18+
import com.google.api.client.util.StringUtils;
1819

1920
import junit.framework.TestCase;
2021

22+
import java.io.ByteArrayInputStream;
2123
import java.io.IOException;
24+
import java.io.InputStream;
25+
import java.nio.charset.Charset;
2226

2327
/**
2428
* Tests {@link NetHttpResponse}.
@@ -27,6 +31,9 @@
2731
*/
2832
public class NetHttpResponseTest extends TestCase {
2933

34+
private static final String VALID_RESPONSE = "This is a valid response.";
35+
private static final String ERROR_RESPONSE = "This is an error response.";
36+
3037
public void testGetStatusCode() throws IOException {
3138
subtestGetStatusCode(0, -1);
3239
subtestGetStatusCode(200, 200);
@@ -45,16 +52,48 @@ public void testGetContent() throws IOException {
4552
subtestGetContent(307);
4653
subtestGetContent(404);
4754
subtestGetContent(503);
55+
56+
subtestGetContentWithShortRead(0);
57+
subtestGetContentWithShortRead(200);
58+
subtestGetContentWithShortRead(304);
59+
subtestGetContentWithShortRead(307);
60+
subtestGetContentWithShortRead(404);
61+
subtestGetContentWithShortRead(503);
4862
}
4963

5064
public void subtestGetContent(int responseCode) throws IOException {
5165
NetHttpResponse response =
52-
new NetHttpResponse(new MockHttpURLConnection(null).setResponseCode(responseCode));
53-
int bytes = response.getC 234B ontent().read(new byte[100]);
66+
new NetHttpResponse(new MockHttpURLConnection(null).setResponseCode(responseCode)
67+
.setInputStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(VALID_RESPONSE)))
68+
.setErrorStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(ERROR_RESPONSE))));
69+
InputStream is = response.getContent();
70+
byte[] buf = new byte[100];
71+
int bytes = 0, n = 0;
72+
while ((n = is.read(buf)) != -1) {
73+
bytes += n;
74+
}
75+
if (responseCode < 400) {
76+
assertEquals(VALID_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
77+
} else {
78+
assertEquals(ERROR_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
79+
}
80+
}
81+
82+
public void subtestGetContentWithShortRead(int responseCode) throws IOException {
83+
NetHttpResponse response =
84+
new NetHttpResponse(new MockHttpURLConnection(null).setResponseCode(responseCode)
85+
.setInputStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(VALID_RESPONSE)))
86+
.setErrorStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(ERROR_RESPONSE))));
87+
InputStream is = response.getContent();
88+
byte[] buf = new byte[100];
89+
int bytes = 0, b = 0;
90+
while ((b = is.read()) != -1) {
91+
buf[bytes++] = (byte) b;
92+
}
5493
if (responseCode < 400) {
55-
assertEquals(MockHttpURLConnection.INPUT_BUF.length, bytes);
94+
assertEquals(VALID_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
5695
} else {
57-
assertEquals(MockHttpURLConnection.ERROR_BUF.length, bytes);
96+
assertEquals(ERROR_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
5897
}
5998
}
6099
}

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

Lines changed: 55 additions & 0 deletions
116
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121

2222
import junit.framework.TestCase;
2323

24+
import java.io.ByteArrayInputStream;
2425
import java.io.IOException;
26+
import java.io.InputStream;
2527
import java.net.HttpURLConnection;
2628
import java.net.URL;
2729

@@ -59,6 +61,7 @@ public void testExecute_mock() throws Exception {
5961
}
6062
}
6163

64+
6265
public void testExecute_methodUnchanged() throws Exception {
6366
for (String method : METHODS) {
6467
HttpURLConnection connection =
@@ -75,6 +78,58 @@ public void testExecute_methodUnchanged() throws Exception {
7578
}
7679
}
7780

81+
public void testAbruptTerminationIsNoticedWithContentLength() throws Exception {
82+
String incompleteBody = ""
83+
+ "Fixed size body test.\r\n"
84+
+ "Incomplete response.";
85+
byte[] buf = StringUtils.getBytesUtf8(incompleteBody);
86+
MockHttpURLConnection connection = new MockHttpURLConnection(new URL(HttpTesting.SIMPLE_URL))
87+
.setResponseCode(200)
88+
.addHeader("Content-Length", "205")
89+
.setInputStream(new ByteArrayInputStream(buf));
90+
connection.setRequestMethod("GET");
91+
NetHttpRequest request = new NetHttpRequest(connection);
92+
setContent(request, null, "");
93+
NetHttpResponse response = (NetHttpResponse) (request.execute());
94+
95+
InputStream in = response.getContent();
96+
boolean thrown = false;
97+
try {
98+
while (in.read() != -1) {
99+
// This space is intentionally left blank.
100+
}
101+
} catch (IOException ioe) {
102+
thrown = true;
103+
}
104+
assertTrue(thrown);
105+
}
106+
107+
public void testAbruptTerminationIsNoticedWithContentLengthWithReadToBuf() throws Exception {
108+
String incompleteBody = ""
109+
+ "Fixed size body test.\r\n"
110+
+ "Incomplete response.";
111+
byte[] buf = StringUtils.getBytesUtf8(incompleteBody);
112+
MockHttpURLConnection connection = new MockHttpURLConnection(new URL(HttpTesting.SIMPLE_URL))
113+
.setResponseCode(200)
114+
.addHeader("Content-Length", "205")
115+
.setInputStream(new ByteArrayInputStream(buf));
+
connection.setRequestMethod("GET");
117+
NetHttpRequest request = new NetHttpRequest(connection);
118+
setContent(request, null, "");
119+
NetHttpResponse response = (NetHttpResponse) (request.execute());
120+
121+
InputStream in = response.getContent();
122+
boolean thrown = false;
123+
try {
124+
while (in.read(new byte[100]) != -1) {
125+
// This space is intentionally left blank.
126+
}
127+
} catch (IOException ioe) {
128+
thrown = true;
129+
}
130+
assertTrue(thrown);
131+
}
132+
78133
private void setContent(NetHttpRequest request, String type, String value) throws Exception {
79134
byte[] bytes = StringUtils.getBytesUtf8(value);
80135
request.setStreamingContent(new ByteArrayStreamingContent(bytes));

0 commit comments

Comments
 (0)
0