HTTPCLIENT-951: Non-repeatable entity enclosing requests are not correctly retried when 'expect-continue' handshake is active

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@956989 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,6 +1,10 @@
 Changes since 4.1 ALPHA2
 -------------------
 
+* [HTTPCLIENT-951] Non-repeatable entity enclosing requests are not correctly
+  retried when 'expect-continue' handshake is active.
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * [HTTPCLIENT-948] In rare circumstances the idle connection handling code  
   can leave closed connections in a inconsistent state.
   Contributed by Oleg Kalnichevski <olegk at apache.org>

httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java

@@ -608,7 +608,7 @@ public class DefaultRequestDirector implements RequestDirector {
             execCount++;
             // Increment exec count for this particular request
             wrapper.incrementExecCount();
-            if (wrapper.getExecCount() > 1 && !wrapper.isRepeatable()) {
+            if (!wrapper.isRepeatable()) {
                 this.log.debug("Cannot retry non-repeatable request");
                 if (retryReason != null) {
                     throw new NonRepeatableRequestException("Cannot retry request " +

httpclient/src/main/java/org/apache/http/impl/client/EntityEnclosingRequestWrapper.java

@@ -27,7 +27,12 @@
 
 package org.apache.http.impl.client;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
 import org.apache.http.annotation.NotThreadSafe;
+import org.apache.http.entity.HttpEntityWrapper;
 
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
@@ -51,11 +56,12 @@ public class EntityEnclosingRequestWrapper extends RequestWrapper
     implements HttpEntityEnclosingRequest {
 
     private HttpEntity entity;
+    private boolean consumed;
     
     public EntityEnclosingRequestWrapper(final HttpEntityEnclosingRequest request)
         throws ProtocolException {
         super(request);
-        this.entity = request.getEntity();
+        setEntity(request.getEntity());
     }
 
     public HttpEntity getEntity() {
@@ -63,7 +69,8 @@ public class EntityEnclosingRequestWrapper extends RequestWrapper
     }
 
     public void setEntity(final HttpEntity entity) {
-        this.entity = entity;
+        this.entity = entity != null ? new EntityWrapper(entity) : null;
+        this.consumed = false;
     }
 
     public boolean expectContinue() {
@@ -73,7 +80,33 @@ public class EntityEnclosingRequestWrapper extends RequestWrapper
 
     @Override
     public boolean isRepeatable() {
-        return this.entity == null || this.entity.isRepeatable();
+        return this.entity == null || this.entity.isRepeatable() || !this.consumed;
+    }
+
+    class EntityWrapper extends HttpEntityWrapper {
+        
+        EntityWrapper(final HttpEntity entity) {
+            super(entity);
+        }
+
+        @Override
+        public void consumeContent() throws IOException {
+            consumed = true;
+            super.consumeContent();
+        }
+
+        @Override
+        public InputStream getContent() throws IOException {
+            consumed = true;
+            return super.getContent();
+        }
+
+        @Override
+        public void writeTo(final OutputStream outstream) throws IOException {
+            consumed = true;
+            super.writeTo(outstream);
+        }
+        
     }
     
 }

httpclient/src/test/java/org/apache/http/impl/client/TestClientAuthentication.java

@@ -62,7 +62,6 @@ import org.apache.http.protocol.ResponseDate;
 import org.apache.http.protocol.ResponseServer;
 import org.junit.Assert;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -219,7 +218,7 @@ public class TestClientAuthentication extends BasicServerTestBase {
         Assert.assertEquals("test realm", authscope.getRealm());
     }
 
-    @Test @Ignore
+    @Test
     public void testBasicAuthenticationSuccessOnNonRepeatablePutExpectContinue() throws Exception {
         BasicHttpProcessor httpproc = new BasicHttpProcessor();
         httpproc.addInterceptor(new ResponseDate());

httpclient/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java

@@ -653,12 +653,13 @@ public class TestDefaultClientRequestDirector extends BasicServerTestBase {
                 final HttpClientConnection conn,
                 final HttpContext context) throws IOException, HttpException {
 
+            HttpResponse response = super.execute(request, conn, context);
             Object marker = context.getAttribute(MARKER);
             if (marker == null) {
                 context.setAttribute(MARKER, Boolean.TRUE);
                 throw new IOException(failureMsg);
             }
-            return super.execute(request, conn, context);
+            return response;
         }
 
     }
@@ -740,6 +741,16 @@ public class TestDefaultClientRequestDirector extends BasicServerTestBase {
         String failureMsg = "a message showing that this failed";
 
         FaultyHttpClient client = new FaultyHttpClient(failureMsg);
+        client.setHttpRequestRetryHandler(new HttpRequestRetryHandler() {
+
+            public boolean retryRequest(
+                    final IOException exception,
+                    int executionCount,
+                    final HttpContext context) {
+                return true;
+            }
+
+        });
         HttpContext context = new BasicHttpContext();
 
         String s = "http://localhost:" + port;