Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

jetty-9 some progress on HTTP error handling but more work needed. See SelectChannelServerTest

This commit is contained in:
Greg Wilkins 2012-05-18 13:12:38 +02:00
parent b87e0f776b
commit 40ed3a013a
8 changed files with 162 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -869,9 +869,12 @@ public class HttpParser
{
// process end states
if (_state == State.END)
// TODO should we consume white space here?
return false;
if (_state == State.CONTENT && _contentPosition == _contentLength)
{
// TODO why is this not _state=_persistent?State.END:State.SEEKING_EOF;
_state=State.END;
if(_handler.messageComplete(_contentPosition))
return true;
@ -1068,7 +1071,7 @@ public class HttpParser
}
catch(Exception e)
{
LOG.debug(e);
LOG.warn(e);
_handler.badMessage(e.toString());
return true;
}
@ -1118,7 +1121,8 @@ public class HttpParser
public void reset()
{
// reset state
_state=_persistent?State.START:_state==State.END?State.END:State.SEEKING_EOF;
// TODO why is this not _state=_persistent?State.START:(_state=State.SEEKING_EOF);
_state=_persistent?State.START:(_state==State.END?State.END:State.SEEKING_EOF);
_endOfContent=EndOfContent.UNKNOWN_CONTENT;
_contentPosition=0;
_responseStatus=0;

11
jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java
View File

@ -57,17 +57,18 @@ abstract public class WriteFlusher
return;
}
}
if (!_writing.compareAndSet(true,false))
throw new ConcurrentModificationException();
callback.completed(context);
}
catch (IOException e)
{
if (!_writing.compareAndSet(true,false))
throw new ConcurrentModificationException();
throw new ConcurrentModificationException(e);
callback.failed(context,e);
return;
}
if (!_writing.compareAndSet(true,false))
throw new ConcurrentModificationException();
callback.completed(context);
}
/* ------------------------------------------------------------ */

24
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
View File

