diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java index 3fe68cc340d..5531f289740 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java @@ -27,6 +27,7 @@ import java.util.regex.Pattern; import org.eclipse.jetty.client.api.Authentication; import org.eclipse.jetty.client.api.Connection; +import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; @@ -86,7 +87,7 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler { HttpRequest request = (HttpRequest)result.getRequest(); ContentResponse response = new HttpContentResponse(result.getResponse(), getContent(), getMediaType(), getEncoding()); - if (result.isFailed()) + if (result.getResponseFailure() != null) { if (LOG.isDebugEnabled()) LOG.debug("Authentication challenge failed {}", result.getFailure()); @@ -98,7 +99,7 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler HttpConversation conversation = request.getConversation(); if (conversation.getAttribute(authenticationAttribute) != null) { - // We have already tried to authenticate, but we failed again + // We have already tried to authenticate, but we failed again. if (LOG.isDebugEnabled()) LOG.debug("Bad credentials for {}", request); forwardSuccessComplete(request, response); @@ -111,7 +112,7 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler { if (LOG.isDebugEnabled()) LOG.debug("Authentication challenge without {} header", header); - forwardFailureComplete(request, null, response, new HttpResponseException("HTTP protocol violation: Authentication challenge without " + header + " header", response)); + forwardFailureComplete(request, result.getRequestFailure(), response, new HttpResponseException("HTTP protocol violation: Authentication challenge without " + header + " header", response)); return; } @@ -138,9 +139,18 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler return; } + ContentProvider requestContent = request.getContent(); + if (requestContent != null && !requestContent.isReproducible()) + { + if (LOG.isDebugEnabled()) + LOG.debug("Request content not reproducible for {}", request); + forwardSuccessComplete(request, response); + return; + } + try { - final Authentication.Result authnResult = authentication.authenticate(request, response, headerInfo, conversation); + Authentication.Result authnResult = authentication.authenticate(request, response, headerInfo, conversation); if (LOG.isDebugEnabled()) LOG.debug("Authentication result {}", authnResult); if (authnResult == null) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/ContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/ContentProvider.java index 03754f72c9b..6419b11e8f4 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/ContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/ContentProvider.java @@ -22,6 +22,7 @@ import java.io.Closeable; import java.nio.ByteBuffer; import java.util.Iterator; +import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.util.ByteBufferContentProvider; import org.eclipse.jetty.client.util.PathContentProvider; @@ -48,6 +49,22 @@ public interface ContentProvider extends Iterable */ long getLength(); + /** + *

Whether this ContentProvider can produce exactly the same content more + * than once.

+ *

Implementations should return {@code true} only if the content can be + * produced more than once, which means that invocations to {@link #iterator()} + * must return a new, independent, iterator instance over the content.

+ *

The {@link HttpClient} implementation may use this method in particular + * cases where it detects that it is safe to retry a request that failed.

+ * + * @return whether the content can be produced more than once + */ + default boolean isReproducible() + { + return false; + } + /** * An extension of {@link ContentProvider} that provides a content type string * to be used as a {@code Content-Type} HTTP header in requests. diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java index 68386758da9..2bc5233f8be 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java @@ -57,6 +57,12 @@ public class ByteBufferContentProvider extends AbstractTypedContentProvider return length; } + @Override + public boolean isReproducible() + { + return true; + } + @Override public Iterator iterator() { @@ -85,12 +91,6 @@ public class ByteBufferContentProvider extends AbstractTypedContentProvider throw new NoSuchElementException(); } } - - @Override - public void remove() - { - throw new UnsupportedOperationException(); - } }; } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/BytesContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/BytesContentProvider.java index d2c080aa700..39f5fd5ad51 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/BytesContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/BytesContentProvider.java @@ -53,6 +53,12 @@ public class BytesContentProvider extends AbstractTypedContentProvider return length; } + @Override + public boolean isReproducible() + { + return true; + } + @Override public Iterator iterator() { @@ -78,12 +84,6 @@ public class BytesContentProvider extends AbstractTypedContentProvider throw new NoSuchElementException(); } } - - @Override - public void remove() - { - throw new UnsupportedOperationException(); - } }; } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/PathContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/PathContentProvider.java index d1051d94136..00147a556df 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/PathContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/PathContentProvider.java @@ -85,6 +85,12 @@ public class PathContentProvider extends AbstractTypedContentProvider return fileSize; } + @Override + public boolean isReproducible() + { + return true; + } + public ByteBufferPool getByteBufferPool() { return bufferPool; diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java index b591cd7481f..4eea728f923 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.toolchain.test.TestTracker; +import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.After; @@ -96,6 +97,7 @@ public abstract class AbstractHttpClientServerTest clientThreads.setName("client"); client = new HttpClient(transport, sslContextFactory); client.setExecutor(clientThreads); + client.setSocketAddressResolver(new SocketAddressResolver.Sync()); client.start(); } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java index 6e1a5ca074d..14d23267228 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java @@ -22,11 +22,14 @@ import java.io.File; import java.io.IOException; import java.net.URI; import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; +import java.util.Iterator; +import java.util.NoSuchElementException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.IntFunction; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -34,6 +37,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.api.Authentication; import org.eclipse.jetty.client.api.AuthenticationStore; +import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; @@ -41,6 +45,7 @@ import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.client.util.BasicAuthentication; import org.eclipse.jetty.client.util.DeferredContentProvider; import org.eclipse.jetty.client.util.DigestAuthentication; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.security.Authenticator; import org.eclipse.jetty.security.ConstraintMapping; import org.eclipse.jetty.security.ConstraintSecurityHandler; @@ -220,7 +225,7 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest { baseRequest.setHandled(true); if (requests.incrementAndGet() == 1) - response.sendRedirect(URIUtil.newURI(scheme,request.getServerName(),request.getServerPort(),request.getRequestURI(),null)); + response.sendRedirect(URIUtil.newURI(scheme, request.getServerName(), request.getServerPort(), request.getRequestURI(), null)); } }); @@ -259,7 +264,7 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest { baseRequest.setHandled(true); if (request.getRequestURI().endsWith("/redirect")) - response.sendRedirect(URIUtil.newURI(scheme,request.getServerName(),request.getServerPort(),"/secure",null)); + response.sendRedirect(URIUtil.newURI(scheme, request.getServerName(), request.getServerPort(), "/secure", null)); } }); @@ -424,6 +429,40 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest Assert.assertEquals(1, requests.get()); } + @Test + public void test_NonReproducibleContent() throws Exception + { + startBasic(new EmptyServerHandler()); + + AuthenticationStore authenticationStore = client.getAuthenticationStore(); + URI uri = URI.create(scheme + "://localhost:" + connector.getLocalPort()); + BasicAuthentication authentication = new BasicAuthentication(uri, realm, "basic", "basic"); + authenticationStore.addAuthentication(authentication); + + CountDownLatch resultLatch = new CountDownLatch(1); + byte[] data = new byte[]{'h', 'e', 'l', 'l', 'o'}; + DeferredContentProvider content = new DeferredContentProvider(ByteBuffer.wrap(data)) + { + @Override + public boolean isReproducible() + { + return false; + } + }; + Request request = client.newRequest(uri) + .path("/secure") + .content(content); + request.send(result -> + { + if (result.isSucceeded() && result.getResponse().getStatus() == HttpStatus.UNAUTHORIZED_401) + resultLatch.countDown(); + }); + + content.close(); + + Assert.assertTrue(resultLatch.await(5, TimeUnit.SECONDS)); + } + @Test public void test_RequestFailsAfterResponse() throws Exception { @@ -434,32 +473,111 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest BasicAuthentication authentication = new BasicAuthentication(uri, realm, "basic", "basic"); authenticationStore.addAuthentication(authentication); - CountDownLatch successLatch = new CountDownLatch(1); + AtomicBoolean fail = new AtomicBoolean(true); + GeneratingContentProvider content = new GeneratingContentProvider(index -> + { + switch (index) + { + case 0: + return ByteBuffer.wrap(new byte[]{'h', 'e', 'l', 'l', 'o'}); + case 1: + return ByteBuffer.wrap(new byte[]{'w', 'o', 'r', 'l', 'd'}); + case 2: + if (fail.compareAndSet(true, false)) + { + // Wait for the 401 response to arrive + // to the authentication protocol handler. + sleep(1000); + // Trigger request failure. + throw new RuntimeException(); + } + else + { + return null; + } + default: + throw new IllegalStateException(); + } + }); CountDownLatch resultLatch = new CountDownLatch(1); - DeferredContentProvider content = new DeferredContentProvider(); - Request request = client.newRequest("localhost", connector.getLocalPort()) + client.newRequest("localhost", connector.getLocalPort()) .scheme(scheme) .path("/secure") .content(content) - .onResponseSuccess(response -> successLatch.countDown()); - request.send(result -> - { - if (result.isFailed() && result.getResponseFailure() == null) - resultLatch.countDown(); - }); + .send(result -> + { + if (result.isSucceeded() && result.getResponse().getStatus() == HttpStatus.OK_200) + resultLatch.countDown(); + }); - // Send some content to make sure the request is dispatched on the server. - content.offer(ByteBuffer.wrap("hello".getBytes(StandardCharsets.UTF_8))); - - // Wait for the response to arrive to - // the authentication protocol handler. - Thread.sleep(1000); - - // Trigger request failure. - request.abort(new Exception()); - - // Verify that the response was successful, it's the request that failed. - Assert.assertTrue(successLatch.await(5, TimeUnit.SECONDS)); Assert.assertTrue(resultLatch.await(5, TimeUnit.SECONDS)); } + + private void sleep(long time) + { + try + { + Thread.sleep(time); + } + catch (InterruptedException x) + { + throw new RuntimeException(x); + } + } + + private static class GeneratingContentProvider implements ContentProvider + { + private static final ByteBuffer DONE = ByteBuffer.allocate(0); + + private final IntFunction generator; + + private GeneratingContentProvider(IntFunction generator) + { + this.generator = generator; + } + + @Override + public long getLength() + { + return -1; + } + + @Override + public boolean isReproducible() + { + return true; + } + + @Override + public Iterator iterator() + { + return new Iterator() + { + private int index; + public ByteBuffer current; + + @Override + public boolean hasNext() + { + if (current == null) + { + current = generator.apply(index++); + if (current == null) + current = DONE; + } + return current != DONE; + } + + @Override + public ByteBuffer next() + { + ByteBuffer result = current; + current = null; + if (result == null) + throw new NoSuchElementException(); + return result; + } + }; + } + } }