Merged branch 'jetty-12.0.x' into 'jetty-12.1.x'.
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
commit
cffda533e3
|
@ -95,8 +95,7 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory
|
|||
public void onOpen()
|
||||
{
|
||||
Map<Integer, Integer> settings = listener.onPreface(getSession());
|
||||
if (settings == null)
|
||||
settings = new HashMap<>();
|
||||
settings = settings == null ? new HashMap<>() : new HashMap<>(settings);
|
||||
|
||||
// Below we want to populate any settings to send to the server
|
||||
// that have a different default than what prescribed by the RFC.
|
||||
|
|
|
@ -1248,6 +1248,11 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
|
|||
this.stream = stream;
|
||||
}
|
||||
|
||||
public Frame frame()
|
||||
{
|
||||
return frame;
|
||||
}
|
||||
|
||||
public abstract int getFrameBytesGenerated();
|
||||
|
||||
public int getDataBytesRemaining()
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
package org.eclipse.jetty.http2.internal;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.channels.ClosedChannelException;
|
||||
import java.util.ArrayDeque;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
|
@ -24,10 +25,12 @@ import java.util.List;
|
|||
import java.util.Queue;
|
||||
import java.util.Set;
|
||||
|
||||
import org.eclipse.jetty.http2.ErrorCode;
|
||||
import org.eclipse.jetty.http2.FlowControlStrategy;
|
||||
import org.eclipse.jetty.http2.HTTP2Connection;
|
||||
import org.eclipse.jetty.http2.HTTP2Session;
|
||||
import org.eclipse.jetty.http2.HTTP2Stream;
|
||||
import org.eclipse.jetty.http2.frames.FrameType;
|
||||
import org.eclipse.jetty.http2.frames.WindowUpdateFrame;
|
||||
import org.eclipse.jetty.http2.hpack.HpackException;
|
||||
import org.eclipse.jetty.io.EndPoint;
|
||||
|
@ -95,10 +98,9 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
|
|||
entries.offerFirst(entry);
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Prepended {}, entries={}", entry, entries.size());
|
||||
return true;
|
||||
}
|
||||
}
|
||||
if (closed == null)
|
||||
return true;
|
||||
closed(entry, closed);
|
||||
return false;
|
||||
}
|
||||
|
@ -109,15 +111,17 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
|
|||
try (AutoLock ignored = lock.lock())
|
||||
{
|
||||
closed = terminated;
|
||||
// If it was not possible to HPACK encode, then allow to send a GOAWAY.
|
||||
if (closed instanceof HpackException.SessionException && entry.frame().getType() == FrameType.GO_AWAY)
|
||||
closed = null;
|
||||
if (closed == null)
|
||||
{
|
||||
entries.offer(entry);
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Appended {}, entries={}, {}", entry, entries.size(), this);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
if (closed == null)
|
||||
return true;
|
||||
closed(entry, closed);
|
||||
return false;
|
||||
}
|
||||
|
@ -133,10 +137,9 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
|
|||
list.forEach(entries::offer);
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Appended {}, entries={} {}", list, entries.size(), this);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
if (closed == null)
|
||||
return true;
|
||||
list.forEach(entry -> closed(entry, closed));
|
||||
return false;
|
||||
}
|
||||
|
@ -166,7 +169,21 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
|
|||
try (AutoLock ignored = lock.lock())
|
||||
{
|
||||
if (terminated != null)
|
||||
throw terminated;
|
||||
{
|
||||
boolean rethrow = true;
|
||||
if (terminated instanceof HpackException.SessionException)
|
||||
{
|
||||
HTTP2Session.Entry entry = entries.peek();
|
||||
if (entry != null && entry.frame().getType() == FrameType.GO_AWAY)
|
||||
{
|
||||
// Allow a SessionException to be processed once to send a GOAWAY.
|
||||
terminated = new ClosedChannelException().initCause(terminated);
|
||||
rethrow = false;
|
||||
}
|
||||
}
|
||||
if (rethrow)
|
||||
throw terminated;
|
||||
}
|
||||
|
||||
WindowEntry windowEntry;
|
||||
while ((windowEntry = windows.poll()) != null)
|
||||
|
@ -251,6 +268,15 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
|
|||
entry.failed(failure);
|
||||
pending.remove();
|
||||
}
|
||||
catch (HpackException.SessionException failure)
|
||||
{
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Failure generating {}", entry, failure);
|
||||
onSessionFailure(failure);
|
||||
// The method above will try to send
|
||||
// a GOAWAY, so we will iterate again.
|
||||
return Action.IDLE;
|
||||
}
|
||||
catch (Throwable failure)
|
||||
{
|
||||
// Failure to generate the entry is catastrophic.
|
||||
|
@ -345,6 +371,29 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
|
|||
|
||||
@Override
|
||||
protected void onFailure(Throwable x)
|
||||
{
|
||||
Throwable closed = fail(x);
|
||||
// If the failure came from within the
|
||||
// flusher, we need to close the connection.
|
||||
if (closed == null)
|
||||
session.onWriteFailure(x);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void onCompleteFailure(Throwable x)
|
||||
{
|
||||
release();
|
||||
}
|
||||
|
||||
private void onSessionFailure(Throwable x)
|
||||
{
|
||||
release();
|
||||
Throwable closed = fail(x);
|
||||
if (closed == null)
|
||||
session.close(ErrorCode.COMPRESSION_ERROR.code, null, NOOP);
|
||||
}
|
||||
|
||||
private Throwable fail(Throwable x)
|
||||
{
|
||||
Throwable closed;
|
||||
Set<HTTP2Session.Entry> allEntries;
|
||||
|
@ -367,17 +416,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
|
|||
allEntries.addAll(pendingEntries);
|
||||
pendingEntries.clear();
|
||||
allEntries.forEach(entry -> entry.failed(x));
|
||||
|
||||
// If the failure came from within the
|
||||
// flusher, we need to close the connection.
|
||||
if (closed == null)
|
||||
session.onWriteFailure(x);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void onCompleteFailure(Throwable x)
|
||||
{
|
||||
release();
|
||||
return closed;
|
||||
}
|
||||
|
||||
public void terminate(Throwable cause)
|
||||
|
|
|
@ -403,11 +403,20 @@ public class HTTP2ServerTest extends AbstractServerTest
|
|||
accumulator.writeTo(Content.Sink.from(output), false);
|
||||
output.flush();
|
||||
|
||||
AtomicBoolean goAway = new AtomicBoolean();
|
||||
Parser parser = new Parser(bufferPool, 8192);
|
||||
parser.init(new Parser.Listener() {});
|
||||
parser.init(new Parser.Listener()
|
||||
{
|
||||
@Override
|
||||
public void onGoAway(GoAwayFrame frame)
|
||||
{
|
||||
goAway.set(true);
|
||||
}
|
||||
});
|
||||
boolean closed = parseResponse(client, parser);
|
||||
|
||||
assertTrue(closed);
|
||||
assertFalse(closed);
|
||||
assertTrue(goAway.get());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -16,7 +16,9 @@ package org.eclipse.jetty.http2.tests;
|
|||
import java.nio.ByteBuffer;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.CompletableFuture;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.ExecutionException;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
|
@ -37,9 +39,17 @@ import org.eclipse.jetty.http2.frames.SettingsFrame;
|
|||
import org.eclipse.jetty.http2.hpack.HpackException;
|
||||
import org.eclipse.jetty.io.RetainableByteBuffer;
|
||||
import org.eclipse.jetty.util.Callback;
|
||||
import org.junit.jupiter.api.Assertions;
|
||||
import org.hamcrest.Matchers;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.awaitility.Awaitility.await;
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.notNullValue;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
public class SettingsTest extends AbstractTest
|
||||
{
|
||||
@Test
|
||||
|
@ -107,11 +117,11 @@ public class SettingsTest extends AbstractTest
|
|||
.flip();
|
||||
((HTTP2Session)clientSession).getEndPoint().write(Callback.NOOP, byteBuffer);
|
||||
|
||||
Assertions.assertFalse(serverSettingsLatch.get().await(1, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(serverFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(serverCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(clientGoAwayLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(clientCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
assertFalse(serverSettingsLatch.get().await(1, TimeUnit.SECONDS));
|
||||
assertTrue(serverFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(serverCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(clientGoAwayLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(clientCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -181,11 +191,11 @@ public class SettingsTest extends AbstractTest
|
|||
.flip();
|
||||
((HTTP2Session)clientSession).getEndPoint().write(Callback.NOOP, byteBuffer);
|
||||
|
||||
Assertions.assertFalse(serverSettingsLatch.get().await(1, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(serverFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(serverCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(clientGoAwayLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(clientCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
assertFalse(serverSettingsLatch.get().await(1, TimeUnit.SECONDS));
|
||||
assertTrue(serverFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(serverCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(clientGoAwayLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(clientCloseLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -213,7 +223,7 @@ public class SettingsTest extends AbstractTest
|
|||
}
|
||||
});
|
||||
|
||||
Assertions.assertTrue(serverFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(serverFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -242,7 +252,7 @@ public class SettingsTest extends AbstractTest
|
|||
}
|
||||
});
|
||||
|
||||
Assertions.assertTrue(clientFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(clientFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -304,9 +314,9 @@ public class SettingsTest extends AbstractTest
|
|||
}
|
||||
});
|
||||
|
||||
Assertions.assertTrue(serverPushFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertTrue(clientResponseLatch.await(5, TimeUnit.SECONDS));
|
||||
Assertions.assertFalse(clientPushLatch.await(1, TimeUnit.SECONDS));
|
||||
assertTrue(serverPushFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(clientResponseLatch.await(5, TimeUnit.SECONDS));
|
||||
assertFalse(clientPushLatch.await(1, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -362,6 +372,122 @@ public class SettingsTest extends AbstractTest
|
|||
HeadersFrame frame = new HeadersFrame(request, null, true);
|
||||
clientSession.newStream(frame, Stream.Listener.AUTO_DISCARD);
|
||||
|
||||
Assertions.assertTrue(clientFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
assertTrue(clientFailureLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testMaxHeaderListSizeExceededServerSendsGoAway() throws Exception
|
||||
{
|
||||
int maxHeadersSize = 512;
|
||||
start(new ServerSessionListener()
|
||||
{
|
||||
@Override
|
||||
public void onSettings(Session session, SettingsFrame frame)
|
||||
{
|
||||
((HTTP2Session)session).getParser().getHpackDecoder().setMaxHeaderListSize(maxHeadersSize);
|
||||
}
|
||||
});
|
||||
|
||||
CountDownLatch goAwayLatch = new CountDownLatch(1);
|
||||
Session clientSession = newClientSession(new Session.Listener()
|
||||
{
|
||||
@Override
|
||||
public void onGoAway(Session session, GoAwayFrame frame)
|
||||
{
|
||||
goAwayLatch.countDown();
|
||||
}
|
||||
});
|
||||
HttpFields requestHeaders = HttpFields.build()
|
||||
.put("X-Large", "x".repeat(maxHeadersSize * 2));
|
||||
MetaData.Request request = newRequest("GET", requestHeaders);
|
||||
HeadersFrame frame = new HeadersFrame(request, null, true);
|
||||
Stream stream = clientSession.newStream(frame, new Stream.Listener() {}).get(5, TimeUnit.SECONDS);
|
||||
|
||||
// The request can be sent by the client, the server will reject it.
|
||||
// The spec suggests to send 431, but we do not want to "taint" the
|
||||
// HPACK context with large headers.
|
||||
assertNotNull(stream);
|
||||
|
||||
// The server should send a GOAWAY.
|
||||
assertTrue(goAwayLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testMaxHeaderListSizeExceededByClient() throws Exception
|
||||
{
|
||||
int maxHeadersSize = 512;
|
||||
CountDownLatch goAwayLatch = new CountDownLatch(1);
|
||||
start(new ServerSessionListener()
|
||||
{
|
||||
@Override
|
||||
public Map<Integer, Integer> onPreface(Session session)
|
||||
{
|
||||
return Map.of(SettingsFrame.MAX_HEADER_LIST_SIZE, maxHeadersSize);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onGoAway(Session session, GoAwayFrame frame)
|
||||
{
|
||||
goAwayLatch.countDown();
|
||||
}
|
||||
});
|
||||
|
||||
Session clientSession = newClientSession(new Session.Listener() {});
|
||||
HttpFields requestHeaders = HttpFields.build()
|
||||
.put("X-Large", "x".repeat(maxHeadersSize * 2));
|
||||
MetaData.Request request = newRequest("GET", requestHeaders);
|
||||
HeadersFrame frame = new HeadersFrame(request, null, true);
|
||||
|
||||
Throwable failure = assertThrows(ExecutionException.class,
|
||||
() -> clientSession.newStream(frame, new Stream.Listener() {}).get(5, TimeUnit.SECONDS))
|
||||
.getCause();
|
||||
// The HPACK context is compromised trying to encode the large header.
|
||||
assertThat(failure, Matchers.instanceOf(HpackException.SessionException.class));
|
||||
|
||||
assertTrue(goAwayLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testMaxHeaderListSizeExceededByServer() throws Exception
|
||||
{
|
||||
int maxHeadersSize = 512;
|
||||
AtomicReference<CompletableFuture<Stream>> responseRef = new AtomicReference<>();
|
||||
start(new ServerSessionListener()
|
||||
{
|
||||
@Override
|
||||
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
|
||||
{
|
||||
HttpFields responseHeaders = HttpFields.build()
|
||||
.put("X-Large", "x".repeat(maxHeadersSize * 2));
|
||||
MetaData.Response response = new MetaData.Response(HttpStatus.OK_200, null, HttpVersion.HTTP_2, responseHeaders);
|
||||
responseRef.set(stream.headers(new HeadersFrame(stream.getId(), response, null, true)));
|
||||
return null;
|
||||
}
|
||||
});
|
||||
|
||||
CountDownLatch goAwayLatch = new CountDownLatch(1);
|
||||
Session clientSession = newClientSession(new Session.Listener()
|
||||
{
|
||||
@Override
|
||||
public Map<Integer, Integer> onPreface(Session session)
|
||||
{
|
||||
return Map.of(SettingsFrame.MAX_HEADER_LIST_SIZE, maxHeadersSize);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onGoAway(Session session, GoAwayFrame frame)
|
||||
{
|
||||
goAwayLatch.countDown();
|
||||
}
|
||||
});
|
||||
MetaData.Request request = newRequest("GET", HttpFields.EMPTY);
|
||||
HeadersFrame frame = new HeadersFrame(request, null, true);
|
||||
clientSession.newStream(frame, new Stream.Listener() {});
|
||||
|
||||
CompletableFuture<Stream> completable = await().atMost(5, TimeUnit.SECONDS).until(responseRef::get, notNullValue());
|
||||
Throwable failure = assertThrows(ExecutionException.class, () -> completable.get(5, TimeUnit.SECONDS)).getCause();
|
||||
assertThat(failure, Matchers.instanceOf(HpackException.SessionException.class));
|
||||
|
||||
assertTrue(goAwayLatch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue