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

Issue #80 (Spin loop in case of HTTP/2 prefaces without H2C). 

Fixed by changing the state of the parser before calling the parser
handler callbacks, and by closing the parser if the upgrade to HTTP/2
is not successful.
This commit is contained in:
Simone Bordet 2016-02-15 20:48:52 +01:00
parent 6a9002991a
commit 26b6c848f3
4 changed files with 96 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -641,8 +641,8 @@ public class HttpParser
}
else if (ch < HttpTokens.SPACE && ch>=0)
{
handle=_responseHandler.startResponse(_version, _responseStatus, null)||handle;
setState(State.HEADER);
handle=_responseHandler.startResponse(_version, _responseStatus, null)||handle;
}
else
{
@ -714,8 +714,8 @@ public class HttpParser
{
if (_responseHandler!=null)
{
handle=_responseHandler.startResponse(_version, _responseStatus, null)||handle;
setState(State.HEADER);
handle=_responseHandler.startResponse(_version, _responseStatus, null)||handle;
}
else
{
@ -761,7 +761,6 @@ public class HttpParser
if (ch == HttpTokens.LINE_FEED)
{
String reason=takeString();
setState(State.HEADER);
handle=_responseHandler.startResponse(_version, _responseStatus, reason)||handle;
continue;
@ -953,8 +952,8 @@ public class HttpParser
return handle;
case NO_CONTENT:
handle=_handler.headerComplete()||handle;
setState(State.END);
handle=_handler.headerComplete()||handle;
handle=_handler.messageComplete()||handle;
return handle;

94
jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/HTTP2CServerTest.java
View File

@ -18,7 +18,9 @@
package org.eclipse.jetty.http2.server;
import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.Socket;
import java.nio.ByteBuffer;
@ -26,6 +28,7 @@ import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jetty.http.HostPortHttpField;
@ -40,11 +43,17 @@ import org.eclipse.jetty.http2.frames.SettingsFrame;
import org.eclipse.jetty.http2.generator.Generator;
import org.eclipse.jetty.http2.parser.Parser;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.MappedByteBufferPool;
import org.eclipse.jetty.server.NetworkConnector;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpConnection;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.Utf8StringBuilder;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@ -56,27 +65,24 @@ import static org.junit.Assert.assertTrue;
public class HTTP2CServerTest extends AbstractServerTest
{
private HTTP2CServer _server;
private int _port;
@Before
public void before() throws Exception
{
_server = new HTTP2CServer(0);
_server.start();
_port = ((NetworkConnector)_server.getConnectors()[0]).getLocalPort();
server = new HTTP2CServer(0);
server.start();
connector = (ServerConnector)server.getConnectors()[0];
}
@After
public void after() throws Exception
{
_server.stop();
server.stop();
}
@Test
public void testHTTP_1_0_Simple() throws Exception
{
try (Socket client = new Socket("localhost", _port))
try (Socket client = new Socket("localhost", connector.getLocalPort()))
{
client.getOutputStream().write("GET / HTTP/1.0\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1));
client.getOutputStream().flush();
@ -91,7 +97,7 @@ public class HTTP2CServerTest extends AbstractServerTest
@Test
public void testHTTP_1_1_Simple() throws Exception
{
try (Socket client = new Socket("localhost", _port))
try (Socket client = new Socket("localhost", connector.getLocalPort()))
{
client.getOutputStream().write("GET /one HTTP/1.1\r\nHost: localhost\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1));
client.getOutputStream().write("GET /two HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1));
@ -109,7 +115,7 @@ public class HTTP2CServerTest extends AbstractServerTest
@Test
public void testHTTP_1_1_Upgrade() throws Exception
{
try (Socket client = new Socket("localhost", _port))
try (Socket client = new Socket("localhost", connector.getLocalPort()))
{
OutputStream output = client.getOutputStream();
output.write(("" +
@ -186,7 +192,7 @@ public class HTTP2CServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP, new HostPortHttpField("localhost:" + _port), "/two", HttpVersion.HTTP_2, new HttpFields());
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP, new HostPortHttpField("localhost:" + connector.getLocalPort()), "/two", HttpVersion.HTTP_2, new HttpFields());
generator.control(lease, new HeadersFrame(3, metaData, null, true));
for (ByteBuffer buffer : lease.getByteBuffers())
output.write(BufferUtil.toArray(buffer));
@ -222,10 +228,10 @@ public class HTTP2CServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP, new HostPortHttpField("localhost:" + _port), "/test", HttpVersion.HTTP_2, new HttpFields());
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP, new HostPortHttpField("localhost:" + connector.getLocalPort()), "/test", HttpVersion.HTTP_2, new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
try (Socket client = new Socket("localhost", _port))
try (Socket client = new Socket("localhost", connector.getLocalPort()))
{
OutputStream output = client.getOutputStream();
for (ByteBuffer buffer : lease.getByteBuffers())
@ -276,4 +282,64 @@ public class HTTP2CServerTest extends AbstractServerTest
assertThat(s, containsString("uri=/test"));
}
}
@Test
public void testHTTP_2_0_DirectWithoutH2C() throws Exception
{
AtomicLong fills = new AtomicLong();
// Remove "h2c", leaving only "http/1.1".
connector.clearConnectionFactories();
HttpConnectionFactory connectionFactory = new HttpConnectionFactory()
{
@Override
public Connection newConnection(Connector connector, EndPoint endPoint)
{
HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint)
{
@Override
public void onFillable()
{
fills.incrementAndGet();
super.onFillable();
}
};
return configure(connection, connector, endPoint);
}
};
connector.addConnectionFactory(connectionFactory);
connector.setDefaultProtocol(connectionFactory.getProtocol());
// Now send a HTTP/2 direct request, which
// will have the PRI * HTTP/2.0 preface.
byteBufferPool = new MappedByteBufferPool();
generator = new Generator(byteBufferPool);
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
try (Socket client = new Socket("localhost", connector.getLocalPort()))
{
OutputStream output = client.getOutputStream();
for (ByteBuffer buffer : lease.getByteBuffers())
output.write(BufferUtil.toArray(buffer));
// We sent a HTTP/2 preface, but the server has no "h2c" connection
// factory so it does not know how to handle this request.
InputStream input = client.getInputStream();
BufferedReader reader = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8));
String responseLine = reader.readLine();
Assert.assertThat(responseLine, Matchers.containsString(" 426 "));
while (true)
{
if (reader.read() < 0)
break;
}
}
// Make sure we did not spin.
Thread.sleep(1000);
Assert.assertThat(fills.get(), Matchers.lessThan(5L));
}
}

1
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
View File

@ -331,6 +331,7 @@ class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandl
return true;
badMessage(HttpStatus.UPGRADE_REQUIRED_426,null);
_httpConnection.getParser().close();
return false;
}

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

@ -217,19 +217,20 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
HttpConnection last=setCurrentConnection(this);
try
{
while (true)
while (getEndPoint().isOpen())
{
// Fill the request buffer (if needed)
// Fill the request buffer (if needed).
int filled = fillRequestBuffer();
// Parse the request buffer
// Parse the request buffer.
boolean handle = parseRequestBuffer();
// If there was a connection upgrade, the other
// connection took over, nothing more to do here.
if (getEndPoint().getConnection()!=this)
break;
// Handle close parser
// Handle closed parser.
if (_parser.isClose() || _parser.isClosed())
{
close();
@ -245,13 +246,14 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
if (suspended || getEndPoint().getConnection() != this)
break;
}
// Continue or break?
else if (filled<=0)
else
{
if (filled==0)
fillInterested();
break;
if (filled <= 0)
{
if (filled == 0)
fillInterested();
break;
}
}
}
}