Merge branch 'jetty-9.4.x' into jetty-9.4.x-2232-HttpParser-cleanup

This commit is contained in:
Greg Wilkins 2018-03-14 09:32:51 +11:00
commit c1f038d3bf
20 changed files with 161 additions and 161 deletions

View File

@ -26,7 +26,7 @@ import org.eclipse.jetty.client.HttpExchange;
import org.eclipse.jetty.client.HttpReceiver;
import org.eclipse.jetty.client.HttpResponse;
import org.eclipse.jetty.client.HttpResponseException;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpParser;
@ -325,13 +325,13 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
}
@Override
public void badMessage(int status, String reason)
public void badMessage(BadMessageException failure)
{
HttpExchange exchange = getHttpExchange();
if (exchange != null)
{
HttpResponse response = exchange.getResponse();
response.status(status).reason(reason);
response.status(failure.getCode()).reason(failure.getReason());
failAndClose(new HttpResponseException("HTTP protocol violation: bad response on " + getHttpConnection(), response));
}
}

View File

@ -18,8 +18,6 @@
package org.eclipse.jetty.client;
import static org.junit.Assert.assertThat;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@ -1297,8 +1295,8 @@ public class HttpClientTest extends AbstractHttpClientServerTest
}
catch(ExecutionException e)
{
assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
Assert.assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
Assert.assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
}
}
@ -1311,8 +1309,8 @@ public class HttpClientTest extends AbstractHttpClientServerTest
}
catch(ExecutionException e)
{
assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
Assert.assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
Assert.assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
}
}

View File

@ -305,9 +305,9 @@ public class ResponseContentParser extends StreamContentParser
}
@Override
public void badMessage(int status, String reason)
public void badMessage(BadMessageException failure)
{
fail(new BadMessageException(status, reason));
fail(failure);
}
protected void fail(Throwable failure)

View File

@ -25,7 +25,9 @@ import java.util.concurrent.ConcurrentMap;
import org.eclipse.jetty.fcgi.FCGI;
import org.eclipse.jetty.fcgi.generator.Flusher;
import org.eclipse.jetty.fcgi.parser.ServerParser;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.io.AbstractConnection;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.EndPoint;
@ -187,9 +189,7 @@ public class ServerFCGIConnection extends AbstractConnection
if (LOG.isDebugEnabled())
LOG.debug("Request {} failure on {}: {}", request, channel, failure);
if (channel != null)
{
channel.onBadMessage(400, failure.toString());
}
channel.onBadMessage(new BadMessageException(HttpStatus.BAD_REQUEST_400, null, failure));
}
}
}

View File

