Yup, I'm trying it on OS X and linux. I think the problem is in HttpMethodBase.execute() in particular the following line:
if (responseStream != null) { responseStream.close(); }
Since there is no content length or chunking the response stream is not being wrapped and is therefore actually closed by the above code. Take a look at AutoCloseInputStream.close(). I vaguely remember talking about this case when we added the connection release/reuse code. I may be wrong but I think that the connection should be getting closed in this case as there is no safe way to consume the response.
Mike,
You are right it is definitely those three lines, though it took me a while to find where the extra wrapped stream got added.
Now, when the input stream is closed HttpMethodBase.responseBodyConsumed() is called, it checks shouldCloseConnection() which erroneously returns false and so the HttpConnection is never marked as closed, despite the underlying socket having been closed.
So we need to make shouldCloseConnection more intelligent so that if we have a Connection: keep-alive but not Content-Length it returns true, which is insanely easy to add.
We do however have a test that depends on this bug. TestResponseHeaders.testDuplicateConnection sends two Connection: keep-alive headers, but no Content-Length header and then expects the connection to remain open. I'm certain this test case should be changed to send a Content-Length as well (largely because I'm going to add a test case with a Connection: keep-alive and no Content-Length and check that the connection was closed).
The other problem tests that I'm not so sure about are the proxy tests. TestResponseHeaders.testDuplicateProxyConnection sends two proxy-connection: keep-alive headers but no Content-Length. I believe in this case we should also close the connection since we have the same problem of not knowing where the last response ended as when there's no proxy. Does this evaluation seem correct to others? If so, all that needs to be done is to add Content-Length: 0 to the test and it passes again.
I have another problem with proxies in that even if shouldCloseConnection() returns true, the connection to the proxy isn't actually closed and I'm not sure why. Any hints about where proxy connections are closed?
Mike
Anyway, here is the patch so far, I've modified the two tests that previously failed and added two new tests to ensure that the connection is closed if there is no Content-Length - the proxy version of these tests is currently failing and I'm not sure why.
Index: src/java/org/apache/commons/httpclient/HttpMethodBase.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/ httpclient/HttpMethodBase.java,v
retrieving revision 1.152
diff -u -r1.152 HttpMethodBase.java
--- src/java/org/apache/commons/httpclient/HttpMethodBase.java 13 Jun 2003 21:32:17 -0000 1.152
+++ src/java/org/apache/commons/httpclient/HttpMethodBase.java 20 Jun 2003 05:07:17 -0000
@@ -900,6 +900,10 @@
}
return true;
} else if (connectionHeader.getValue().equalsIgnoreCase("keep-alive")) {
+ if (getResponseContentLength() < 0) {
+ LOG.debug("Should close connection as content-length is missing.");
+ return true;
+ }
if (LOG.isDebugEnabled()) {
LOG.debug("Should NOT close connection in response to "
+ connectionHeader.toExternalForm());
Index: src/test/org/apache/commons/httpclient/TestResponseHeaders.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/ httpclient/TestResponseHeaders.java,v
retrieving revision 1.7
diff -u -r1.7 TestResponseHeaders.java
--- src/test/org/apache/commons/httpclient/TestResponseHeaders.java 10 Jun 2003 22:42:52 -0000 1.7
+++ src/test/org/apache/commons/httpclient/TestResponseHeaders.java 20 Jun 2003 05:07:19 -0000
@@ -153,6 +153,7 @@
"HTTP/1.1 200 OK\r\n"
+ "proxy-connection: close\r\n"
+ "proxy-connection: close\r\n"
+ + "Content-Length: 0\r\n"
+ "\r\n";
conn.addResponse(headers, "");
@@ -169,6 +170,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"
+ "\r\n"; conn.addResponse(headers, "");
@@ -202,6 +204,7 @@
"HTTP/1.0 200 OK\r\n"
+"Connection: keep-alive\r\n"
+"Connection: keep-alive\r\n"
+ +"Content-Length: 0\r\n"
+"\r\n"; conn.addResponse(headers, "");
@@ -210,6 +213,38 @@
method.getResponseBodyAsString(); assertTrue(conn.isOpen());
+ }
+
+ public void testNoContentLength() throws Exception {
+ SimpleHttpConnection conn = new SimpleHttpConnection();
+ String headers =
+ "HTTP/1.1 200 OK\r\n"
+ + "Connection: keep-alive\r\n"
+ + "\r\n";
+
+ conn.addResponse(headers, "12345");
+ GetMethod method = new GetMethod("/");
+ method.execute(new HttpState(), conn);
+ method.getResponseBodyAsString();
+
+ assertFalse(conn.isOpen());
+ }
+
+ public void testProxyNoContentLength() throws Exception {
+ System.err.println("------------");
+ SimpleHttpConnection conn = new SimpleHttpConnection();
+ String headers =
+ "HTTP/1.1 200 OK\r\n"
+ + "proxy-connection: keep-alive\r\n"
+ + "\r\n";
+
+ conn.addResponse(headers, "12345");
+ GetMethod method = new GetMethod("/");
+ method.execute(new HttpState(), conn);
+ method.getResponseBodyAsString();
+
+ assertFalse(conn.isOpen());
+ System.err.println("------------");
}public void testNullHeaders() throws Exception {
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
