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 #2321 from eclipse/jetty-9.4.x-2318-bad_message_handling 

Fixes #2318 - HttpParser.Listener.onBadMessage() should take BadMessageException
This commit is contained in:
Simone Bordet 2018-03-13 09:36:36 +01:00 committed by GitHub
parent f35bd03810 8b391a88dd
commit 9338aa171d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 58 additions and 53 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/http/HttpReceiverOverHTTP.java
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));
}
}

10
jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java
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"));
}
}

4
jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ResponseContentParser.java
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)

6
jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java
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));
}
}
}

20
jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View File

@ -1499,7 +1499,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;
}
}
@ -1525,7 +1525,7 @@ public class HttpParser
if (_headerComplete)
_handler.earlyEOF();
else
_handler.badMessage(x._code, x._reason);
_handler.badMessage(x);
}
protected boolean parseContent(ByteBuffer buffer)
@ -1804,10 +1804,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

17
jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java
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

9
jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View File

@ -18,8 +18,6 @@
package org.eclipse.jetty.http;
import static org.hamcrest.Matchers.contains;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
@ -34,6 +32,8 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import static org.hamcrest.Matchers.contains;
public class HttpParserTest
{
static
@ -2201,9 +2201,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

4
jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java
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()

8
jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpChannelOverHTTP2.java
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;
}
}

14
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
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

8
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
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;
}

5
jetty-server/src/main/java/org/eclipse/jetty/server/LocalConnector.java
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)
{