Fixes #2318 - HttpParser.Listener.onBadMessage() should take BadMessageException.
Changed the signature of HttpParser.Listener.onBadMessage() to take a BadMessageException and updated dependent code. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
parent
ff341fb420
commit
8b391a88dd
|
@ -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));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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"));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
{
|
||||
|
|
Loading…
Reference in New Issue