From 18aeca6567ba6ea0d5ba19782f670bc7fed4662e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 16 Mar 2016 22:14:01 +0100 Subject: [PATCH] Fixes #242 (Expose HTTP/2 LastStream error) --- .../jetty/http2/frames/GoAwayFrame.java | 8 +- .../client/http/HttpChannelOverHTTP2.java | 2 + .../HttpClientTransportOverHTTP2Test.java | 82 +++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java index f327860c741..c5805cc8c62 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.http2.frames; import java.nio.ByteBuffer; +import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.util.BufferUtil; public class GoAwayFrame extends Frame @@ -69,6 +70,11 @@ public class GoAwayFrame extends Frame @Override public String toString() { - return String.format("%s,%d/%s", super.toString(), error, tryConvertPayload()); + ErrorCode errorCode = ErrorCode.from(error); + return String.format("%s,%d/%s/%s", + super.toString(), + lastStreamId, + errorCode != null ? errorCode.toString() : String.valueOf(error), + tryConvertPayload()); } } diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java index df999131b73..eb41f55a96d 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.client.HttpReceiver; import org.eclipse.jetty.client.HttpSender; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.HTTP2Stream; import org.eclipse.jetty.http2.api.Session; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.ResetFrame; @@ -77,6 +78,7 @@ public class HttpChannelOverHTTP2 extends HttpChannel public void setStream(Stream stream) { this.stream = stream; + getHttpExchange().getRequest().attribute(HTTP2Stream.class.getName(), stream); } @Override diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java index 2f52810e071..2882c2b7cf7 100644 --- a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java @@ -20,10 +20,13 @@ package org.eclipse.jetty.http2.client.http; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -32,15 +35,23 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.HTTP2Stream; +import org.eclipse.jetty.http2.api.Session; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.api.server.ServerSessionListener; import org.eclipse.jetty.http2.client.HTTP2Client; import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.GoAwayFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; import org.eclipse.jetty.http2.frames.ResetFrame; +import org.eclipse.jetty.http2.frames.SettingsFrame; +import org.eclipse.jetty.http2.server.RawHTTP2ServerConnectionFactory; +import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.Callback; @@ -178,6 +189,77 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest Assert.assertEquals(HttpStatus.OK_200, response.getStatus()); } + @Test + public void testLastStreamId() throws Exception + { + prepareServer(new RawHTTP2ServerConnectionFactory(new HttpConfiguration(), new ServerSessionListener.Adapter() + { + @Override + public Map onPreface(Session session) + { + Map settings = new HashMap<>(); + settings.put(SettingsFrame.MAX_CONCURRENT_STREAMS, 1); + return settings; + } + + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + MetaData.Request request = (MetaData.Request)frame.getMetaData(); + if (HttpMethod.HEAD.is(request.getMethod())) + { + stream.getSession().close(ErrorCode.REFUSED_STREAM_ERROR.code, null, Callback.NOOP); + } + else + { + MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); + stream.headers(new HeadersFrame(stream.getId(), response, null, true), Callback.NOOP); + } + return null; + } + })); + server.start(); + + CountDownLatch latch = new CountDownLatch(2); + AtomicInteger lastStream = new AtomicInteger(); + client = new HttpClient(new HttpClientTransportOverHTTP2(new HTTP2Client()) + { + @Override + protected void onClose(HttpConnectionOverHTTP2 connection, GoAwayFrame frame) + { + super.onClose(connection, frame); + lastStream.set(frame.getLastStreamId()); + latch.countDown(); + } + }, null); + QueuedThreadPool clientExecutor = new QueuedThreadPool(); + clientExecutor.setName("client"); + client.setExecutor(clientExecutor); + client.start(); + + // Prime the connection to allow client and server prefaces to be exchanged. + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .path("/zero") + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.assertEquals(HttpStatus.OK_200, response.getStatus()); + + org.eclipse.jetty.client.api.Request request = client.newRequest("localhost", connector.getLocalPort()) + .method(HttpMethod.HEAD) + .path("/one"); + request.send(result -> + { + if (result.isFailed()) + latch.countDown(); + }); + + Assert.assertTrue(latch.await(5, TimeUnit.SECONDS)); + + Stream stream = (Stream)request.getAttributes().get(HTTP2Stream.class.getName()); + Assert.assertNotNull(stream); + Assert.assertEquals(lastStream.get(), stream.getId()); + } + @Ignore @Test public void testExternalServer() throws Exception