diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 75d7a01b1..6c64d4153 100644 --- a/RELEASE_NOTES.txt +++ b/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 + * [HTTPCLIENT-948] In rare circumstances the idle connection handling code can leave closed connections in a inconsistent state. Contributed by Oleg Kalnichevski diff --git a/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java b/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java index 3b3d51437..db4eab2ed 100644 --- a/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java +++ b/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 " + diff --git a/httpclient/src/main/java/org/apache/http/impl/client/EntityEnclosingRequestWrapper.java b/httpclient/src/main/java/org/apache/http/impl/client/EntityEnclosingRequestWrapper.java index 8aa9ab27a..2f1d3d1bb 100644 --- a/httpclient/src/main/java/org/apache/http/impl/client/EntityEnclosingRequestWrapper.java +++ b/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); + } + + } + } diff --git a/httpclient/src/test/java/org/apache/http/impl/client/TestClientAuthentication.java b/httpclient/src/test/java/org/apache/http/impl/client/TestClientAuthentication.java index ce43a647a..a7431cb3e 100644 --- a/httpclient/src/test/java/org/apache/http/impl/client/TestClientAuthentication.java +++ b/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()); diff --git a/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java b/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java index ca297cbee..966ed6693 100644 --- a/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java +++ b/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;