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

Merged branch 'jetty-9.3.x' into 'jetty-9.4.x'.

This commit is contained in:
Simone Bordet 2018-08-27 10:29:12 +02:00
parent 33a1367325 29d466e704
commit 5b5f2fcf5f
4 changed files with 114 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -426,16 +426,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
if (getHttpExchanges().isEmpty())
{
if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty())
{
// There is a race condition between this thread removing the destination
// and another thread queueing a request to this same destination.
// If this destination is removed, but the request queued, a new connection
// will be opened, the exchange will be executed and eventually the connection
// will idle timeout and be closed. Meanwhile a new destination will be created
// in HttpClient and will be used for other requests.
getHttpClient().removeDestination(this);
}
tryRemoveIdleDestination();
}
else
{
@ -460,6 +451,22 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
// The call to Request.abort() will remove the exchange from the exchanges queue.
for (HttpExchange exchange : new ArrayList<>(exchanges))
exchange.getRequest().abort(cause);
if (exchanges.isEmpty())
tryRemoveIdleDestination();
}
private void tryRemoveIdleDestination()
{
if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty())
{
// There is a race condition between this thread removing the destination
// and another thread queueing a request to this same destination.
// If this destination is removed, but the request queued, a new connection
// will be opened, the exchange will be executed and eventually the connection
// will idle timeout and be closed. Meanwhile a new destination will be created
// in HttpClient and will be used for other requests.
getHttpClient().removeDestination(this);
}
}
@Override

28
jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java
View File

@ -32,6 +32,7 @@ import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Destination;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.util.ssl.SslContextFactory;
@ -268,6 +269,33 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
Assert.assertNotSame(destinationBefore, destinationAfter);
}
@Test
public void testDestinationIsRemovedAfterConnectionError() throws Exception
{
String host = "localhost";
int port = connector.getLocalPort();
client.setRemoveIdleDestinations(true);
Assert.assertTrue("Destinations of a fresh client must be empty", client.getDestinations().isEmpty());
server.stop();
Request request = client.newRequest(host, port).scheme(this.scheme);
try
{
request.send();
Assert.fail("Request to a closed port must fail");
}
catch (Exception expected)
{
}
long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(1);
while (!client.getDestinations().isEmpty() && System.nanoTime() < deadline)
{
Thread.sleep(10);
}
Assert.assertTrue("Destination must be removed after connection error", client.getDestinations().isEmpty());
}
private Connection pollIdleConnection(DuplexConnectionPool connectionPool, long time, TimeUnit unit) throws InterruptedException
{
return await(() -> connectionPool.getIdleConnections().poll(), time, unit);

11
jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View File

@ -2449,8 +2449,6 @@ public class Request implements HttpServletRequest
/* ------------------------------------------------------------ */
public void mergeQueryParameters(String oldQuery,String newQuery, boolean updateQueryString)
{
// TODO This is seriously ugly
MultiMap<String> newQueryParams = null;
// Have to assume ENCODING because we can't know otherwise.
if (newQuery!=null)
@ -2463,7 +2461,14 @@ public class Request implements HttpServletRequest
if (oldQueryParams == null && oldQuery != null)
{
oldQueryParams = new MultiMap<>();
UrlEncoded.decodeTo(oldQuery, oldQueryParams, getQueryEncoding());
try
{
UrlEncoded.decodeTo(oldQuery, oldQueryParams, getQueryEncoding());
}
catch(Throwable th)
{
throw new BadMessageException(400,"Bad query encoding",th);
}
}
MultiMap<String> mergedQueryParams;

64
jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java
View File

@ -18,6 +18,7 @@
package org.eclipse.jetty.servlet;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
@ -59,6 +60,8 @@ import org.eclipse.jetty.server.handler.ResourceHandler;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.UrlEncoded;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@ -157,6 +160,42 @@ public class DispatcherTest
assertEquals(expected, responses);
}
@Test
public void testForwardWithBadParams() throws Exception
{
try(StacklessLogging nostack = new StacklessLogging(ServletHandler.class))
{
Log.getLogger(ServletHandler.class).info("Expect Not valid UTF8 warnings...");
_contextHandler.addServlet(AlwaysForwardServlet.class, "/forward/*");
_contextHandler.addServlet(EchoServlet.class, "/echo/*");
String response;
response = _connector.getResponse("GET /context/forward/?echo=allgood HTTP/1.0\n\n");
assertThat(response,containsString(" 200 OK"));
assertThat(response,containsString("allgood"));
response = _connector.getResponse("GET /context/forward/params?echo=allgood HTTP/1.0\n\n");
assertThat(response,containsString(" 200 OK"));
assertThat(response,containsString("allgood"));
assertThat(response,containsString("forward"));
response = _connector.getResponse("GET /context/forward/badparams?echo=badparams HTTP/1.0\n\n");
assertThat(response,containsString(" 500 "));
response = _connector.getResponse("GET /context/forward/?echo=badclient&bad=%88%A4 HTTP/1.0\n\n");
assertThat(response,containsString(" 400 "));
response = _connector.getResponse("GET /context/forward/params?echo=badclient&bad=%88%A4 HTTP/1.0\n\n");
assertThat(response,containsString(" 400 "));
response = _connector.getResponse("GET /context/forward/badparams?echo=badclientandparam&bad=%88%A4 HTTP/1.0\n\n");
assertThat(response,containsString(" 500 "));
}
}
@Test
public void testInclude() throws Exception
{
@ -358,6 +397,20 @@ public class DispatcherTest
dispatcher.forward(request, response);
}
}
public static class AlwaysForwardServlet extends HttpServlet implements Servlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
if ("/params".equals(request.getPathInfo()))
getServletContext().getRequestDispatcher("/echo?echo=forward").forward(request, response);
else if ("/badparams".equals(request.getPathInfo()))
getServletContext().getRequestDispatcher("/echo?echo=forward&fbad=%88%A4").forward(request, response);
else
getServletContext().getRequestDispatcher("/echo").forward(request, response);
}
}
public static class ForwardNonUTF8Servlet extends HttpServlet implements Servlet
@ -484,15 +537,20 @@ public class DispatcherTest
@Override
public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException
{
String echoText = req.getParameter("echo");
String[] echoText = req.getParameterValues("echo");
if ( echoText == null )
if ( echoText == null || echoText.length==0)
{
throw new ServletException("echo is a required parameter");
}
else if (echoText.length==1)
{
res.getWriter().print(echoText[0]);
}
else
{
res.getWriter().print(echoText);
for (String text:echoText)
res.getWriter().print(text);
}
}
}