hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r1084708 - in /httpcomponents/httpclient/branches/4.1.x/httpclient-cache: ./ src/main/java/org/apache/http/client/cache/ src/main/java/org/apache/http/impl/client/cache/ src/test/java/org/apache/http/impl/client/cache/
Date Wed, 23 Mar 2011 20:03:42 GMT
Author: jonm
Date: Wed Mar 23 20:03:42 2011
New Revision: 1084708

URL: http://svn.apache.org/viewvc?rev=1084708&view=rev
Log:
HTTPCLIENT-1073: receiving a 206 (Partial Content) response to a
request that did not request partial content is problematic from
a cache perspective, so throwing a ClientProtocolException is ok.
However, we should properly consume the body of the 206 response
before throwing the exception to ensure that resources are properly
released. (backport of 41084649 from trunk to 4.1.x branch)

Modified:
    httpcomponents/httpclient/branches/4.1.x/httpclient-cache/   (props changed)
    httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCacheStorage.java
  (props changed)
    httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCacheStorage.java
  (props changed)
    httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseProtocolCompliance.java
    httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseProtocolCompliance.java

Propchange: httpcomponents/httpclient/branches/4.1.x/httpclient-cache/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar 23 20:03:42 2011
@@ -1,4 +1,4 @@
 /httpcomponents/httpclient/branches/4.0.x/httpclient-cache:950681-950688
 /httpcomponents/httpclient/branches/branch_4_1/httpclient-cache:755593-811107
 /httpcomponents/httpclient/branches/notice-plugin-test/httpclient-cache:1024348-1031454
-/httpcomponents/httpclient/trunk/httpclient-cache:1080419,1080422,1080575,1084590,1084607,1084610-1084611
+/httpcomponents/httpclient/trunk/httpclient-cache:1080419,1080422,1080575,1084590,1084607,1084610-1084611,1084649

Propchange: httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCacheStorage.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar 23 20:03:42 2011
@@ -1,3 +1,3 @@
 /httpcomponents/httpclient/branches/4.0.x/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java:950681-950688
 /httpcomponents/httpclient/branches/branch_4_1/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java:755593-811107
-/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCacheStorage.java:1080419,1080422,1080575,1084590,1084607,1084610-1084611
+/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCacheStorage.java:1080419,1080422,1080575,1084590,1084607,1084610-1084611,1084649

Propchange: httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCacheStorage.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar 23 20:03:42 2011
@@ -1,3 +1,3 @@
 /httpcomponents/httpclient/branches/4.0.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java:950681-950688
 /httpcomponents/httpclient/branches/branch_4_1/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java:755593-811107
-/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCacheStorage.java:1080419,1080422,1080575,1084590,1084607,1084610-1084611
+/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCacheStorage.java:1080419,1080422,1080575,1084590,1084607,1084610-1084611,1084649

