diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java index 8754edc4607..894461c8fda 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java @@ -68,7 +68,7 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler @Override public Response.Listener getResponseListener() { - // Return new instances every time to keep track of the response content + // Return new instances every time to keep track of the response content. return new AuthenticationListener(); } @@ -122,6 +122,14 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler super(maxContentLength); } + @Override + public void onSuccess(Response response) + { + // The request may still be sending content, stop it. + Request request = response.getRequest(); + request.abort(new HttpRequestException("Aborting request after receiving a %d response".formatted(response.getStatus()), request)); + } + @Override public void onComplete(Result result) { @@ -159,16 +167,13 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler Authentication authentication = null; Authentication.HeaderInfo headerInfo = null; URI authURI = resolveURI(request, getAuthenticationURI(request)); - if (authURI != null) + for (HeaderInfo element : headerInfos) { - for (Authentication.HeaderInfo element : headerInfos) + authentication = client.getAuthenticationStore().findAuthentication(element.getType(), authURI, element.getRealm()); + if (authentication != null) { - authentication = client.getAuthenticationStore().findAuthentication(element.getType(), authURI, element.getRealm()); - if (authentication != null) - { - headerInfo = element; - break; - } + headerInfo = element; + break; } } if (authentication == null) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java index 7e66b8e12cc..2a9225e9ee0 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java @@ -352,13 +352,6 @@ public class HttpRedirector } } - redirect.onRequestBegin(request -> - { - Throwable cause = httpRequest.getAbortCause(); - if (cause != null) - request.abort(cause); - }); - redirect.send(listener); return redirect; } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java index 8422cf459f8..161a29948a6 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java @@ -52,16 +52,24 @@ public class RedirectProtocolHandler implements ProtocolHandler, Response.Listen public boolean onHeader(Response response, HttpField field) { // Avoid that the content is decoded, which could generate - // errors, since we are discarding the content anyway. + // errors, since we are discarding the response content anyway. return field.getHeader() != HttpHeader.CONTENT_ENCODING; } + @Override + public void onSuccess(Response response) + { + // The request may still be sending content, stop it. + Request request = response.getRequest(); + request.abort(new HttpRequestException("Aborting request after receiving a %d response".formatted(response.getStatus()), request)); + } + @Override public void onComplete(Result result) { Request request = result.getRequest(); Response response = result.getResponse(); - if (result.isSucceeded()) + if (result.getResponseFailure() == null) redirector.redirect(request, response, null); else redirector.fail(request, response, result.getFailure()); diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpExchange.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpExchange.java index ddef6a57d57..cbf84951abf 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpExchange.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpExchange.java @@ -245,15 +245,15 @@ public class HttpExchange implements CyclicTimeouts.Expirable abortResponse = completeResponse(failure); } - if (LOG.isDebugEnabled()) - LOG.debug("Failed {}: req={}/rsp={}", this, abortRequest, abortResponse, failure); - if (!abortRequest && !abortResponse) { promise.succeeded(false); return; } + if (LOG.isDebugEnabled()) + LOG.debug("Failed {}: req={}/rsp={}", this, abortRequest, abortResponse, failure); + // We failed this exchange, deal with it. // Applications could be blocked providing @@ -266,7 +266,7 @@ public class HttpExchange implements CyclicTimeouts.Expirable if (destination.remove(this)) { if (LOG.isDebugEnabled()) - LOG.debug("Aborting while queued {}: {}", this, failure); + LOG.debug("Aborting while queued {}", this, failure); notifyFailureComplete(failure); promise.succeeded(true); return; @@ -279,7 +279,7 @@ public class HttpExchange implements CyclicTimeouts.Expirable // Because this exchange is failed, when associate() is called // it will return false, and the caller will dispose the channel. if (LOG.isDebugEnabled()) - LOG.debug("Aborting before association {}: {}", this, failure); + LOG.debug("Aborting before association {}", this, failure); notifyFailureComplete(failure); promise.succeeded(true); return; @@ -287,7 +287,7 @@ public class HttpExchange implements CyclicTimeouts.Expirable // Case #3: exchange was already associated. if (LOG.isDebugEnabled()) - LOG.debug("Aborting while active {}: {}", this, failure); + LOG.debug("Aborting while active {}", this, failure); channel.abort(this, abortRequest ? failure : null, abortResponse ? failure : null, promise); } diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpRequestAbortTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpRequestAbortTest.java index 45b170a0550..209ed24563a 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpRequestAbortTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpRequestAbortTest.java @@ -491,30 +491,16 @@ public class HttpRequestAbortTest extends AbstractHttpClientServerTest } }); - // The test may fail to abort the request in this way: - // T1 aborts the request, which aborts the sender, which shuts down the output; - // server reads -1 and closes; T2 reads -1 and the receiver fails the response with an EOFException; - // T1 tries to abort the receiver, but it's already failed. - - final Throwable cause = new Exception(); - final AtomicBoolean aborted = new AtomicBoolean(); - final CountDownLatch latch = new CountDownLatch(1); + Throwable cause = new Exception(); client.getProtocolHandlers().clear(); client.getProtocolHandlers().put(new RedirectProtocolHandler(client) { @Override public void onComplete(Result result) { - // Abort the request after the 3xx response but before issuing the next request - if (!result.isFailed()) - { - result.getRequest().abort(cause).thenAccept(b -> - { - aborted.set(b); - latch.countDown(); - }); - } - super.onComplete(result); + // Fake the fact that the redirect failed. + Result newResult = new Result(result, cause); + super.onComplete(newResult); } }); @@ -526,8 +512,6 @@ public class HttpRequestAbortTest extends AbstractHttpClientServerTest .timeout(5, TimeUnit.SECONDS) .send(); }); - assertTrue(latch.await(5, TimeUnit.SECONDS)); - if (aborted.get()) - assertSame(cause, e.getCause()); + assertSame(cause, e.getCause()); } } diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ReproducibleRequestContentTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ReproducibleRequestContentTest.java new file mode 100644 index 00000000000..81ee1f7d968 --- /dev/null +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ReproducibleRequestContentTest.java @@ -0,0 +1,285 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.test.client.transport; + +import java.net.URI; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.client.BasicAuthentication; +import org.eclipse.jetty.client.CompletableResponseListener; +import org.eclipse.jetty.client.ContentResponse; +import org.eclipse.jetty.client.StringRequestContent; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class ReproducibleRequestContentTest extends AbstractTest +{ + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testRedirectWithReproducibleRequestContent(Transport transport) throws Exception + { + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + if (Request.getPathInContext(request).equals("/ok")) + Content.copy(request, response, callback); + else + Response.sendRedirect(request, response, callback, HttpStatus.TEMPORARY_REDIRECT_307, "/ok", true); + return true; + } + }); + + String text = "hello world"; + ContentResponse response = client.newRequest(newURI(transport)) + .method(HttpMethod.POST) + .body(new StringRequestContent(text)) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(text, response.getContentAsString()); + } + + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testBasicAuthenticationWithReproducibleRequestContent(Transport transport) throws Exception + { + String realm = "test-realm"; + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + if (request.getHeaders().contains(HttpHeader.AUTHORIZATION)) + { + Content.copy(request, response, callback); + } + else + { + response.getHeaders().put(HttpHeader.WWW_AUTHENTICATE, "Basic realm=\"%s\"".formatted(realm)); + Response.writeError(request, response, callback, HttpStatus.UNAUTHORIZED_401); + } + return true; + } + }); + + URI uri = newURI(transport); + client.getAuthenticationStore().addAuthentication(new BasicAuthentication(uri, realm, "test", "secret")); + + String text = "hello world"; + ContentResponse response = client.newRequest(uri) + .method(HttpMethod.POST) + .body(new StringRequestContent(text)) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(text, response.getContentAsString()); + } + + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testRedirectWithReproducibleRequestContentSplitAndDelayed(Transport transport) throws Exception + { + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + if (Request.getPathInContext(request).equals("/ok")) + Content.copy(request, response, callback); + else + Response.sendRedirect(request, response, callback, HttpStatus.TEMPORARY_REDIRECT_307, "/ok", true); + return true; + } + }); + + String text1 = "hello"; + String text2 = "world"; + ReproducibleAsyncRequestContent body = new ReproducibleAsyncRequestContent(); + body.write(StandardCharsets.UTF_8.encode(text1)); + CompletableFuture completable = new CompletableResponseListener( + client.newRequest(newURI(transport)) + .method(HttpMethod.POST) + .body(body) + ).send(); + + // The request was sent, wait for the server to redirect. + Thread.sleep(1000); + + // Complete the request content. + body.write(StandardCharsets.UTF_8.encode(text2)); + body.close(); + + ContentResponse response = completable.get(5, TimeUnit.SECONDS); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(text1 + text2, response.getContentAsString()); + } + + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testBasicAuthenticationWithReproducibleRequestContentSplitAndDelayed(Transport transport) throws Exception + { + String realm = "test-realm"; + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + if (request.getHeaders().contains(HttpHeader.AUTHORIZATION)) + { + Content.copy(request, response, callback); + } + else + { + response.getHeaders().put(HttpHeader.WWW_AUTHENTICATE, "Basic realm=\"%s\"".formatted(realm)); + Response.writeError(request, response, callback, HttpStatus.UNAUTHORIZED_401); + } + return true; + } + }); + + URI uri = newURI(transport); + client.getAuthenticationStore().addAuthentication(new BasicAuthentication(uri, realm, "test", "secret")); + + String text1 = "hello"; + String text2 = "world"; + ReproducibleAsyncRequestContent body = new ReproducibleAsyncRequestContent(); + body.write(StandardCharsets.UTF_8.encode(text1)); + CompletableFuture completable = new CompletableResponseListener( + client.newRequest(newURI(transport)) + .method(HttpMethod.POST) + .body(body) + ).send(); + + // The request was sent, wait for the server to reply with 401. + Thread.sleep(1000); + + // Complete the request content. + body.write(StandardCharsets.UTF_8.encode(text2)); + body.close(); + + ContentResponse response = completable.get(5, TimeUnit.SECONDS); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(text1 + text2, response.getContentAsString()); + } + + private static class ReproducibleAsyncRequestContent implements org.eclipse.jetty.client.Request.Content, AutoCloseable + { + private static final ByteBuffer EOF = ByteBuffer.allocate(0); + + private final AutoLock lock = new AutoLock(); + private final List chunks = new ArrayList<>(); + private Runnable demand; + private int index; + + @Override + public Content.Chunk read() + { + try (AutoLock ignored = lock.lock()) + { + if (index == chunks.size()) + return null; + ByteBuffer byteBuffer = chunks.get(index); + if (byteBuffer == EOF) + return Content.Chunk.EOF; + ++index; + return Content.Chunk.from(byteBuffer.slice(), false); + } + } + + @Override + public void demand(Runnable demandCallback) + { + boolean invoke; + try (AutoLock ignored = lock.lock()) + { + if (demand != null) + throw new IllegalStateException(); + invoke = index < chunks.size(); + if (!invoke) + demand = demandCallback; + } + if (invoke) + invokeDemand(demandCallback); + } + + private void invokeDemand(Runnable demandCallback) + { + demandCallback.run(); + } + + @Override + public void fail(Throwable failure) + { + // Nothing to do in this simple implementation. + } + + @Override + public boolean rewind() + { + try (AutoLock ignored = lock.lock()) + { + demand = null; + index = 0; + } + return true; + } + + public void write(ByteBuffer byteBuffer) + { + offer(byteBuffer); + } + + @Override + public void close() + { + offer(EOF); + } + + private void offer(ByteBuffer byteBuffer) + { + Runnable demandCallback = null; + try (AutoLock ignored = lock.lock()) + { + if (index == chunks.size()) + { + demandCallback = demand; + demand = null; + } + chunks.add(byteBuffer); + } + if (demandCallback != null) + invokeDemand(demandCallback); + } + } +} diff --git a/pom.xml b/pom.xml index 60ffcce1fef..caa899d587d 100644 --- a/pom.xml +++ b/pom.xml @@ -2365,6 +2365,7 @@ true + org/eclipse/jetty/version/build.properties about.html .api_description about_files/LICENSE-2.0.txt