Issue #1234 onBadMessage
Added a boolean to determine if headerComplete has been called, and if so then earlyEOF is called
This commit is contained in:
parent
d2b8980243
commit
36dcf47f18
|
@ -156,6 +156,7 @@ public class HttpParser
|
|||
private int _responseStatus;
|
||||
private int _headerBytes;
|
||||
private boolean _host;
|
||||
private boolean _headerComplete;
|
||||
|
||||
/* ------------------------------------------------------------------------------- */
|
||||
private volatile State _state=State.START;
|
||||
|
@ -730,6 +731,7 @@ public class HttpParser
|
|||
setState(State.END);
|
||||
BufferUtil.clear(buffer);
|
||||
handle=_handler.headerComplete()||handle;
|
||||
_headerComplete=true;
|
||||
handle=_handler.messageComplete()||handle;
|
||||
return handle;
|
||||
}
|
||||
|
@ -800,6 +802,7 @@ public class HttpParser
|
|||
setState(State.END);
|
||||
BufferUtil.clear(buffer);
|
||||
handle=_handler.headerComplete()||handle;
|
||||
_headerComplete=true;
|
||||
handle=_handler.messageComplete()||handle;
|
||||
return handle;
|
||||
}
|
||||
|
@ -1057,22 +1060,26 @@ public class HttpParser
|
|||
case EOF_CONTENT:
|
||||
setState(State.EOF_CONTENT);
|
||||
handle=_handler.headerComplete()||handle;
|
||||
_headerComplete=true;
|
||||
return handle;
|
||||
|
||||
case CHUNKED_CONTENT:
|
||||
setState(State.CHUNKED_CONTENT);
|
||||
handle=_handler.headerComplete()||handle;
|
||||
_headerComplete=true;
|
||||
return handle;
|
||||
|
||||
case NO_CONTENT:
|
||||
setState(State.END);
|
||||
handle=_handler.headerComplete()||handle;
|
||||
_headerComplete=true;
|
||||
handle=_handler.messageComplete()||handle;
|
||||
return handle;
|
||||
|
||||
default:
|
||||
setState(State.CONTENT);
|
||||
handle=_handler.headerComplete()||handle;
|
||||
_headerComplete=true;
|
||||
return handle;
|
||||
}
|
||||
}
|
||||
|
@ -1426,39 +1433,30 @@ public class HttpParser
|
|||
LOG.warn("parse exception: {} in {} for {}",e.toString(),_state,_handler);
|
||||
if (DEBUG)
|
||||
LOG.debug(e);
|
||||
badMessage();
|
||||
|
||||
switch(_state)
|
||||
{
|
||||
case CLOSED:
|
||||
break;
|
||||
case CLOSE:
|
||||
_handler.earlyEOF();
|
||||
break;
|
||||
default:
|
||||
setState(State.CLOSE);
|
||||
_handler.badMessage(400,"Bad Message "+e.toString());
|
||||
}
|
||||
}
|
||||
catch(Exception|Error e)
|
||||
{
|
||||
BufferUtil.clear(buffer);
|
||||
|
||||
LOG.warn("parse exception: "+e.toString()+" for "+_handler,e);
|
||||
|
||||
switch(_state)
|
||||
{
|
||||
case CLOSED:
|
||||
break;
|
||||
case CLOSE:
|
||||
_handler.earlyEOF();
|
||||
break;
|
||||
default:
|
||||
setState(State.CLOSE);
|
||||
_handler.badMessage(400,null);
|
||||
}
|
||||
badMessage();
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
protected void badMessage()
|
||||
{
|
||||
if (_headerComplete)
|
||||
{
|
||||
_handler.earlyEOF();
|
||||
}
|
||||
else if (_state!=State.CLOSED)
|
||||
{
|
||||
setState(State.CLOSE);
|
||||
_handler.badMessage(400,_requestHandler!=null?"Bad Request":"Bad Response");
|
||||
}
|
||||
}
|
||||
|
||||
protected boolean parseContent(ByteBuffer buffer)
|
||||
{
|
||||
|
@ -1677,6 +1675,7 @@ public class HttpParser
|
|||
_contentChunk=null;
|
||||
_headerBytes=0;
|
||||
_host=false;
|
||||
_headerComplete=false;
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------------------- */
|
||||
|
|
|
@ -219,11 +219,15 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
|
|||
@Override
|
||||
public void earlyEOF()
|
||||
{
|
||||
_httpConnection.getGenerator().setPersistent(false);
|
||||
// If we have no request yet, just close
|
||||
if (_metadata.getMethod() == null)
|
||||
_httpConnection.close();
|
||||
else if (onEarlyEOF())
|
||||
else if (onEarlyEOF() || _delayedForContent)
|
||||
{
|
||||
_delayedForContent = false;
|
||||
handle();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -297,6 +297,7 @@ public class AsyncRequestReadTest
|
|||
BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
|
||||
assertThat(in.readLine(),containsString("HTTP/1.1 200 OK"));
|
||||
assertThat(in.readLine(),containsString("Content-Length:"));
|
||||
assertThat(in.readLine(),containsString("Connection: close"));
|
||||
assertThat(in.readLine(),containsString("Server:"));
|
||||
in.readLine();
|
||||
assertThat(in.readLine(),containsString("XXXXXXX"));
|
||||
|
|
|
@ -421,7 +421,7 @@ public class HttpConnectionTest
|
|||
|
||||
try(StacklessLogging stackless = new StacklessLogging(HttpParser.class))
|
||||
{
|
||||
response=connector.getResponses("GET /bad/encoding%1 HTTP/1.1\r\n"+
|
||||
response=connector.getResponse("GET /bad/encoding%1 HTTP/1.1\r\n"+
|
||||
"Host: localhost\r\n"+
|
||||
"Connection: close\r\n"+
|
||||
"\r\n");
|
||||
|
|
|
@ -49,6 +49,7 @@ import javax.servlet.ServletOutputStream;
|
|||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
import org.eclipse.jetty.http.HttpTester;
|
||||
import org.eclipse.jetty.io.EndPoint;
|
||||
import org.eclipse.jetty.io.EofException;
|
||||
import org.eclipse.jetty.server.handler.AbstractHandler;
|
||||
|
@ -666,6 +667,54 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testBlockingReadBadChunk() throws Exception
|
||||
{
|
||||
configureServer(new ReadHandler());
|
||||
|
||||
try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort()))
|
||||
{
|
||||
client.setSoTimeout(600000);
|
||||
OutputStream os = client.getOutputStream();
|
||||
InputStream is = client.getInputStream();
|
||||
|
||||
os.write((
|
||||
"GET /data HTTP/1.1\r\n" +
|
||||
"host: " + _serverURI.getHost() + ":" + _serverURI.getPort() + "\r\n" +
|
||||
"content-type: unknown\r\n" +
|
||||
"transfer-encoding: chunked\r\n" +
|
||||
"\r\n"
|
||||
).getBytes());
|
||||
os.flush();
|
||||
Thread.sleep(50);
|
||||
os.write((
|
||||
"a\r\n" +
|
||||
"123456890\r\n"
|
||||
).getBytes());
|
||||
os.flush();
|
||||
|
||||
Thread.sleep(50);
|
||||
os.write((
|
||||
"4\r\n" +
|
||||
"abcd\r\n"
|
||||
).getBytes());
|
||||
os.flush();
|
||||
|
||||
Thread.sleep(50);
|
||||
os.write((
|
||||
"X\r\n" +
|
||||
"abcd\r\n"
|
||||
).getBytes());
|
||||
os.flush();
|
||||
|
||||
HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(is));
|
||||
|
||||
assertThat(response.getStatus(),is(200));
|
||||
assertThat(response.getContent(),containsString("EofException"));
|
||||
assertThat(response.getContent(),containsString("Early EOF"));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testBlockingWhileWritingResponseContent() throws Exception
|
||||
{
|
||||
|
|
|
@ -186,7 +186,28 @@ public class HttpServerTestFixture
|
|||
response.getOutputStream().print("Hello world\r\n");
|
||||
}
|
||||
}
|
||||
|
||||
protected static class ReadHandler extends AbstractHandler
|
||||
{
|
||||
@Override
|
||||
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
|
||||
{
|
||||
baseRequest.setHandled(true);
|
||||
response.setStatus(200);
|
||||
|
||||
try
|
||||
{
|
||||
InputStream in = request.getInputStream();
|
||||
String input= IO.toString(in);
|
||||
response.getWriter().printf("read %d%n",input.length());
|
||||
}
|
||||
catch(Exception e)
|
||||
{
|
||||
response.getWriter().printf("caught %s%n",e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
protected static class DataHandler extends AbstractHandler
|
||||
{
|
||||
@Override
|
||||
|
|
|
@ -159,11 +159,8 @@ public class PostServletTest
|
|||
req.append("\r\n");
|
||||
req.append("\r\n");
|
||||
|
||||
try (StacklessLogging scope = new StacklessLogging(ServletHandler.class))
|
||||
{
|
||||
String resp = connector.getResponses(req.toString());
|
||||
assertThat(resp,is("")); // Aborted before response committed
|
||||
}
|
||||
String resp = connector.getResponse(req.toString());
|
||||
assertThat(resp,startsWith("HTTP/1.1 200 OK")); // exception eaten by handler
|
||||
assertTrue(complete.await(5,TimeUnit.SECONDS));
|
||||
assertThat(ex0.get(),not(nullValue()));
|
||||
assertThat(ex1.get(),not(nullValue()));
|
||||
|
@ -178,31 +175,26 @@ public class PostServletTest
|
|||
req.append("Transfer-Encoding: chunked\r\n");
|
||||
req.append("\r\n");
|
||||
|
||||
try (StacklessLogging scope = new StacklessLogging(ServletHandler.class))
|
||||
{
|
||||
LocalConnector.LocalEndPoint endp=connector.executeRequest(req.toString());
|
||||
Thread.sleep(1000);
|
||||
assertFalse(posted.get());
|
||||
LocalConnector.LocalEndPoint endp=connector.executeRequest(req.toString());
|
||||
Thread.sleep(1000);
|
||||
assertFalse(posted.get());
|
||||
|
||||
req.setLength(0);
|
||||
// intentionally bad (not a valid chunked char here)
|
||||
for (int i=1024;i-->0;)
|
||||
req.append("xxxxxxxxxxxx");
|
||||
req.append("\r\n");
|
||||
req.append("\r\n");
|
||||
req.setLength(0);
|
||||
// intentionally bad (not a valid chunked char here)
|
||||
for (int i=1024;i-->0;)
|
||||
req.append("xxxxxxxxxxxx");
|
||||
req.append("\r\n");
|
||||
req.append("\r\n");
|
||||
|
||||
endp.addInput(req.toString());
|
||||
endp.addInput(req.toString());
|
||||
|
||||
endp.waitUntilClosedOrIdleFor(1,TimeUnit.SECONDS);
|
||||
String resp = endp.takeOutputString();
|
||||
endp.waitUntilClosedOrIdleFor(1,TimeUnit.SECONDS);
|
||||
String resp = endp.takeOutputString();
|
||||
|
||||
assertThat("resp", resp, containsString("HTTP/1.1 400 "));
|
||||
|
||||
}
|
||||
|
||||
// null because it was never dispatched!
|
||||
assertThat(ex0.get(),nullValue());
|
||||
assertThat(ex1.get(),nullValue());
|
||||
assertThat(resp,startsWith("HTTP/1.1 200 OK")); // exception eaten by handler
|
||||
assertTrue(complete.await(5,TimeUnit.SECONDS));
|
||||
assertThat(ex0.get(),not(nullValue()));
|
||||
assertThat(ex1.get(),not(nullValue()));
|
||||
}
|
||||
|
||||
|
||||
|
|
Loading…
Reference in New Issue