Modified: httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseProtocolCompliance.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseProtocolCompliance.java?rev=1084708&r1=1084707&r2=1084708&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseProtocolCompliance.java
(original)
+++ httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseProtocolCompliance.java
Wed Mar 23 20:03:42 2011
@@ -56,6 +56,8 @@ import org.apache.http.util.EntityUtils;
 @Immutable
 class ResponseProtocolCompliance {
 
+    private static final String UNEXPECTED_PARTIAL_CONTENT = "partial content was returned
for a request that did not ask for it";
+
     /**
      * When we get a response from a down stream server (Origin Server)
      * we attempt to see if it is HTTP 1.1 Compliant and if not, attempt to
@@ -68,8 +70,7 @@ class ResponseProtocolCompliance {
     public void ensureProtocolCompliance(HttpRequest request, HttpResponse response)
             throws IOException {
         if (backendResponseMustNotHaveBody(request, response)) {
-            HttpEntity body = response.getEntity();
-            if (body != null) EntityUtils.consume(body);
+            consumeBody(response);
             response.setEntity(null);
         }
 
@@ -90,6 +91,11 @@ class ResponseProtocolCompliance {
         warningsWithNonMatchingWarnDatesAreRemoved(response);
     }
 
+    private void consumeBody(HttpResponse response) throws IOException {
+        HttpEntity body = response.getEntity();
+        if (body != null) EntityUtils.consume(body);
+    }
+
     private void warningsWithNonMatchingWarnDatesAreRemoved(
             HttpResponse response) {
         Date responseDate = null;
@@ -157,15 +163,13 @@ class ResponseProtocolCompliance {
     }
 
     private void ensurePartialContentIsNotSentToAClientThatDidNotRequestIt(HttpRequest request,
-            HttpResponse response) throws ClientProtocolException {
-        if (request.getFirstHeader(HeaderConstants.RANGE) != null)
+            HttpResponse response) throws IOException {
+        if (request.getFirstHeader(HeaderConstants.RANGE) != null
+                || response.getStatusLine().getStatusCode() != HttpStatus.SC_PARTIAL_CONTENT)

             return;
-
-        if (response.getFirstHeader(HeaderConstants.CONTENT_RANGE) != null) {
-            throw new ClientProtocolException(
-                    "Content-Range was returned for a request that did not ask for a Content-Range.");
-        }
-
+        
+        consumeBody(response);
+        throw new ClientProtocolException(UNEXPECTED_PARTIAL_CONTENT);
     }
 
     private void ensure200ForOPTIONSRequestWithNoBodyHasContentLengthZero(HttpRequest request,

Modified: httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseProtocolCompliance.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseProtocolCompliance.java?rev=1084708&r1=1084707&r2=1084708&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseProtocolCompliance.java
(original)
+++ httpcomponents/httpclient/branches/4.1.x/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestResponseProtocolCompliance.java
Wed Mar 23 20:03:42 2011
@@ -35,6 +35,8 @@ import org.apache.http.HttpRequest;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.HttpVersion;
+import org.apache.http.client.ClientProtocolException;
+import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpHead;
 import org.apache.http.entity.InputStreamEntity;
 import org.apache.http.impl.cookie.DateUtils;
@@ -51,32 +53,76 @@ public class TestResponseProtocolComplia
     public void setUp() {
         impl = new ResponseProtocolCompliance();
     }
+
+    private static class Flag {
+        public boolean set;
+    }
     
-    @Test
-    public void consumesBodyIfOriginSendsOneInResponseToHEAD() throws Exception {
-        HttpRequest req = new HttpHead("http://foo.example.com/");
-        HttpResponse resp = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK,
"OK");
+    private void setMinimalResponseHeaders(HttpResponse resp) {
         resp.setHeader("Date", DateUtils.formatDate(new Date()));
         resp.setHeader("Server", "MyServer/1.0");
-
-        int nbytes = 128;
-        resp.setHeader("Content-Length","" + nbytes);
+    }
+    
+    private ByteArrayInputStream makeTrackableBody(int nbytes, final Flag closed) {
         byte[] buf = HttpTestUtils.getRandomBytes(nbytes);
-        final Flag closed = new Flag();
         ByteArrayInputStream bais = new ByteArrayInputStream(buf) {
             @Override
             public void close() {
                 closed.set = true;
             }
         };
+        return bais;
+    }
+    
+    private HttpResponse makePartialResponse(int nbytes) {
+        HttpResponse resp = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_PARTIAL_CONTENT,
"Partial Content");
+        setMinimalResponseHeaders(resp);
+        resp.setHeader("Content-Length","" + nbytes);
+        resp.setHeader("Content-Range","0-127/256");
+        return resp;
+    }
+    
+    @Test
+    public void consumesBodyIfOriginSendsOneInResponseToHEAD() throws Exception {
+        HttpRequest req = new HttpHead("http://foo.example.com/");
+        int nbytes = 128;
+        HttpResponse resp = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK,
"OK");
+        setMinimalResponseHeaders(resp);
+        resp.setHeader("Content-Length","" + nbytes);
+
+        final Flag closed = new Flag();
+        ByteArrayInputStream bais = makeTrackableBody(nbytes, closed);
         resp.setEntity(new InputStreamEntity(bais, -1));
         
         impl.ensureProtocolCompliance(req, resp);
         assertNull(resp.getEntity());
         assertTrue(closed.set || bais.read() == -1);
     }
-    
-    private static class Flag {
-        public boolean set;
+
+    @Test(expected=ClientProtocolException.class)
+    public void throwsExceptionIfOriginReturnsPartialResponseWhenNotRequested() throws Exception
{
+        HttpRequest req = new HttpGet("http://foo.example.com/");
+        int nbytes = 128;
+        HttpResponse resp = makePartialResponse(nbytes);
+        resp.setEntity(HttpTestUtils.makeBody(nbytes));
+        
+        impl.ensureProtocolCompliance(req, resp);
+    }
+
+    @Test
+    public void consumesPartialContentFromOriginEvenIfNotRequested() throws Exception {
+        HttpRequest req = new HttpGet("http://foo.example.com/");
+        int nbytes = 128;
+        HttpResponse resp = makePartialResponse(nbytes);
+        
+        final Flag closed = new Flag();
+        ByteArrayInputStream bais = makeTrackableBody(nbytes, closed);
+        resp.setEntity(new InputStreamEntity(bais, -1));
+        
+        try {
+            impl.ensureProtocolCompliance(req, resp);
+        } catch (ClientProtocolException expected) {
+        }
+        assertTrue(closed.set || bais.read() == -1);
     }
 }



Mime
View raw message