From ef067cba3befbeefc3516b0e703f9b97a8d23471 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Tue, 22 Jun 2010 20:24:39 +0000 Subject: [PATCH] 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/branches/4.0.x@957015 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 4 ++ .../impl/client/DefaultRequestDirector.java | 2 +- .../client/EntityEnclosingRequestWrapper.java | 41 +++++++++++++++++-- .../TestDefaultClientRequestDirector.java | 13 +++++- 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 0425284a73..98fd654d96 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,10 @@ Changes since 4.0.1 ------------------- +* [HTTPCLIENT-951] Non-repeatable entity enclosing requests are not correctly + retried when 'expect-continue' handshake is active. + Contributed by Oleg Kalnichevski + * [HTTPCLIENT-953] IllegalStateException thrown by RouteSpecificPool. Contributed by Guillaume 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 049858b859..25a76eb391 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 @@ -464,7 +464,7 @@ public HttpResponse execute(HttpHost target, HttpRequest request, 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 0ab2da4727..d79df93c98 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) + public EntityEnclosingRequestWrapper(final HttpEntityEnclosingRequest request) throws ProtocolException { super(request); - this.entity = request.getEntity(); + setEntity(request.getEntity()); } public HttpEntity getEntity() { @@ -63,7 +69,8 @@ public HttpEntity getEntity() { } 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 boolean expectContinue() { @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/TestDefaultClientRequestDirector.java b/httpclient/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java index 2c087b2f77..553fe863d4 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 @@ -658,12 +658,13 @@ public HttpResponse execute( 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; } } @@ -744,6 +745,16 @@ public void testNonRepeatableEntity() throws Exception { 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;