diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientAuthentication.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientAuthentication.java index 71948c390..e03877086 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientAuthentication.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientAuthentication.java @@ -35,8 +35,6 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicLong; -import org.apache.hc.client5.http.ClientProtocolException; -import org.apache.hc.client5.http.NonRepeatableRequestException; import org.apache.hc.client5.http.auth.AuthCache; import org.apache.hc.client5.http.auth.AuthChallenge; import org.apache.hc.client5.http.auth.AuthScheme; @@ -215,7 +213,7 @@ public void testBasicAuthenticationSuccessOnNonRepeatablePutExpectContinue() thr Assert.assertNotNull(entity); } - @Test(expected=ClientProtocolException.class) + @Test public void testBasicAuthenticationFailureOnNonRepeatablePutDontExpectContinue() throws Exception { this.server.registerHandler("*", new EchoHandler()); final HttpHost target = start(); @@ -233,15 +231,11 @@ public void testBasicAuthenticationFailureOnNonRepeatablePutDontExpectContinue() new UsernamePasswordCredentials("test", "boom".toCharArray())); context.setCredentialsProvider(credsProvider); - try { - this.httpclient.execute(target, httpput, context); - Assert.fail("ClientProtocolException should have been thrown"); - } catch (final ClientProtocolException ex) { - final Throwable cause = ex.getCause(); - Assert.assertNotNull(cause); - Assert.assertTrue(cause instanceof NonRepeatableRequestException); - throw ex; - } + final CloseableHttpResponse response = this.httpclient.execute(target, httpput, context); + final HttpEntity entity = response.getEntity(); + Assert.assertEquals(401, response.getCode()); + Assert.assertNotNull(entity); + EntityUtils.consume(entity); } @Test @@ -267,7 +261,7 @@ public void testBasicAuthenticationSuccessOnRepeatablePost() throws Exception { Assert.assertEquals("test realm", authscope.getRealm()); } - @Test(expected=ClientProtocolException.class) + @Test public void testBasicAuthenticationFailureOnNonRepeatablePost() throws Exception { this.server.registerHandler("*", new EchoHandler()); final HttpHost target = start(); @@ -278,19 +272,18 @@ public void testBasicAuthenticationFailureOnNonRepeatablePost() throws Exception new byte[] { 0,1,2,3,4,5,6,7,8,9 }), -1)); final HttpClientContext context = HttpClientContext.create(); + context.setRequestConfig(RequestConfig.custom() + .setExpectContinueEnabled(false) + .build()); final TestCredentialsProvider credsProvider = new TestCredentialsProvider( new UsernamePasswordCredentials("test", "test".toCharArray())); context.setCredentialsProvider(credsProvider); - try { - this.httpclient.execute(target, httppost, context); - Assert.fail("ClientProtocolException should have been thrown"); - } catch (final ClientProtocolException ex) { - final Throwable cause = ex.getCause(); - Assert.assertNotNull(cause); - Assert.assertTrue(cause instanceof NonRepeatableRequestException); - throw ex; - } + final CloseableHttpResponse response = this.httpclient.execute(target, httppost, context); + final HttpEntity entity = response.getEntity(); + Assert.assertEquals(401, response.getCode()); + Assert.assertNotNull(entity); + EntityUtils.consume(entity); } static class TestTargetAuthenticationStrategy extends DefaultAuthenticationStrategy { diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java index c12d6cf33..f89d4a18f 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java @@ -31,9 +31,7 @@ import java.net.URI; import java.util.List; -import org.apache.hc.client5.http.ClientProtocolException; import org.apache.hc.client5.http.HttpRequestRetryHandler; -import org.apache.hc.client5.http.NonRepeatableRequestException; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -161,7 +159,7 @@ public boolean retryRequest( Assert.assertEquals(1, myheaders.length); } - @Test(expected=ClientProtocolException.class) + @Test(expected=IOException.class) public void testNonRepeatableEntity() throws Exception { this.server.registerHandler("*", new SimpleService()); @@ -192,16 +190,7 @@ public boolean retryRequest( new ByteArrayInputStream( new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9 } ), -1)); - - try { - this.httpclient.execute(target, httppost, context); - } catch (final ClientProtocolException ex) { - Assert.assertTrue(ex.getCause() instanceof NonRepeatableRequestException); - final NonRepeatableRequestException nonRepeat = (NonRepeatableRequestException)ex.getCause(); - Assert.assertTrue(nonRepeat.getCause() instanceof IOException); - Assert.assertEquals("a message showing that this failed", nonRepeat.getCause().getMessage()); - throw ex; - } + this.httpclient.execute(target, httppost, context); } @Test diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/NonRepeatableRequestException.java b/httpclient5/src/main/java/org/apache/hc/client5/http/NonRepeatableRequestException.java deleted file mode 100644 index 90287df67..000000000 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/NonRepeatableRequestException.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * ==================================================================== - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ -package org.apache.hc.client5.http; - -import org.apache.hc.core5.http.ProtocolException; - -/** - * Signals failure to retry the request due to non-repeatable request - * entity. - * - * - * @since 4.0 - */ -public class NonRepeatableRequestException extends ProtocolException { - - private static final long serialVersionUID = 82685265288806048L; - - /** - * Creates a new NonRepeatableEntityException with a {@code null} detail message. - */ - public NonRepeatableRequestException() { - super(); - } - - /** - * Creates a new NonRepeatableEntityException with the specified detail message. - * - * @param message The exception detail message - */ - public NonRepeatableRequestException(final String message) { - super(message); - } - - /** - * Creates a new NonRepeatableEntityException with the specified detail message. - * - * @param message The exception detail message - * @param cause the cause - */ - public NonRepeatableRequestException(final String message, final Throwable cause) { - super(message, cause); - } - - - -} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleRequestProducer.java b/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleRequestProducer.java index 69bb6fad2..ffa2f50cd 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleRequestProducer.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/async/methods/SimpleRequestProducer.java @@ -47,16 +47,7 @@ public static SimpleRequestProducer create(final SimpleHttpRequest request, fina if (body.isText()) { entityProducer = new StringAsyncEntityProducer(body.getBodyText(), body.getContentType()); } else { - entityProducer = new BasicAsyncEntityProducer(body.getBodyBytes(), body.getContentType()) { - - //TODO: return the actual content length once - // the entity producers made repeatable in HttpCore - @Override - public long getContentLength() { - return -1; - } - - }; + entityProducer = new BasicAsyncEntityProducer(body.getBodyBytes(), body.getContentType()); } } else { entityProducer = null; diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java index decf267b5..622c2694e 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java @@ -207,16 +207,24 @@ public void completed() { } if (challenged.get()) { - // Reset request headers - final HttpRequest original = scope.originalRequest; - request.setHeaders(); - for (final Iterator
it = original.headerIterator(); it.hasNext(); ) { - request.addHeader(it.next()); - } - try { - internalExecute(challenged, request, entityProducer, scope, chain, asyncExecCallback); - } catch (final HttpException | IOException ex) { - asyncExecCallback.failed(ex); + if (entityProducer != null && !entityProducer.isRepeatable()) { + log.debug("Cannot retry non-repeatable request"); + asyncExecCallback.completed(); + } else { + // Reset request headers + final HttpRequest original = scope.originalRequest; + request.setHeaders(); + for (final Iterator
it = original.headerIterator(); it.hasNext(); ) { + request.addHeader(it.next()); + } + try { + if (entityProducer != null) { + entityProducer.releaseResources(); + } + internalExecute(challenged, request, entityProducer, scope, chain, asyncExecCallback); + } catch (final HttpException | IOException ex) { + asyncExecCallback.failed(ex); + } } } else { asyncExecCallback.completed(); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java index 3b2982061..f59277e74 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRedirectExec.java @@ -166,13 +166,19 @@ public void completed() { if (state.redirectURI == null) { asyncExecCallback.completed(); } else { - try { - if (state.reroute) { - scope.execRuntime.releaseConnection(); + final AsyncEntityProducer entityProducer = state.currentEntityProducer; + if (entityProducer != null && !entityProducer.isRepeatable()) { + log.debug("Cannot redirect non-repeatable request"); + asyncExecCallback.completed(); + } else { + try { + if (state.reroute) { + scope.execRuntime.releaseConnection(); + } + internalExecute(state, chain, asyncExecCallback); + } catch (final IOException | HttpException ex) { + asyncExecCallback.failed(ex); } - internalExecute(state, chain, asyncExecCallback); - } catch (final IOException | HttpException ex) { - asyncExecCallback.failed(ex); } } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRetryExec.java index d08cc1f03..309d44b8c 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRetryExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncRetryExec.java @@ -83,7 +83,9 @@ public void failed(final Exception cause) { if (cause instanceof IOException) { final HttpRoute route = scope.route; final HttpClientContext clientContext = scope.clientContext; - if (retryHandler.retryRequest(request, (IOException) cause, execCount, clientContext)) { + if (entityProducer != null && !entityProducer.isRepeatable()) { + log.debug("Cannot retry non-repeatable request"); + } else if (retryHandler.retryRequest(request, (IOException) cause, execCount, clientContext)) { if (log.isInfoEnabled()) { log.info("I/O exception ("+ cause.getClass().getName() + ") caught when processing request to " @@ -99,6 +101,9 @@ public void failed(final Exception cause) { } try { scope.execRuntime.discardConnection(); + if (entityProducer != null) { + entityProducer.releaseResources(); + } internalExecute(execCount + 1, request, entityProducer, scope, chain, asyncExecCallback); return; } catch (final IOException | HttpException ex) { diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java index 02b4d9e26..0a9f3c9b2 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java @@ -38,6 +38,7 @@ import org.apache.hc.client5.http.async.AsyncExecCallback; import org.apache.hc.client5.http.async.AsyncExecChain; import org.apache.hc.client5.http.async.AsyncExecRuntime; +import org.apache.hc.client5.http.async.methods.SimpleRequestProducer; import org.apache.hc.client5.http.auth.AuthSchemeProvider; import org.apache.hc.client5.http.auth.CredentialsProvider; import org.apache.hc.client5.http.config.Configurable; @@ -201,7 +202,8 @@ public void failed(final Exception cause) { @Override public boolean isRepeatable() { - return false; + //TODO: use AsyncRequestProducer#isRepeatable once available + return requestProducer instanceof SimpleRequestProducer; } @Override diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java index 39ffcefad..96794b370 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java @@ -34,7 +34,6 @@ import org.apache.hc.client5.http.AuthenticationStrategy; import org.apache.hc.client5.http.HttpRoute; -import org.apache.hc.client5.http.NonRepeatableRequestException; import org.apache.hc.client5.http.StandardMethods; import org.apache.hc.client5.http.auth.AuthExchange; import org.apache.hc.client5.http.auth.ChallengeType; @@ -138,15 +137,7 @@ public ClassicHttpResponse execute( final AuthExchange targetAuthExchange = context.getAuthExchange(target); final AuthExchange proxyAuthExchange = proxy != null ? context.getAuthExchange(proxy) : new AuthExchange(); - for (int execCount = 1;; execCount++) { - - if (execCount > 1) { - final HttpEntity entity = request.getEntity(); - if (entity != null && !entity.isRepeatable()) { - throw new NonRepeatableRequestException("Cannot retry request " + - "with a non-repeatable request entity."); - } - } + for (;;) { // Run request protocol interceptors context.setAttribute(HttpClientContext.HTTP_ROUTE, route); @@ -176,12 +167,16 @@ public ClassicHttpResponse execute( // Do not perform authentication for TRACE request return response; } - + final HttpEntity requestEntity = request.getEntity(); + if (requestEntity != null && !requestEntity.isRepeatable()) { + log.debug("Cannot retry non-repeatable request"); + return response; + } if (needAuthentication(targetAuthExchange, proxyAuthExchange, route, request, response, context)) { // Make sure the response body is fully consumed, if present - final HttpEntity entity = response.getEntity(); + final HttpEntity responseEntity = response.getEntity(); if (execRuntime.isConnectionReusable()) { - EntityUtils.consume(entity); + EntityUtils.consume(responseEntity); } else { execRuntime.disconnect(); if (proxyAuthExchange.getState() == AuthExchange.State.SUCCESS diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java index a4fae9058..99b02cb29 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RedirectExec.java @@ -49,6 +49,7 @@ import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpStatus; @@ -110,7 +111,11 @@ public ClassicHttpResponse execute( final ClassicHttpResponse response = chain.proceed(currentRequest, currentScope); try { if (config.isRedirectsEnabled() && this.redirectStrategy.isRedirected(request, response, context)) { - + final HttpEntity requestEntity = request.getEntity(); + if (requestEntity != null && !requestEntity.isRepeatable()) { + this.log.debug("Cannot redirect non-repeatable request"); + return response; + } if (redirectCount >= maxRedirects) { throw new RedirectException("Maximum redirects ("+ maxRedirects + ") exceeded"); } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RetryExec.java index 42167b171..8e7c8a50e 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RetryExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/RetryExec.java @@ -31,7 +31,6 @@ import org.apache.hc.client5.http.HttpRequestRetryHandler; import org.apache.hc.client5.http.HttpRoute; -import org.apache.hc.client5.http.NonRepeatableRequestException; import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecChainHandler; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -88,6 +87,11 @@ public ClassicHttpResponse execute( if (scope.execRuntime.isExecutionAborted()) { throw new RequestFailedException("Request aborted"); } + final HttpEntity requestEntity = request.getEntity(); + if (requestEntity != null && !requestEntity.isRepeatable()) { + this.log.debug("Cannot retry non-repeatable request"); + throw ex; + } if (retryHandler.retryRequest(request, ex, execCount, context)) { if (this.log.isInfoEnabled()) { this.log.info("I/O exception ("+ ex.getClass().getName() + @@ -99,12 +103,6 @@ public ClassicHttpResponse execute( if (this.log.isDebugEnabled()) { this.log.debug(ex.getMessage(), ex); } - final HttpEntity entity = request.getEntity(); - if (entity != null && !entity.isRepeatable()) { - this.log.debug("Cannot retry non-repeatable request"); - throw new NonRepeatableRequestException("Cannot retry request " + - "with a non-repeatable request entity", ex); - } currentRequest = ClassicRequestCopier.INSTANCE.copy(scope.originalRequest); if (this.log.isInfoEnabled()) { this.log.info("Retrying request to " + route); diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java index ca498be93..db1a06443 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java @@ -36,7 +36,6 @@ import org.apache.hc.client5.http.AuthenticationStrategy; import org.apache.hc.client5.http.HttpRoute; -import org.apache.hc.client5.http.NonRepeatableRequestException; import org.apache.hc.client5.http.auth.AuthChallenge; import org.apache.hc.client5.http.auth.AuthExchange; import org.apache.hc.client5.http.auth.AuthScheme; @@ -295,7 +294,7 @@ public void testExecEntityEnclosingRequestRetryOnAuthChallenge() throws Exceptio Assert.assertNull(proxyAuthExchange.getAuthScheme()); } - @Test(expected = NonRepeatableRequestException.class) + @Test public void testExecEntityEnclosingRequest() throws Exception { final HttpRoute route = new HttpRoute(target); final HttpClientContext context = new HttpClientContext(); @@ -335,7 +334,8 @@ public HttpResponse answer(final InvocationOnMock invocationOnMock) throws Throw Mockito.any())).thenReturn(Collections.singletonList(new BasicScheme())); final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); - protocolExec.execute(request, scope, chain); + final ClassicHttpResponse response = protocolExec.execute(request, scope, chain); + Assert.assertEquals(401, response.getCode()); } } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRetryExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRetryExec.java index 030d1757d..61b180271 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRetryExec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestRetryExec.java @@ -32,7 +32,6 @@ import org.apache.hc.client5.http.HttpRequestRetryHandler; import org.apache.hc.client5.http.HttpRoute; -import org.apache.hc.client5.http.NonRepeatableRequestException; import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecRuntime; import org.apache.hc.client5.http.classic.methods.HttpGet; @@ -144,7 +143,7 @@ public void testAbortedRequest() throws Exception { } } - @Test(expected = NonRepeatableRequestException.class) + @Test(expected = IOException.class) public void testNonRepeatableRequest() throws Exception { final HttpRoute route = new HttpRoute(target); final HttpPost originalRequest = new HttpPost("/test");