hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Johnson <e...@tibco.com>
Subject Re: [PATCH] Re: Bug in Head Method (with authenticated server)
Date Tue, 17 Dec 2002 14:39:54 GMT
Ortwin,

If I read the patch correctly, I think it is actually more complicated 
than need be.

Specifically, the "exhaust" function is equivalent to calling "close" on 
the _response_ input stream (which is a wrapping of the underlying 
stream).  So instead, you should remove the "exhaust" call and call 
"close" on the stream instead.  Even that might be more complicated than 
needed.

Let me see if I can express the problem(s) as set of cases:

   1. Normal HEAD response - does not include Content-Length or
      Transfer-Encoding in response.  Response body should be assumed to
      be zero-length, whereas HttpMethodBase._readResponseBody() would
      otherwise assume that the response ends when the underlying stream
      is closed.
   2. Non-compliant response - includes a Content-Length or
      Transfer-Encoding header - and expects the body to be read (not
      consistent with RFC 2616!).
   3. Either of the above, including a "Connection: close" header.
       (Latest Tomcat with the deprecated HttpConnector instead of
      Coyote will always send back a "Connection: close" on a request
      that does not include a content length indication of some sort,
      such as a HEAD request - even when it isn't necessary).

I think an easier way to make it all work would be to change the 
HttpMethodBase.canResponseHaveBody() function to check for the method 
being "HEAD".  Then the override of readResponseBody() in HeadMethod 
could be done away with altogether, as well as the responseHasBody() 
function.

I'm just guessing, though.  I don't have time to try it out at the moment.

In any case, I would like to see a test case submitted with the problem 
that actually demonstrates the failure, so that we have a reproducible 
way of assuring ourselves that we don't reintroduce the problem at a 
later date.

-Eric.

Ortwin Gl├╝ck wrote:

> This has been addressed now.
>
> Happy?
>
> Pill, Juergen wrote:
>
>> 2) performance is very slow, I believe the second call (rsp=200) does 
>> not
>> deliever a body, but a content length, now the client waits until the 
>> Stream
>> is closed.
>
>
>------------------------------------------------------------------------
>
>? patch.diff
>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.90
>diff -u -w -r1.90 HttpMethodBase.java
>--- java/org/apache/commons/httpclient/HttpMethodBase.java	16 Dec 2002 05:16:04 -0000
1.90
>+++ java/org/apache/commons/httpclient/HttpMethodBase.java	17 Dec 2002 12:53:15 -0000
>@@ -1647,8 +1647,7 @@
>         if (stream == null) {
>             // done using the connection!
>             responseBodyConsumed();
>-        }
>-        else {
>+        } else {
>             conn.setLastResponseInputStream(stream);
>             setResponseStream(stream);
>         }
>@@ -2124,6 +2123,20 @@
>     }
> 
>     /**
>+     * Checks if the response heaeders will be followed by a response body.
>+     * Per RFC 2616 this is the case if the server sends either a Content-Length
>+     * header or a Transfer-Encoding header.
>+     * This method should only be called after headers are read. Typically from
>+     * the readResponseBody().
>+     * @return true if either content-length or transfer-encoding headers are present.
>+     */
>+    protected boolean responseHasBody() {
>+        Header lengthHeader = getResponseHeader("Content-Length");
>+        Header transferEncodingHeader = getResponseHeader("Transfer-Encoding");
>+        return ((lengthHeader != null) || (transferEncodingHeader != null));
>+    }
>+
>+    /**
>      * process a response that requires authentication
>      *
>      * @param state the current state
>@@ -2354,6 +2367,15 @@
>      */
>     public int getRecoverableExceptionCount() {
>         return recoverableExceptionCount;
>+    }
>+
>+    /**
>+     * Reads a stream completely to the end and discards all data.
>+     * @param in The stream to exhaust.
>+     */
>+    public static void exhaust(InputStream in) throws IOException {
>+        byte[] buffer = new byte[10000];
>+        while (in.read(buffer) != -1) ;
>     }
> 
>     /**
>Index: java/org/apache/commons/httpclient/methods/HeadMethod.java
>===================================================================
>RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/methods/HeadMethod.java,v
>retrieving revision 1.12
>diff -u -w -r1.12 HeadMethod.java
>--- java/org/apache/commons/httpclient/methods/HeadMethod.java	9 Dec 2002 09:16:17 -0000
1.12
>+++ java/org/apache/commons/httpclient/methods/HeadMethod.java	17 Dec 2002 12:53:16 -0000
>@@ -62,6 +62,7 @@
> package org.apache.commons.httpclient.methods;
> 
> import java.io.IOException;
>+import java.io.InputStream;
> 
> import org.apache.commons.httpclient.HttpConnection;
> import org.apache.commons.httpclient.HttpException;
>@@ -148,7 +149,12 @@
> 
>         // despite the possible presence of a content-length header,
>         // HEAD returns no response body
>-        responseBodyConsumed();
>+        super.readResponseBody(state, conn);
>+        InputStream in = this.getResponseBodyAsStream();
>+        if ((in != null) && responseHasBody()) {
>+          exhaust(in);
>+        }
>+        //responseBodyConsumed();
>         return;
>     }
> }
>
>  
>
>------------------------------------------------------------------------
>
>--
>To unsubscribe, e-mail:   <mailto:commons-httpclient-dev-unsubscribe@jakarta.apache.org>
>For additional commands, e-mail: <mailto:commons-httpclient-dev-help@jakarta.apache.org>
>


Mime
View raw message