Index: java/org/apache/commons/httpclient/ConnectMethod.java =================================================================== RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/ConnectMethod.java,v retrieving revision 1.12 diff -u -r1.12 ConnectMethod.java --- java/org/apache/commons/httpclient/ConnectMethod.java 25 Apr 2003 18:03:28 -0000 1.12 +++ java/org/apache/commons/httpclient/ConnectMethod.java 27 Apr 2003 21:28:06 -0000 @@ -117,8 +117,7 @@ * @see HttpMethodBase#addAuthorizationRequestHeader(HttpState, HttpConnection) */ protected void addAuthorizationRequestHeader(HttpState state, HttpConnection conn) - throws IOException, HttpException - { + throws IOException, HttpException { // Do nothing. Not applicable to CONNECT method } @@ -136,8 +135,7 @@ * @see HttpMethodBase#addContentLengthRequestHeader(HttpState, HttpConnection) */ protected void addContentLengthRequestHeader(HttpState state, HttpConnection conn) - throws IOException, HttpException - { + throws IOException, HttpException { // Do nothing. Not applicable to CONNECT method } @@ -155,8 +153,7 @@ * @see HttpMethodBase#addCookieRequestHeader(HttpState, HttpConnection) */ protected void addCookieRequestHeader(HttpState state, HttpConnection conn) - throws IOException, HttpException - { + throws IOException, HttpException { // Do nothing. Not applicable to CONNECT method } @@ -179,8 +176,7 @@ * @see #writeRequestHeaders */ protected void addRequestHeaders(HttpState state, HttpConnection conn) - throws IOException, HttpException - { + throws IOException, HttpException { LOG.trace("enter ConnectMethod.addRequestHeaders(HttpState, " + "HttpConnection)"); addUserAgentRequestHeader(state, conn); @@ -219,13 +215,22 @@ // contains the correct status line, headers & // response body. - LOG.debug("Release connection and fake the response for the original method"); - releaseConnection(); - if (method instanceof HttpMethodBase ) { - // This sucks - ((HttpMethodBase)method).fakeResponse( - this.getStatusLine(), - this.getResponseHeaderGroup()); + LOG.debug("CONNECT failed, fake the response for the original method"); + if (method instanceof HttpMethodBase) { + // Pass the status, headers and response stream to the wrapped + // method. + // To ensure that the connection is not released more than once + // this method is still responsible for releasing the connection. + // This will happen when the response body is consumed, or when + // the wrapped method closes the response connection in + // releaseConnection(). + ((HttpMethodBase) method).fakeResponse( + getStatusLine(), + getResponseHeaderGroup(), + getResponseStream() + ); + } else { + releaseConnection(); } } return code; Index: java/org/apache/commons/httpclient/HttpMethodBase.java =================================================================== RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v retrieving revision 1.139 diff -u -r1.139 HttpMethodBase.java --- java/org/apache/commons/httpclient/HttpMethodBase.java 27 Apr 2003 19:43:41 -0000 1.139 +++ java/org/apache/commons/httpclient/HttpMethodBase.java 27 Apr 2003 21:28:12 -0000 @@ -814,6 +814,13 @@ } /** + * @return the current response stream + */ + protected InputStream getResponseStream() { + return responseStream; + } + + /** * Provide access to the status text * * @return the status text (or "reason phrase") associated with the latest @@ -863,6 +870,8 @@ * only be left open if we are using HTTP1.1 or if "Connection: keep-alive" * was sent. * + * @param conn the connection in question + * * @return boolean true if we should close the connection. */ protected boolean shouldCloseConnection(HttpConnection conn) { @@ -882,14 +891,14 @@ if (connectionHeader != null) { if (connectionHeader.getValue().equalsIgnoreCase("close")) { if (LOG.isDebugEnabled()) { - LOG.debug("Should close connection in response to " + - connectionHeader.toExternalForm()); + LOG.debug("Should close connection in response to " + + connectionHeader.toExternalForm()); } return true; } else if (connectionHeader.getValue().equalsIgnoreCase("keep-alive")) { if (LOG.isDebugEnabled()) { - LOG.debug("Should NOT close connection in response to " + - connectionHeader.toExternalForm()); + LOG.debug("Should NOT close connection in response to " + + connectionHeader.toExternalForm()); } return false; } else { @@ -2627,15 +2636,22 @@ /** * This method is a dirty hack intended to work around * current (2.0) design flaw that prevents the user from - * obraining correct status code and headers from the + * obtaining correct status code, headers and response body from the * preceding HTTP CONNECT method. * * TODO: Remove this crap as soon as possible */ - - protected void fakeResponse(StatusLine statusline, HeaderGroup responseheaders) { + protected void fakeResponse( + StatusLine statusline, + HeaderGroup responseheaders, + InputStream responseStream + ) { + // set used so that the response can be read + this.used = true; this.statusLine = statusline; this.responseHeaders = responseheaders; + this.responseBody = null; + this.responseStream = responseStream; } } Index: test/org/apache/commons/httpclient/TestHttpConnectionManager.java =================================================================== RCS file: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnectionManager.java,v retrieving revision 1.7 diff -u -r1.7 TestHttpConnectionManager.java --- test/org/apache/commons/httpclient/TestHttpConnectionManager.java 13 Apr 2003 03:51:39 -0000 1.7 +++ test/org/apache/commons/httpclient/TestHttpConnectionManager.java 27 Apr 2003 21:28:14 -0000 @@ -108,6 +108,81 @@ assertEquals("MaxConnections", 10, mgr.getMaxConnectionsPerHost()); } + /** + * Test that the ConnectMethod correctly releases connections when + * CONNECT fails. + */ + public void testConnectMethodFailureRelease() { + + MultiThreadedHttpConnectionManager mgr = new MultiThreadedHttpConnectionManager(); + mgr.setMaxTotalConnections(1); + + // we're going to execute a connect method against the localhost, assuming + // that CONNECT is not supported. This should test the fakeResponse() + // code on HttpMethodBase. + HostConfiguration hostConfiguration = new HostConfiguration(); + hostConfiguration.setHost(getHost(), getPort(), getProtocol()); + + GetMethod get = new GetMethod("/"); + try { + HttpConnection connection = mgr.getConnection(hostConfiguration); + ConnectMethod connect = new ConnectMethod(get); + assertTrue(connect.execute(new HttpState(), connection) != 200); + } catch (IOException e) { + e.printStackTrace(); + fail("Error executing connect: " + e); + } + + // this should calling releaseConnection() releases the connection + try { + get.releaseConnection(); + mgr.getConnection(hostConfiguration, 1).releaseConnection(); + } catch (HttpException e1) { + fail("Connection should have been available."); + } + + get = new GetMethod("/"); + + try { + HttpConnection connection = mgr.getConnection(hostConfiguration); + ConnectMethod connect = new ConnectMethod(get); + assertTrue(connect.execute(new HttpState(), connection) != 200); + } catch (IOException e) { + e.printStackTrace(); + fail("Error executing connect: " + e); + } + + // make sure reading the response fully releases the connection + try { + get.getResponseBodyAsString(); + mgr.getConnection(hostConfiguration, 1).releaseConnection(); + } catch (HttpException e1) { + fail("Connection should have been available."); + } + + get = new GetMethod("/"); + + try { + HttpConnection connection = mgr.getConnection(hostConfiguration); + ConnectMethod connect = new ConnectMethod(get); + assertTrue(connect.execute(new HttpState(), connection) != 200); + } catch (IOException e) { + e.printStackTrace(); + fail("Error executing connect: " + e); + } + + // make sure closing the output stream releases the connection + try { + get.getResponseBodyAsStream().close(); + mgr.getConnection(hostConfiguration, 1).releaseConnection(); + } catch (HttpException e) { + fail("Connection should have been available."); + } catch (IOException e) { + e.printStackTrace(); + fail("Close connection failed: " + e); + } + } + public void testGetConnection() { MultiThreadedHttpConnectionManager mgr = new MultiThreadedHttpConnectionManager();