You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ol...@apache.org on 2003/12/11 23:54:19 UTC
cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestBadContentLength.java TestMethodCharEncoding.java TestNoHost.java TestResponseHeaders.java
olegk 2003/12/11 14:54:19
Modified: httpclient/src/java/org/apache/commons/httpclient
HttpMethodBase.java
httpclient/src/java/org/apache/commons/httpclient/params
HttpMethodParams.java
httpclient/src/test/org/apache/commons/httpclient
TestBadContentLength.java
TestMethodCharEncoding.java TestNoHost.java
TestResponseHeaders.java
Log:
PR #24560 (HttpClient loops endlessly while trying to retrieve status line)
Contributed by Christian Kohlschuetter
Reviewed By Oleg Kalnichevski & Adrian Sutton
Revision Changes Path
1.194 +43 -19 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java
Index: HttpMethodBase.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v
retrieving revision 1.193
retrieving revision 1.194
diff -u -r1.193 -r1.194
--- HttpMethodBase.java 11 Dec 2003 01:19:32 -0000 1.193
+++ HttpMethodBase.java 11 Dec 2003 22:54:18 -0000 1.194
@@ -120,6 +120,7 @@
* @author <a href="mailto:oleg@ural.ru">Oleg Kalnichevski</a>
* @author <a href="mailto:mbowler@GargoyleSoftware.com">Mike Bowler</a>
* @author <a href="mailto:ggregory@seagullsw.com">Gary Gregory</a>
+ * @author Christian Kohlschuetter
*
* @version $Revision$ $Date$
*/
@@ -872,7 +873,6 @@
* @return boolean true if we should close the connection.
*/
protected boolean shouldCloseConnection(HttpConnection conn) {
-
// Connection must be closed due to an abnormal circumstance
if (isConnectionCloseForced()) {
LOG.debug("Should force-close connection.");
@@ -1750,24 +1750,29 @@
throws IOException, HttpRecoverableException, HttpException {
LOG.trace("enter HttpMethodBase.readStatusLine(HttpState, HttpConnection)");
+ final int maxGarbageLines = getParams().
+ getIntParameter(HttpMethodParams.STATUS_LINE_GARBAGE_LIMIT, Integer.MAX_VALUE);
+
//read out the HTTP status string
- String s = conn.readLine();
- while ((s != null) && !StatusLine.startsWithHTTP(s)) {
+ int count = 0;
+ String s;
+ do {
+ s = conn.readLine();
if (Wire.enabled()) {
Wire.input(s + "\r\n");
}
- s = conn.readLine();
- }
- if (s == null) {
- // A null statusString means the connection was lost before we got a
- // response. Try again.
- throw new HttpRecoverableException("Error in parsing the status "
- + " line from the response: unable to find line starting with"
- + " \"HTTP\"");
- }
- if (Wire.enabled()) {
- Wire.input(s + "\r\n");
- }
+ if (s != null && StatusLine.startsWithHTTP(s)) {
+ // Got one
+ break;
+ } else if (s == null || count >= maxGarbageLines) {
+ // Giving up
+ throw new HttpRecoverableException("Error in parsing the status "
+ + " line from the response: unable to find line starting with"
+ + " \"HTTP\"");
+ }
+ count++;
+ } while(true);
+
//create the status line from the status string
statusLine = new StatusLine(s);
@@ -2166,6 +2171,25 @@
if (responseConnection != null) {
responseConnection.setLastResponseInputStream(null);
+ // At this point, no response data should be available.
+ // If there is data available, regard the connection as being
+ // unreliable and close it.
+
+ try {
+ if(responseConnection.isResponseAvailable()) {
+ boolean logExtraInput =
+ getParams().isParameterTrue(HttpMethodParams.WARN_EXTRA_INPUT);
+
+ if(logExtraInput) {
+ LOG.warn("Extra response data detected - closing connection");
+ }
+ setConnectionCloseForced(true);
+ }
+ }
+ catch (IOException e) {
+ LOG.info(e.getMessage());
+ responseConnection.close();
+ }
if (shouldCloseConnection(responseConnection)) {
responseConnection.close();
}
1.7 +48 -6 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/params/HttpMethodParams.java
Index: HttpMethodParams.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/params/HttpMethodParams.java,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- HttpMethodParams.java 11 Dec 2003 01:19:32 -0000 1.6
+++ HttpMethodParams.java 11 Dec 2003 22:54:19 -0000 1.7
@@ -76,6 +76,7 @@
* its value will be drawn from the parent collection of parameters.
*
* @author <a href="mailto:oleg@ural.ru">Oleg Kalnichevski</a>
+ * @author Christian Kohlschuetter
*
* @version $Revision$
*/
@@ -202,7 +203,45 @@
* This parameter expects a value of type {@link String}.
* </p>
*/
- public static final String COOKIE_POLICY = "http.protocol.cookie-policy";
+ public static final String COOKIE_POLICY = "http.protocol.cookie-policy";
+
+ /**
+ * Defines HttpClient's behavior when a response provides more bytes than
+ * expected (specified with Content-Length, for example).
+ * <p>
+ * Such surplus data makes the HTTP connection unreliable for keep-alive
+ * requests, as malicious response data (faked headers etc.) can lead to undesired
+ * results on the next request using that connection.
+ * </p>
+ * <p>
+ * If this parameter is set to <code>true</code>, any detection of extra
+ * input data will generate a warning in the log.
+ * </p>
+ * <p>
+ * This parameter expects a value of type {@link Boolean}.
+ * </p>
+ */
+ public static final String WARN_EXTRA_INPUT = "http.protocol.warn-extra-input";
+
+ /**
+ * Defines the maximum number of ignorable lines before we expect
+ * a HTTP response's status code.
+ * <p>
+ * With HTTP/1.1 persistent connections, the problem arises that
+ * broken scripts could return a wrong Content-Length
+ * (there are more bytes sent than specified).<br />
+ * Unfortunately, in some cases, this is not possible after the bad response,
+ * but only before the next one. <br />
+ * So, HttpClient must be able to skip those surplus lines this way.
+ * </p>
+ * <p>
+ * Set this to 0 to disallow any garbage/empty lines before the status line.<br />
+ * To specify no limit, use {@link Integer.MAX_VALUE} (default in lenient mode).
+ * </p>
+ *
+ * This parameter expects a value of type {@link Integer}.
+ */
+ public static final String STATUS_LINE_GARBAGE_LIMIT = "http.protocol.status-line-garbage-limit";
/**
* Creates a new collection of parameters with the collection returned
@@ -332,7 +371,8 @@
UNAMBIGUOUS_STATUS_LINE,
SINGLE_COOKIE_HEADER,
STRICT_TRANSFER_ENCODING,
- REJECT_HEAD_BODY
+ REJECT_HEAD_BODY,
+ WARN_EXTRA_INPUT
};
/**
@@ -344,6 +384,7 @@
*/
public void makeStrict() {
setParameters(PROTOCOL_STRICTNESS_PARAMETERS, new Boolean(true));
+ setIntParameter(STATUS_LINE_GARBAGE_LIMIT, 0);
}
/**
@@ -354,6 +395,7 @@
*/
public void makeLenient() {
setParameters(PROTOCOL_STRICTNESS_PARAMETERS, new Boolean(false));
+ setIntParameter(STATUS_LINE_GARBAGE_LIMIT, Integer.MAX_VALUE);
}
}
1.2 +8 -13 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestBadContentLength.java
Index: TestBadContentLength.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestBadContentLength.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- TestBadContentLength.java 5 Dec 2003 21:23:14 -0000 1.1
+++ TestBadContentLength.java 11 Dec 2003 22:54:19 -0000 1.2
@@ -194,22 +194,17 @@
client.executeMethod(m);
assertEquals(200, m.getStatusCode());
- assertEquals(null, m.getResponseBodyAsString());
+ assertEquals("12345", m.getResponseBodyAsString());
m.recycle();
client.executeMethod(m);
assertEquals(200, m.getStatusCode());
- boolean gotException = false;
- try {
- InputStream in = m.getResponseBodyAsStream();
- while (in.read() != -1) {
- }
- fail("Did not receive expected ProtocolException");
- } catch (ProtocolException e) {
- /* expected */
+ InputStream in = m.getResponseBodyAsStream();
+ while (in.read() != -1) {
}
+
m.releaseConnection();
}
1.5 +5 -9 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestMethodCharEncoding.java
Index: TestMethodCharEncoding.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestMethodCharEncoding.java,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- TestMethodCharEncoding.java 16 Jul 2003 20:48:28 -0000 1.4
+++ TestMethodCharEncoding.java 11 Dec 2003 22:54:19 -0000 1.5
@@ -151,37 +151,33 @@
SimpleHttpConnection conn = new SimpleHttpConnection();
String body = "stuff";
String headers1 = "HTTP/1.1 200 OK\r\n"
- +"Content-Length: 4\r\n";
+ +"Content-Length: 5\r\n";
conn.addResponse(headers1, body);
conn.open();
GetMethod httpget = new GetMethod("/");
httpget.execute(new HttpState(), conn);
assertEquals(CHARSET_DEFAULT, httpget.getResponseCharSet());
conn.close();
- httpget.recycle();
+ httpget = new GetMethod("/");
String headers2 = "HTTP/1.1 200 OK\r\n"
+"Content-Type: text/plain\r\n"
- +"Content-Length: 4\r\n";
+ +"Content-Length: 5\r\n";
conn.addResponse(headers2, body);
conn.open();
- httpget.setPath("/");
httpget.execute(new HttpState(), conn);
assertEquals(CHARSET_DEFAULT, httpget.getResponseCharSet());
conn.close();
- httpget.recycle();
+ httpget = new GetMethod("/");
String headers3 = "HTTP/1.1 200 OK\r\n"
+"Content-Type: text/plain; charset=" + CHARSET_UTF8 + "\r\n"
- +"Content-Length: 4\r\n";
+ +"Content-Length: 5\r\n";
conn.addResponse(headers3, body);
conn.open();
- httpget.setPath("/");
httpget.execute(new HttpState(), conn);
assertEquals(CHARSET_UTF8, httpget.getResponseCharSet());
conn.close();
- httpget.recycle();
-
}
1.28 +5 -5 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestNoHost.java
Index: TestNoHost.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestNoHost.java,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- TestNoHost.java 5 Dec 2003 21:23:14 -0000 1.27
+++ TestNoHost.java 11 Dec 2003 22:54:19 -0000 1.28
@@ -108,7 +108,7 @@
suite.addTest(TestExceptions.suite());
suite.addTest(TestHttpVersion.suite());
suite.addTest(TestHttpParser.suite());
- //TODO enable when ready: suite.addTest(TestBadContentLength.suite());
+ suite.addTest(TestBadContentLength.suite());
return suite;
}
1.12 +7 -6 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestResponseHeaders.java
Index: TestResponseHeaders.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestResponseHeaders.java,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- TestResponseHeaders.java 3 Nov 2003 23:03:02 -0000 1.11
+++ TestResponseHeaders.java 11 Dec 2003 22:54:19 -0000 1.12
@@ -153,7 +153,7 @@
"HTTP/1.0 200 OK\r\n"
+ "proxy-connection: keep-alive\r\n"
+ "proxy-connection: keep-alive\r\n"
- + "Content-Length: 0\r\n"
+ + "Content-Length: 2\r\n"
+ "\r\n";
conn.addResponse(headers, "");
@@ -170,6 +170,7 @@
"HTTP/1.1 200 OK\r\n"
+ "Connection: close\r\n"
+ "Connection: close\r\n"
+ + "Content-Length: 0\r\n"
+ "\r\n";
GetMethod method = new GetMethod("/");
@@ -183,7 +184,7 @@
"HTTP/1.0 200 OK\r\n"
+"Connection: keep-alive\r\n"
+"Connection: keep-alive\r\n"
- + "Content-Length: 0\r\n"
+ + "Content-Length: 2\r\n"
+"\r\n";
method = new GetMethod("/");
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org