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

Merge pull request #4428 from eclipse/jetty-9.4.x-4427-httpclient_retried_request_duplicates_cookies 

Fixes #4427 - Retried request duplicates cookies.
This commit is contained in:
Simone Bordet 2019-12-19 22:00:15 +01:00 committed by GitHub
parent d1d8f9a779 91239b01e3
commit af7cb94528
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 218 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java
View File

@ -84,6 +84,12 @@ public abstract class HttpConnection implements Connection
protected void normalizeRequest(Request request)
{
boolean normalized = ((HttpRequest)request).normalized();
if (LOG.isDebugEnabled())
LOG.debug("Normalizing {} {}", !normalized, request);
if (normalized)
return;
HttpVersion version = request.getVersion();
HttpFields headers = request.getHeaders();
ContentProvider content = request.getContent();

14
jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java
View File

@ -326,10 +326,10 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
}
}
public boolean process(final Connection connection)
public boolean process(Connection connection)
{
HttpClient client = getHttpClient();
final HttpExchange exchange = getHttpExchanges().poll();
HttpExchange exchange = getHttpExchanges().poll();
if (LOG.isDebugEnabled())
LOG.debug("Processing exchange {} on {} of {}", exchange, connection, this);
if (exchange == null)
@ -346,7 +346,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
}
else
{
final Request request = exchange.getRequest();
Request request = exchange.getRequest();
Throwable cause = request.getAbortCause();
if (cause != null)
{
@ -368,9 +368,13 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
if (LOG.isDebugEnabled())
LOG.debug("Send failed {} for {}", result, exchange);
if (result.retry)
{
// Resend this exchange, likely on another connection,
// and return false to avoid to re-enter this method.
send(exchange);
else
request.abort(result.failure);
return false;
}
request.abort(result.failure);
}
}
return getHttpExchanges().peek() != null;

18
jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java
View File

@ -88,6 +88,7 @@ public class HttpRequest implements Request
private BiFunction<Request, Request, Response.CompleteListener> pushListener;
private Supplier<HttpFields> trailers;
private Object tag;
private boolean normalized;
protected HttpRequest(HttpClient client, HttpConversation conversation, URI uri)
{
@ -803,6 +804,23 @@ public class HttpRequest implements Request
return aborted.get();
}
/**
* <p>Marks this request as <em>normalized</em>.</p>
* <p>A request is normalized by setting things that applications give
* for granted such as defaulting the method to {@code GET}, adding the
* {@code Host} header, adding the cookies, adding {@code Authorization}
* headers, etc.</p>
*
* @return whether this request was already normalized
* @see HttpConnection#normalizeRequest(Request)
*/
boolean normalized()
{
boolean result = normalized;
normalized = true;
return result;
}
private String buildQuery()
{
StringBuilder result = new StringBuilder();

6
jetty-client/src/main/java/org/eclipse/jetty/client/SendFailure.java
View File

@ -32,6 +32,10 @@ public class SendFailure
@Override
public String toString()
{
return String.format("%s[failure=%s,retry=%b]", super.toString(), failure, retry);
return String.format("%s@%x[failure=%s,retry=%b]",
getClass().getSimpleName(),
hashCode(),
failure,
retry);
}
}

7
jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java
View File

@ -139,12 +139,17 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements Connec
public boolean onIdleExpired()
{
long idleTimeout = getEndPoint().getIdleTimeout();
boolean close = delegate.onIdleTimeout(idleTimeout);
boolean close = onIdleTimeout(idleTimeout);
if (close)
close(new TimeoutException("Idle timeout " + idleTimeout + " ms"));
return false;
}
protected boolean onIdleTimeout(long idleTimeout)
{
return delegate.onIdleTimeout(idleTimeout);
}
@Override
public void onFillable()
{

174
jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientIdleTimeoutTest.java Normal file
View File

@ -0,0 +1,174 @@
//
// ========================================================================
// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.client;
import java.io.IOException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import javax.servlet.ServletException;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
import org.eclipse.jetty.client.http.HttpConnectionOverHTTP;
import org.eclipse.jetty.client.http.HttpDestinationOverHTTP;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.util.Promise;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class HttpClientIdleTimeoutTest
{
private Server server;
private ServerConnector connector;
private HttpClient client;
private void start(Handler handler) throws Exception
{
QueuedThreadPool serverThreads = new QueuedThreadPool();
serverThreads.setName("server");
server = new Server(serverThreads);
connector = new ServerConnector(server, 1, 1);
server.addConnector(connector);
server.setHandler(handler);
server.start();
}
@AfterEach
public void dispose() throws Exception
{
if (server != null)
server.stop();
if (client != null)
client.stop();
}
@Test
public void testRequestIsRetriedWhenSentDuringIdleTimeout() throws Exception
{
start(new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
Cookie[] cookies = request.getCookies();
if (cookies == null || cookies.length == 0)
{
// Send a cookie in the first response.
response.addCookie(new Cookie("name", "value"));
}
else
{
// Verify that there is only one cookie, i.e.
// that the request has not been normalized twice.
assertEquals(1, cookies.length);
}
}
});
CountDownLatch idleTimeoutLatch = new CountDownLatch(1);
CountDownLatch requestLatch = new CountDownLatch(1);
CountDownLatch retryLatch = new CountDownLatch(1);
QueuedThreadPool clientThreads = new QueuedThreadPool();
clientThreads.setName("client");
client = new HttpClient(new HttpClientTransportOverHTTP(1)
{
@Override
public HttpDestination newHttpDestination(Origin origin)
{
return new HttpDestinationOverHTTP(getHttpClient(), origin)
{
@Override
protected SendFailure send(Connection connection, HttpExchange exchange)
{
SendFailure result = super.send(connection, exchange);
if (result != null && result.retry)
retryLatch.countDown();
return result;
}
};
}
@Override
protected HttpConnectionOverHTTP newHttpConnection(EndPoint endPoint, HttpDestination destination, Promise<Connection> promise)
{
return new HttpConnectionOverHTTP(endPoint, destination, promise)
{
@Override
protected boolean onIdleTimeout(long idleTimeout)
{
boolean result = super.onIdleTimeout(idleTimeout);
if (result)
idleTimeoutLatch.countDown();
assertTrue(await(requestLatch));
return result;
}
};
}
}, null);
client.setExecutor(clientThreads);
client.start();
long idleTimeout = 1000;
client.setIdleTimeout(idleTimeout);
// Create one connection.
ContentResponse response = client.newRequest("localhost", connector.getLocalPort()).send();
assertEquals(response.getStatus(), HttpStatus.OK_200);
assertTrue(idleTimeoutLatch.await(2 * idleTimeout, TimeUnit.MILLISECONDS));
// Send a request exactly while the connection is idle timing out.
CountDownLatch responseLatch = new CountDownLatch(1);
client.newRequest("localhost", connector.getLocalPort()).send(result ->
{
assertTrue(result.isSucceeded());
assertEquals(HttpStatus.OK_200, result.getResponse().getStatus());
responseLatch.countDown();
});
assertTrue(retryLatch.await(5, TimeUnit.SECONDS));
requestLatch.countDown();
assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
}
private boolean await(CountDownLatch latch)
{
try
{
return latch.await(15, TimeUnit.SECONDS);
}
catch (InterruptedException x)
{
throw new RuntimeException(x);
}
}
}