@ -342,16 +342,20 @@ public abstract class HttpChannel
{
LOG.ignore(e);
}
catch (ServletException e)
{
// TODO
}
catch (EofException e)
{
LOG.debug(e);
async_exception=e;
_request.setHandled(true);
}
catch (ServletException e)
{
LOG.warn(String.valueOf(_uri),e.toString());
LOG.debug(String.valueOf(_uri),e);
async_exception=e;
_request.setHandled(true);
commitError(500, null, e.toString());
}
catch (Throwable e)
{
LOG.warn(String.valueOf(_uri),e);
@ -364,14 +368,12 @@ public abstract class HttpChannel
handling = !_state.unhandle();
}
}
}
finally
{
__currentChannel.set(null);
if (threadName!=null)
Thread.currentThread().setName(threadName);
if (_state.isUncompleted())
{
@ -397,16 +399,14 @@ public abstract class HttpChannel
}
catch(IOException e)
{
LOG.debug(e);
LOG.warn(e);
}
_request.setHandled(true);
completed();
}
}
}
/* ------------------------------------------------------------ */
protected boolean commitError(final int status, final String reason, String content)
{
@ -583,7 +583,7 @@ public abstract class HttpChannel
case CONTENT_TYPE:
MimeTypes.Type mime=MimeTypes.CACHE.get(value);
String charset=(mime==null)?MimeTypes.getCharsetFromContentType(value):mime.getCharset().toString();
String charset=(mime==null||mime.getCharset()==null)?MimeTypes.getCharsetFromContentType(value):mime.getCharset().toString();
if (charset!=null)
_request.setCharacterEncodingUnchecked(charset);
break;
@ -643,6 +643,10 @@ public abstract class HttpChannel
@Override
public boolean content(ByteBuffer ref)
{
if (LOG.isDebugEnabled())
{
LOG.debug("{} content {}",this,BufferUtil.toDetailString(ref));
}
_in.content(ref);
return true;
}

113
jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
View File

@ -87,6 +87,7 @@ public class HttpConnection extends AbstractAsyncConnection
_parser = new HttpParser(_channel.getEventHandler());
_generator = new HttpGenerator();
_generator.setSendServerVersion(_server.getSendServerVersion());
LOG.debug("New HTTP Connection {}",this);
}
@ -132,15 +133,21 @@ public class HttpConnection extends AbstractAsyncConnection
_generator.reset();
_channel.reset();
_httpInput.recycle();
if (_requestBuffer!=null)
if (_requestBuffer!=null && !_requestBuffer.hasRemaining())
{
_bufferPool.release(_requestBuffer);
_requestBuffer=null;
if (_responseHeader!=null)
_requestBuffer=null;
}
if (_responseHeader!=null && !_responseHeader.hasRemaining())
{
_bufferPool.release(_responseHeader);
_responseHeader=null;
if (_responseBuffer!=null)
_responseHeader=null;
}
if (_responseBuffer!=null && !_responseBuffer.hasRemaining())
{
_bufferPool.release(_responseBuffer);
_responseBuffer=null;
_responseBuffer=null;
}
if (_chunk!=null)
_bufferPool.release(_chunk);
_chunk=null;
@ -252,11 +259,16 @@ public class HttpConnection extends AbstractAsyncConnection
}
// return if the connection has been changed
if (getEndPoint().getAsyncConnection()!=null)
if (getEndPoint().getAsyncConnection()!=this)
{
getEndPoint().getAsyncConnection().onOpen();
return;
}
}
else if (BufferUtil.hasContent(_requestBuffer))
{
LOG.warn("STATE MACHINE FAILURE??? {} {}",_parser,BufferUtil.toDetailString(_requestBuffer));
BufferUtil.clear(_requestBuffer);
}
}
}
@ -347,27 +359,53 @@ public class HttpConnection extends AbstractAsyncConnection
generate(null,Action.COMPLETE);
}
@Override
protected boolean commitError(int status, String reason, String content)
{
if (!super.commitError(status,reason,content))
{
// We could not send the error, so a sudden close of the connection will at least tell
// the client something is wrong
getEndPoint().close();
return false;
}
return true;
}
@Override
protected void completed()
{
LOG.debug("{} completed",this);
LOG.debug(BufferUtil.toDetailString(_requestBuffer));
HttpConnection.this.reset();
// if there is a pipelined request and onReadable has returned
if (_requestBuffer!=null && getCurrentConnection()==null)
execute(new Runnable()
{
@Override public void run() {onReadable();}
});
else if (_parser.isIdle())
scheduleOnReadable();
else if (!getEndPoint().isOutputShutdown() && _parser.getState()==HttpParser.State.SEEKING_EOF)
// if the onReadable method is not executing
if (getCurrentConnection()==null)
{
// TODO This is a catch all indicating some protocol handling failure
// Currently needed for requests saying they are HTTP/2.0.
// This should be removed once better error handling is in place
LOG.warn("Endpoint output not shutdown when seeking EOF");
getEndPoint().close();
// TODO is there a race here?
if (_parser.isIdle())
{
// it wants to eat more
if (_requestBuffer==null)
scheduleOnReadable();
else
{
LOG.debug("{} pipelined",this);
execute(new Runnable()
{
@Override public void run() {onReadable();}
});
}
}
else if (!getEndPoint().isOutputShutdown() && _parser.getState()==HttpParser.State.SEEKING_EOF)
{
// TODO This is a catch all indicating some protocol handling failure
// Currently needed for requests saying they are HTTP/2.0.
// This should be removed once better error handling is in place
LOG.warn("Endpoint output not shutdown when seeking EOF");
getEndPoint().close();
}
}
}
@ -421,8 +459,10 @@ public class HttpConnection extends AbstractAsyncConnection
case FLUSH:
if (_info.isHead())
{
BufferUtil.clear(_chunk);
BufferUtil.clear(_responseBuffer);
if (_chunk!=null)
BufferUtil.clear(_chunk);
if (_responseBuffer!=null)
BufferUtil.clear(_responseBuffer);
}
write(_responseHeader,_chunk,_responseBuffer).get();
continue;
@ -430,8 +470,10 @@ public class HttpConnection extends AbstractAsyncConnection
case FLUSH_CONTENT:
if (_info.isHead())
{
BufferUtil.clear(_chunk);
BufferUtil.clear(content);
if (_chunk!=null)
BufferUtil.clear(_chunk);
if (_responseBuffer!=null)
BufferUtil.clear(content);
}
write(_responseHeader,_chunk,content).get();
break;
@ -512,8 +554,10 @@ public class HttpConnection extends AbstractAsyncConnection
case FLUSH:
if (_info.isHead())
{
BufferUtil.clear(_chunk);
BufferUtil.clear(_responseBuffer);
if (_chunk!=null)
BufferUtil.clear(_chunk);
if (_responseBuffer!=null)
BufferUtil.clear(_responseBuffer);
}
write(_responseHeader,_chunk,_responseBuffer).get();
break;
@ -521,8 +565,10 @@ public class HttpConnection extends AbstractAsyncConnection
case FLUSH_CONTENT:
if (_info.isHead())
{
BufferUtil.clear(_chunk);
BufferUtil.clear(content);
if (_chunk!=null)
BufferUtil.clear(_chunk);
if (_responseBuffer!=null)
BufferUtil.clear(content);
}
// TODO need a proper call back to complete.
write(_responseHeader,_chunk,content);
@ -614,11 +660,10 @@ public class HttpConnection extends AbstractAsyncConnection
that uses the calling thread to block on a readable callback and
then to do the parsing before before attempting the read.
*/
// While progress and the connection has not changed
boolean parsed_event=false;
while (!getEndPoint().isInputShutdown())
boolean parsed_event=_parser.parseNext(_requestBuffer==null?BufferUtil.EMPTY_BUFFER:_requestBuffer);
while (!parsed_event && !getEndPoint().isInputShutdown())
{
try
{

2
jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java
View File

@ -176,7 +176,7 @@ public abstract class HttpInput extends ServletInputStream
public void shutdownInput()
{
synchronized (_inputQ.lock())
{
{
_inputEOF=true;
}
}

41
jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java
View File

@ -40,6 +40,7 @@ import javax.servlet.http.HttpServletResponse;
import junit.framework.Assert;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.HttpServerTestFixture.EchoHandler;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.StringUtil;
@ -171,6 +172,36 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
}
}
@Test
public void testTrailingContent() throws Exception
{
configureServer(new EchoHandler());
Socket client=newSocket(HOST,_connector.getLocalPort());
try
{
OutputStream os=client.getOutputStream();
os.write(("GET /R2 HTTP/1.1\015\012"+
"Host: localhost\015\012"+
"Content-Length: 5\015\012"+
"Content-Type: text/plain\015\012"+
"Connection: close\015\012"+
"\015\012"+
"ABCDE\015\012"+
"\015\012"
).getBytes());
os.flush();
// Read the response.
String response=readResponse(client);
assertTrue (response.indexOf("200")>0);
}
finally
{
client.close();
}
}
/*
* Feed the server fragmentary headers and see how it copes with it.
*/
@ -676,14 +707,12 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
LineNumberReader in = new LineNumberReader(new InputStreamReader(client.getInputStream()));
String line = in.readLine();
System.err.println(line);
int count=0;
while (line!=null)
{
if ("HTTP/1.1 200 OK".equals(line))
count++;
line = in.readLine();
System.err.println(line);
}
assertEquals(pipeline,count);
}
@ -819,8 +848,9 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
).getBytes("iso-8859-1"));
String in = IO.toString(is);
int index=in.indexOf("123456789");
assertTrue(index>0);
index=in.indexOf("123456789",index+1);
@ -944,7 +974,7 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
Socket client=newSocket(HOST,_connector.getLocalPort());
try
{
((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
// ((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
OutputStream os=client.getOutputStream();
InputStream is=client.getInputStream();
@ -956,6 +986,7 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
).getBytes());
os.flush();
client.setSoTimeout(2000);
String in = IO.toString(is);
@ -972,7 +1003,7 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
}
finally
{
((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(false);
// ((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(false);
if (!client.isClosed())
client.close();

10
jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java
View File

@ -77,6 +77,7 @@ public class HttpServerTestFixture
musthavecontent=false;
}
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
@ -92,15 +93,16 @@ public class HttpServerTestFixture
int count=0;
BufferedReader reader=request.getReader();
if (request.getContentLength()!=0)
{
String line;
while ((line=reader.readLine())!=null)
String line=reader.readLine();
while (line!=null)
{
writer.print(line);
writer.print("\n");
count+=line.length();
line=reader.readLine();
}
}
@ -111,7 +113,7 @@ public class HttpServerTestFixture
writer.println("No content");
}
// just to be difficult
reader.close();
writer.close();

14
jetty-server/src/test/java/org/eclipse/jetty/server/SelectChannelServerTest.java
View File

@ -12,7 +12,14 @@
// ========================================================================
package org.eclipse.jetty.server;
import static org.junit.Assert.assertTrue;
import java.io.OutputStream;
import java.net.Socket;
import org.eclipse.jetty.server.HttpServerTestFixture.EchoHandler;
import org.junit.BeforeClass;
import org.junit.Test;
/**
* HttpServer Tester.
@ -31,5 +38,12 @@ public class SelectChannelServerTest extends HttpServerTestBase
super.testCommittedError();
}
@Override
public void testFragmentedChunk() throws Exception
{
super.testFragmentedChunk();
}
}