@ -1506,7 +1506,7 @@ public class HttpParser
if (DEBUG)
LOG.debug("{} EOF in {}",this,_state);
setState(State.CLOSED);
_handler.badMessage(HttpStatus.BAD_REQUEST_400,null);
_handler.badMessage(new BadMessageException(HttpStatus.BAD_REQUEST_400));
break;
}
}
@ -1532,7 +1532,7 @@ public class HttpParser
if (_headerComplete)
_handler.earlyEOF();
else
_handler.badMessage(x._code, x._reason);
_handler.badMessage(x);
}
protected boolean parseContent(ByteBuffer buffer)
@ -1811,10 +1811,20 @@ public class HttpParser
/* ------------------------------------------------------------ */
/** Called to signal that a bad HTTP message has been received.
* @param status The bad status to send
* @param reason The textual reason for badness
* @param failure the failure with the bad message information
*/
public void badMessage(int status, String reason);
public default void badMessage(BadMessageException failure)
{
badMessage(failure.getCode(), failure.getReason());
}
/**
* @deprecated use {@link #badMessage(BadMessageException)} instead
*/
@Deprecated
public default void badMessage(int status, String reason)
{
}
/* ------------------------------------------------------------ */
/** @return the size in bytes of the per parser header cache

View File

@ -1,11 +1,12 @@
# Mapping of mime type to inferred or assumed charset
# inferred charsets are used for encoding/decoding and explicitly set in associated metadata
# assumed charsets are used for encoding/decoding, but are not set in associated metadata
# In this file, assumed charsets are indicatd with a leading '-'
# In this file, assumed charsets are indicated with a leading '-'
text/html=utf-8
text/plain=iso-8859-1
text/xml=utf-8
application/xhtml+xml=utf-8
text/json=-utf-8
application/json=-utf-8
application/vnd.api+json=-utf-8

View File

@ -18,13 +18,6 @@
package org.eclipse.jetty.http;
import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collection;
@ -38,6 +31,12 @@ import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;
import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@RunWith(Parameterized.class)
public class HttpGeneratorServerHTTPTest
{
@ -261,9 +260,9 @@ public class HttpGeneratorServerHTTPTest
}
@Override
public void badMessage(int status, String reason)
public void badMessage(BadMessageException failure)
{
throw new IllegalStateException(reason);
throw failure;
}
@Override

View File

@ -19,8 +19,6 @@
package org.eclipse.jetty.http;
import static org.eclipse.jetty.http.HttpComplianceSection.NO_FIELD_FOLDING;
import static org.hamcrest.Matchers.contains;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
@ -35,6 +33,8 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import static org.hamcrest.Matchers.contains;
public class HttpParserTest
{
static
@ -2208,9 +2208,10 @@ public class HttpParserTest
}
@Override
public void badMessage(int status, String reason)
public void badMessage(BadMessageException failure)
{
_bad = reason == null ? ("" + status) : reason;
String reason = failure.getReason();
_bad = reason == null ? String.valueOf(failure.getCode()) : reason;
}
@Override

View File

@ -408,9 +408,9 @@ public class HttpTester
}
@Override
public void badMessage(int status, String reason)
public void badMessage(BadMessageException failure)
{
throw new RuntimeException(reason);
throw failure;
}
public ByteBuffer generate()

View File

@ -145,12 +145,12 @@ public class HttpChannelOverHTTP2 extends HttpChannel implements Closeable, Writ
}
catch (BadMessageException x)
{
onBadMessage(x.getCode(), x.getReason());
onBadMessage(x);
return null;
}
catch (Throwable x)
{
onBadMessage(HttpStatus.INTERNAL_SERVER_ERROR_500, null);
onBadMessage(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, null, x));
return null;
}
}
@ -177,12 +177,12 @@ public class HttpChannelOverHTTP2 extends HttpChannel implements Closeable, Writ
}
catch (BadMessageException x)
{
onBadMessage(x.getCode(), x.getReason());
onBadMessage(x);
return null;
}
catch (Throwable x)
{
onBadMessage(HttpStatus.INTERNAL_SERVER_ERROR_500, null);
onBadMessage(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, null, x));
return null;
}
}

View File

@ -527,12 +527,10 @@ public class SslConnection extends AbstractConnection
// don't bother writing, just notify of close
getWriteFlusher().onClose();
}
// Else,
else
{
// try to flush what is pending
// execute to avoid recursion
getExecutor().execute(_runCompleteWrite);
// Try again
_runCompleteWrite.run();
}
}
}

View File

@ -514,40 +514,28 @@ public class IOTest
assertThat(key,notNullValue());
assertThat(selector.selectNow(), is(0));
client.write(BufferUtil.toBuffer("X"));
assertThat(selector.select(), is(1));
assertThat(key.readyOps(), is(SelectionKey.OP_READ));
assertThat(selector.selectedKeys(), Matchers.contains(key));
assertThat(selector.select(), is(0));
assertThat(key.readyOps(), is(SelectionKey.OP_READ));
assertThat(selector.selectedKeys(), Matchers.contains(key));
client.write(BufferUtil.toBuffer("X"));
selector.selectedKeys().clear();
assertThat(selector.select(), is(1));
assertThat(key.readyOps(), is(SelectionKey.OP_READ));
assertThat(selector.selectedKeys(), Matchers.contains(key));
ByteBuffer buf = BufferUtil.allocate(1024);
int p = BufferUtil.flipToFill(buf);
assertThat(server.read(buf),is(2));
BufferUtil.flipToFlush(buf,p);
// Test wakeup before select
selector.wakeup();
selector.selectedKeys().clear();
assertThat(selector.select(), is(0));
assertThat(selector.selectedKeys().size(),is(0));
client.write(BufferUtil.toBuffer("X"));
// Test wakeup after select
new Thread()
{
@Override
public void run()
{
try
{
Thread.sleep(100);
selector.wakeup();
selector.selectedKeys().clear();
assertThat(selector.select(), is(1));
assertThat(selector.selectedKeys().size(),is(1));
p = BufferUtil.flipToFill(buf);
assertThat(server.read(buf),is(1));
BufferUtil.flipToFlush(buf,p);
}
catch(Exception e)
{
e.printStackTrace();
}
}
}.start();
assertThat(selector.select(), is(0));
}
}

View File

@ -121,16 +121,18 @@ public class IdleTimeoutTest
@Test
public void testShorten() throws Exception
{
for (int i=0;i<20;i++)
_timeout.setIdleTimeout(2000);
for (int i=0;i<30;i++)
{
Thread.sleep(200);
Thread.sleep(100);
_timeout.notIdle();
}
Assert.assertNull(_expired);
_timeout.setIdleTimeout(100);
long start = System.nanoTime();
while (_expired==null && TimeUnit.NANOSECONDS.toSeconds(System.nanoTime()-start)<4)
while (_expired==null && TimeUnit.NANOSECONDS.toSeconds(System.nanoTime()-start)<5)
Thread.sleep(200);
Assert.assertNotNull(_expired);

View File

@ -707,12 +707,14 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
return _request.getHttpInput().earlyEOF();
}
public void onBadMessage(int status, String reason)
public void onBadMessage(BadMessageException failure)
{
int status = failure.getCode();
String reason = failure.getReason();
if (status < 400 || status > 599)
status = HttpStatus.BAD_REQUEST_400;
failure = new BadMessageException(HttpStatus.BAD_REQUEST_400, reason, failure);
notifyRequestFailure(_request, new BadMessageException(status, reason));
notifyRequestFailure(_request, failure);
Action action;
try
@ -721,10 +723,10 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
}
catch(IllegalStateException e)
{
// The bad message cannot be handled in the current state, so throw
// to hopefull somebody that can handle
// The bad message cannot be handled in the current state,
// so rethrow, hopefully somebody will be able to handle.
abort(e);
throw new BadMessageException(status,reason);
throw failure;
}
try

View File

@ -269,7 +269,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
}
@Override
public void badMessage(int status, String reason)
public void badMessage(BadMessageException failure)
{
_httpConnection.getGenerator().setPersistent(false);
try
@ -283,7 +283,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
LOG.ignore(e);
}
onBadMessage(status, reason);
onBadMessage(failure);
}
@Override
@ -333,7 +333,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
{
if (_unknownExpectation)
{
badMessage(HttpStatus.EXPECTATION_FAILED_417, null);
badMessage(new BadMessageException(HttpStatus.EXPECTATION_FAILED_417));
return false;
}
@ -374,7 +374,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
upgrade())
return true;
badMessage(HttpStatus.UPGRADE_REQUIRED_426, null);
badMessage(new BadMessageException(HttpStatus.UPGRADE_REQUIRED_426));
_httpConnection.getParser().close();
return false;
}

View File

@ -471,11 +471,6 @@ public class LocalConnector extends AbstractConnector
return false;
}
@Override
public void badMessage(int status, String reason)
{
}
@Override
public boolean startResponse(HttpVersion version, int status, String reason)
{

View File

@ -225,7 +225,7 @@ public class ResourceService
String pathInContext=URIUtil.addPaths(servletPath,pathInfo);
boolean endsWithSlash=(pathInfo==null?request.getServletPath():pathInfo).endsWith(URIUtil.SLASH);
boolean endsWithSlash=(pathInfo==null?servletPath:pathInfo).endsWith(URIUtil.SLASH);
boolean checkPrecompressedVariants=_precompressedFormats.length > 0 && !endsWithSlash && !included && reqRanges==null;
HttpContent content=null;
@ -254,7 +254,7 @@ public class ResourceService
}
// Strip slash?
if (endsWithSlash && pathInContext.length()>1)
if (!included && endsWithSlash && pathInContext.length()>1)
{
String q=request.getQueryString();
pathInContext=pathInContext.substring(0,pathInContext.length()-1);

View File

@ -448,6 +448,28 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
interceptor=interceptor.getNextInterceptor();
}
// Special handling for etags
for (ListIterator<HttpField> fields = baseRequest.getHttpFields().listIterator(); fields.hasNext();)
{
HttpField field = fields.next();
if (field.getHeader()==HttpHeader.IF_NONE_MATCH || field.getHeader()==HttpHeader.IF_MATCH)
{
String etag = field.getValue();
int i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote);
if (i>0)
{
baseRequest.setAttribute("o.e.j.s.h.gzip.GzipHandler.etag",etag);
while (i>=0)
{
etag=etag.substring(0,i)+etag.substring(i+CompressedContentFormat.GZIP._etag.length());
i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote,i);
}
fields.set(new HttpField(field.getHeader(),etag));
}
}
}
// If not a supported method - no Vary because no matter what client, this URI is always excluded
if (!_methods.test(baseRequest.getMethod()))
{
@ -495,28 +517,6 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
}
}
// Special handling for etags
for (ListIterator<HttpField> fields = baseRequest.getHttpFields().listIterator(); fields.hasNext();)
{
HttpField field = fields.next();
if (field.getHeader()==HttpHeader.IF_NONE_MATCH || field.getHeader()==HttpHeader.IF_MATCH)
{
String etag = field.getValue();
int i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote);
if (i>0)
{
baseRequest.setAttribute("o.e.j.s.h.gzip.GzipHandler.etag",etag);
while (i>=0)
{
etag=etag.substring(0,i)+etag.substring(i+CompressedContentFormat.GZIP._etag.length());
i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote,i);
}
fields.set(new HttpField(field.getHeader(),etag));
}
}
}
HttpOutput.Interceptor orig_interceptor = out.getInterceptor();
try
{

View File

@ -1,42 +0,0 @@
//
// ========================================================================
// Copyright (c) 1995-2018 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.server.handler.gzip;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;
import java.util.Arrays;
import org.junit.Test;
public class GzipHandlerTest
{
@Test
public void testAddGetPaths()
{
GzipHandler gzip = new GzipHandler();
gzip.addIncludedPaths("/foo");
gzip.addIncludedPaths("^/bar.*$");
String[] includedPaths = gzip.getIncludedPaths();
assertThat("Included Paths.size", includedPaths.length, is(2));
assertThat("Included Paths", Arrays.asList(includedPaths), contains("/foo","^/bar.*$"));
}
}

View File

@ -47,13 +47,13 @@ import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@SuppressWarnings("serial")
public class GzipHandlerTest
{
private static final String __content =
@ -151,6 +151,17 @@ public class GzipHandlerTest
writer.write(__content);
}
}
@Override
protected void doDelete(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
{
String ifm = req.getHeader("If-Match");
if (ifm!=null && ifm.equals(__contentETag))
response.sendError(HttpServletResponse.SC_NO_CONTENT);
else
response.sendError(HttpServletResponse.SC_NOT_MODIFIED);
}
}
public static class EchoServlet extends HttpServlet
@ -347,6 +358,43 @@ public class GzipHandlerTest
assertThat(response.get("ETag"),is(__contentETagGzip));
}
@Test
public void testDeleteETagGzipHandler() throws Exception
{
HttpTester.Request request = HttpTester.newRequest();
HttpTester.Response response;
request.setMethod("DELETE");
request.setURI("/ctx/content");
request.setVersion("HTTP/1.0");
request.setHeader("Host","tester");
request.setHeader("If-Match","WrongEtag--gzip");
request.setHeader("accept-encoding","gzip");
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
assertThat(response.getStatus(),is(HttpServletResponse.SC_NOT_MODIFIED));
assertThat(response.get("Content-Encoding"),not(Matchers.equalToIgnoringCase("gzip")));
request = HttpTester.newRequest();
request.setMethod("DELETE");
request.setURI("/ctx/content");
request.setVersion("HTTP/1.0");
request.setHeader("Host","tester");
request.setHeader("If-Match",__contentETagGzip);
request.setHeader("accept-encoding","gzip");
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
assertThat(response.getStatus(),is(HttpServletResponse.SC_NO_CONTENT));
assertThat(response.get("Content-Encoding"),not(Matchers.equalToIgnoringCase("gzip")));
}
@Test
public void testForwardGzipHandler() throws Exception
{