Revert "434810 better handling of bad messages"
This reverts commit 1e524f378e
.
This commit is contained in:
parent
aa3c9322a6
commit
5e3e0930e0
|
@ -443,13 +443,13 @@ public class HttpParser
|
||||||
else if (ch==0)
|
else if (ch==0)
|
||||||
break;
|
break;
|
||||||
else if (ch<0)
|
else if (ch<0)
|
||||||
throw new BadMessage(-1);
|
throw new BadMessage();
|
||||||
|
|
||||||
// count this white space as a header byte to avoid DOS
|
// count this white space as a header byte to avoid DOS
|
||||||
if (_maxHeaderBytes>0 && ++_headerBytes>_maxHeaderBytes)
|
if (_maxHeaderBytes>0 && ++_headerBytes>_maxHeaderBytes)
|
||||||
{
|
{
|
||||||
LOG.warn("padding is too large >"+_maxHeaderBytes);
|
LOG.warn("padding is too large >"+_maxHeaderBytes);
|
||||||
throw new BadMessage(-1);
|
throw new BadMessage(HttpStatus.BAD_REQUEST_400);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return false;
|
return false;
|
||||||
|
@ -1283,7 +1283,7 @@ public class HttpParser
|
||||||
if (_headerBytes>_maxHeaderBytes)
|
if (_headerBytes>_maxHeaderBytes)
|
||||||
{
|
{
|
||||||
// Don't want to waste time reading data of a closed request
|
// Don't want to waste time reading data of a closed request
|
||||||
throw new BadMessage(-1,"too much data after closed");
|
throw new IllegalStateException("too much data after closed");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1333,16 +1333,12 @@ public class HttpParser
|
||||||
{
|
{
|
||||||
BufferUtil.clear(buffer);
|
BufferUtil.clear(buffer);
|
||||||
|
|
||||||
if (e._code>0)
|
LOG.warn("badMessage: "+e._code+(e._message!=null?" "+e._message:"")+" for "+_handler);
|
||||||
LOG.warn("badMessage: "+e._code+(e._message!=null?" "+e._message:"")+" for "+_handler);
|
|
||||||
else
|
|
||||||
LOG.warn("badMessage: "+(e._message!=null?e._message+" ":"")+"for "+_handler);
|
|
||||||
|
|
||||||
if (DEBUG)
|
if (DEBUG)
|
||||||
LOG.debug(e);
|
LOG.debug(e);
|
||||||
setState(State.CLOSED);
|
setState(State.CLOSED);
|
||||||
_handler.badMessage(e._code, e._message);
|
_handler.badMessage(e._code, e._message);
|
||||||
return true;
|
return false;
|
||||||
}
|
}
|
||||||
catch(Exception e)
|
catch(Exception e)
|
||||||
{
|
{
|
||||||
|
@ -1363,7 +1359,7 @@ public class HttpParser
|
||||||
setState(State.CLOSED);
|
setState(State.CLOSED);
|
||||||
}
|
}
|
||||||
|
|
||||||
return true;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1625,9 +1621,7 @@ public class HttpParser
|
||||||
|
|
||||||
/* ------------------------------------------------------------ */
|
/* ------------------------------------------------------------ */
|
||||||
/** Called to signal that a bad HTTP message has been received.
|
/** Called to signal that a bad HTTP message has been received.
|
||||||
* @param status The bad status to send. If the status is <0, this indicates
|
* @param status The bad status to send
|
||||||
* that the message was so bad that a response should not be sent and the
|
|
||||||
* connection should be immediately closed.
|
|
||||||
* @param reason The textual reason for badness
|
* @param reason The textual reason for badness
|
||||||
*/
|
*/
|
||||||
public void badMessage(int status, String reason);
|
public void badMessage(int status, String reason);
|
||||||
|
|
|
@ -66,7 +66,6 @@ public class HttpChannelState
|
||||||
WRITE_CALLBACK, // handle an IO write callback
|
WRITE_CALLBACK, // handle an IO write callback
|
||||||
READ_CALLBACK, // handle an IO read callback
|
READ_CALLBACK, // handle an IO read callback
|
||||||
WAIT, // Wait for further events
|
WAIT, // Wait for further events
|
||||||
IO_WAIT, // Wait for further IO
|
|
||||||
COMPLETE // Complete the channel
|
COMPLETE // Complete the channel
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -183,7 +182,7 @@ public class HttpChannelState
|
||||||
return Action.COMPLETE;
|
return Action.COMPLETE;
|
||||||
|
|
||||||
case COMPLETED:
|
case COMPLETED:
|
||||||
return Action.IO_WAIT;
|
return Action.WAIT;
|
||||||
|
|
||||||
case ASYNC_WOKEN:
|
case ASYNC_WOKEN:
|
||||||
if (_asyncRead)
|
if (_asyncRead)
|
||||||
|
|
|
@ -474,18 +474,8 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
|
||||||
@Override
|
@Override
|
||||||
public void badMessage(int status, String reason)
|
public void badMessage(int status, String reason)
|
||||||
{
|
{
|
||||||
if (status<0)
|
_generator.setPersistent(false);
|
||||||
{
|
super.badMessage(status,reason);
|
||||||
getEndPoint().close();
|
|
||||||
_parser.atEOF();
|
|
||||||
LOG.info("Bad message close of "+getEndPoint());
|
|
||||||
completed();
|
|
||||||
}
|
|
||||||
else
|
|
||||||
{
|
|
||||||
_generator.setPersistent(false);
|
|
||||||
super.badMessage(status,reason);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|
|
@ -40,7 +40,6 @@ import java.net.URL;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.concurrent.Exchanger;
|
import java.util.concurrent.Exchanger;
|
||||||
import java.util.concurrent.TimeUnit;
|
|
||||||
import java.util.concurrent.atomic.AtomicBoolean;
|
import java.util.concurrent.atomic.AtomicBoolean;
|
||||||
|
|
||||||
import javax.servlet.ServletException;
|
import javax.servlet.ServletException;
|
||||||
|
@ -134,90 +133,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
/*
|
|
||||||
* Feed a full header method
|
|
||||||
*/
|
|
||||||
@Test
|
|
||||||
public void testFullWhite() throws Exception
|
|
||||||
{
|
|
||||||
configureServer(new HelloWorldHandler());
|
|
||||||
|
|
||||||
try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort()))
|
|
||||||
{
|
|
||||||
client.setSoTimeout(10000);
|
|
||||||
((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
|
|
||||||
((StdErrLog) Log.getLogger(HttpConnection.class)).info("expect request is too large...");
|
|
||||||
OutputStream os = client.getOutputStream();
|
|
||||||
|
|
||||||
byte[] buffer = new byte[64 * 1024];
|
|
||||||
Arrays.fill(buffer, (byte)' ');
|
|
||||||
|
|
||||||
os.write(buffer);
|
|
||||||
os.flush();
|
|
||||||
|
|
||||||
// Read the close.
|
|
||||||
readClose(client);
|
|
||||||
}
|
|
||||||
finally
|
|
||||||
{
|
|
||||||
((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
/*
|
|
||||||
* Feed a full header method
|
|
||||||
*/
|
|
||||||
@Test
|
|
||||||
public void testFullWhiteAfter() throws Exception
|
|
||||||
{
|
|
||||||
|
|
||||||
configureServer(new HelloWorldHandler());
|
|
||||||
|
|
||||||
try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort()))
|
|
||||||
{
|
|
||||||
((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
|
|
||||||
((StdErrLog)Log.getLogger(HttpConnection.class)).info("expect Bad Message close ...");
|
|
||||||
OutputStream os = client.getOutputStream();
|
|
||||||
|
|
||||||
byte[] buffer = new byte[64 * 1024];
|
|
||||||
buffer[0]='G';
|
|
||||||
buffer[1]='E';
|
|
||||||
buffer[2]='T';
|
|
||||||
buffer[3]=' ';
|
|
||||||
buffer[4]='/';
|
|
||||||
buffer[5]=' ';
|
|
||||||
buffer[6]='H';
|
|
||||||
buffer[7]='T';
|
|
||||||
buffer[8]='T';
|
|
||||||
buffer[9]='P';
|
|
||||||
buffer[10]='/';
|
|
||||||
buffer[11]='1';
|
|
||||||
buffer[12]='.';
|
|
||||||
buffer[13]='0';
|
|
||||||
buffer[14]='\n';
|
|
||||||
buffer[15]='\n';
|
|
||||||
Arrays.fill(buffer,16,buffer.length-1,(byte)' ');
|
|
||||||
|
|
||||||
os.write(buffer);
|
|
||||||
os.flush();
|
|
||||||
|
|
||||||
// Read the response.
|
|
||||||
long start = System.nanoTime();
|
|
||||||
String response = readResponse(client);
|
|
||||||
long end = System.nanoTime();
|
|
||||||
|
|
||||||
Assert.assertThat(response, Matchers.containsString("HTTP/1.1 200 OK"));
|
|
||||||
|
|
||||||
Assert.assertThat(TimeUnit.NANOSECONDS.toSeconds(end-start),Matchers.lessThan(1L));
|
|
||||||
|
|
||||||
}
|
|
||||||
finally
|
|
||||||
{
|
|
||||||
((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Feed a full header method
|
* Feed a full header method
|
||||||
*/
|
*/
|
||||||
|
@ -1459,29 +1374,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
|
||||||
throw e;
|
throw e;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Read Close.
|
|
||||||
*
|
|
||||||
* @param client Open client socket.
|
|
||||||
* @throws IOException in case of I/O problems
|
|
||||||
*/
|
|
||||||
protected static void readClose(Socket client) throws IOException
|
|
||||||
{
|
|
||||||
try (BufferedReader br = new BufferedReader(new InputStreamReader(client.getInputStream())))
|
|
||||||
{
|
|
||||||
String line;
|
|
||||||
|
|
||||||
if ((line = br.readLine()) != null)
|
|
||||||
throw new IllegalStateException("unexpected data: "+line);
|
|
||||||
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
catch (IOException e)
|
|
||||||
{
|
|
||||||
// expected
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
protected void writeFragments(byte[] bytes, int[] points, StringBuilder message, OutputStream os) throws IOException, InterruptedException
|
protected void writeFragments(byte[] bytes, int[] points, StringBuilder message, OutputStream os) throws IOException, InterruptedException
|
||||||
{
|
{
|
||||||
|
|
|
@ -64,7 +64,7 @@ public class SelectChannelServerSslTest extends HttpServerTestBase
|
||||||
public void testFullMethod() throws Exception
|
public void testFullMethod() throws Exception
|
||||||
{
|
{
|
||||||
// Don't run on Windows (buggy JVM)
|
// Don't run on Windows (buggy JVM)
|
||||||
// Assume.assumeTrue(!OS.IS_WINDOWS);
|
Assume.assumeTrue(!OS.IS_WINDOWS);
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
|
@ -80,7 +80,7 @@ public class SelectChannelServerSslTest extends HttpServerTestBase
|
||||||
public void testFullURI() throws Exception
|
public void testFullURI() throws Exception
|
||||||
{
|
{
|
||||||
// Don't run on Windows (buggy JVM)
|
// Don't run on Windows (buggy JVM)
|
||||||
// Assume.assumeTrue(!OS.IS_WINDOWS);
|
Assume.assumeTrue(!OS.IS_WINDOWS);
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
super.testFullURI();
|
super.testFullURI();
|
||||||
|
|
Loading…
Reference in New Issue