Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixes #10879 - Improve redirect handling with reproducible content (#10880) 

* Fixes #10879 - Improve redirect handling with reproducible content

Now both the redirect and the authentication ProtocolHandlers will abort the request on response success.
If the request is already completed, the abort attempt will be a no-op, proceeding as usual.
Otherwise, the request upload will be aborted and a new request sent with the reproducible content.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Co-authored-by: Olivier Lamy <olamy@apache.org>
This commit is contained in:
Simone Bordet 2023-11-17 11:53:36 +01:00 committed by GitHub
parent 11c510af1b
commit d69dfc8d23
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 321 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java
View File

@ -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)

7
jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java
View File

@ -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;
}

12
jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java
View File

@ -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());

12
jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpExchange.java
View File

@ -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);
}

26
jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpRequestAbortTest.java
View File

@ -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());
}
}

285
jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ReproducibleRequestContentTest.java Normal file
View File

@ -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<ContentResponse> 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<ContentResponse> 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<ByteBuffer> 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);
}
}
}

1
pom.xml
View File

@ -2365,6 +2365,7 @@
<!-- <failBuildInCaseOfConflict>true</failBuildInCaseOfConflict>-->
<failBuildInCaseOfDifferentContentConflict>true</failBuildInCaseOfDifferentContentConflict>
<ignoredResourcePatterns>
<ignoredResourcePattern>org/eclipse/jetty/version/build.properties</ignoredResourcePattern>
<ignoredResourcePattern>about.html</ignoredResourcePattern>
<ignoredResourcePattern>.api_description</ignoredResourcePattern>
<ignoredResourcePattern>about_files/LICENSE-2.0.txt</ignoredResourcePattern>