From 55eb98218577f9986cdfb15ebf47b8b9cb44b4ff Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 22 Feb 2022 11:36:37 +0100 Subject: [PATCH] Fixes #7625 - HTTP/3 error against www.google.com Now properly handling QPACK and HTTP/3 settings. Signed-off-by: Simone Bordet --- .../client/internal/ClientHTTP3Session.java | 5 ++ .../client/internal/HTTP3SessionClient.java | 20 +++++++ .../jetty/http3/frames/SettingsFrame.java | 9 ++- .../jetty/http3/internal/HTTP3Session.java | 23 ++++++++ .../jetty/http3/qpack/QpackDecoder.java | 29 +++++++++- .../jetty/http3/qpack/QpackEncoder.java | 19 ++++++- .../server/internal/HTTP3SessionServer.java | 20 +++++++ .../server/internal/ServerHTTP3Session.java | 5 ++ .../jetty/http3/tests/ClientServerTest.java | 56 +++++++++++++++++++ .../jetty/http3/tests/ExternalServerTest.java | 3 +- 10 files changed, 182 insertions(+), 7 deletions(-) diff --git a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/ClientHTTP3Session.java b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/ClientHTTP3Session.java index be9911a8e80c..9dc44c73d253 100644 --- a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/ClientHTTP3Session.java +++ b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/ClientHTTP3Session.java @@ -92,6 +92,11 @@ public QpackDecoder getQpackDecoder() return decoder; } + public QpackEncoder getQpackEncoder() + { + return encoder; + } + public HTTP3SessionClient getSessionClient() { return session; diff --git a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java index e817564a02d3..92c1f3325d40 100644 --- a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java +++ b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java @@ -146,4 +146,24 @@ protected GoAwayFrame newGoAwayFrame(boolean graceful) return GoAwayFrame.CLIENT_GRACEFUL; return super.newGoAwayFrame(graceful); } + + @Override + protected void onSettingMaxTableCapacity(long value) + { + getProtocolSession().getQpackEncoder().setCapacity((int)value); + } + + @Override + protected void onSettingMaxFieldSectionSize(long value) + { + getProtocolSession().getQpackDecoder().setMaxHeaderSize((int)value); + } + + @Override + protected void onSettingMaxBlockedStreams(long value) + { + ClientHTTP3Session session = getProtocolSession(); + session.getQpackDecoder().setMaxBlockedStreams((int)value); + session.getQpackEncoder().setMaxBlockedStreams((int)value); + } } diff --git a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/frames/SettingsFrame.java b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/frames/SettingsFrame.java index 8e44fbb25313..be8af79c72a4 100644 --- a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/frames/SettingsFrame.java +++ b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/frames/SettingsFrame.java @@ -17,11 +17,18 @@ public class SettingsFrame extends Frame { + public static final long MAX_TABLE_CAPACITY = 0x01; public static final long MAX_FIELD_SECTION_SIZE = 0x06; + public static final long MAX_BLOCKED_STREAMS = 0x07; public static boolean isReserved(long key) { - return key >= 0 && key <= 5; + if (key == MAX_TABLE_CAPACITY || + key == MAX_FIELD_SECTION_SIZE || + key == MAX_BLOCKED_STREAMS) + return false; + // Other HTTP/2 settings are reserved and must not be sent/received. + return key >= 0x00 && key <= 0x05; } private final Map settings; diff --git a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java index ea5fab8dc145..2ffd225548c0 100644 --- a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java +++ b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java @@ -371,9 +371,32 @@ public void onSettings(SettingsFrame frame) { if (LOG.isDebugEnabled()) LOG.debug("received {} on {}", frame, this); + + frame.getSettings().forEach((key, value) -> + { + if (key == SettingsFrame.MAX_TABLE_CAPACITY) + onSettingMaxTableCapacity(value); + else if (key == SettingsFrame.MAX_FIELD_SECTION_SIZE) + onSettingMaxFieldSectionSize(value); + else if (key == SettingsFrame.MAX_BLOCKED_STREAMS) + onSettingMaxBlockedStreams(value); + }); + notifySettings(frame); } + protected void onSettingMaxTableCapacity(long value) + { + } + + protected void onSettingMaxFieldSectionSize(long value) + { + } + + protected void onSettingMaxBlockedStreams(long value) + { + } + private void notifySettings(SettingsFrame frame) { try diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java index 31279e3ef48b..4129979ca68b 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java @@ -50,7 +50,8 @@ public class QpackDecoder implements Dumpable private final List _encodedFieldSections = new ArrayList<>(); private final NBitIntegerParser _integerDecoder = new NBitIntegerParser(); private final InstructionHandler _instructionHandler = new InstructionHandler(); - private final int _maxHeaderSize; + private int _maxHeaderSize; + private int _maxBlockedStreams; private static class MetaDataNotification { @@ -87,6 +88,27 @@ QpackContext getQpackContext() return _context; } + public int getMaxHeaderSize() + { + return _maxHeaderSize; + } + + public void setMaxHeaderSize(int maxHeaderSize) + { + _maxHeaderSize = maxHeaderSize; + } + + public int getMaxBlockedStreams() + { + // TODO: implement logic about blocked streams by calling this method. + return _maxBlockedStreams; + } + + public void setMaxBlockedStreams(int maxBlockedStreams) + { + _maxBlockedStreams = maxBlockedStreams; + } + public interface Handler { void onMetaData(long streamId, MetaData metadata); @@ -110,7 +132,8 @@ public boolean decode(long streamId, ByteBuffer buffer, Handler handler) throws LOG.debug("Decoding: streamId={}, buffer={}", streamId, BufferUtil.toDetailString(buffer)); // If the buffer is big, don't even think about decoding it - if (buffer.remaining() > _maxHeaderSize) + int maxHeaderSize = getMaxHeaderSize(); + if (buffer.remaining() > maxHeaderSize) throw new QpackException.SessionException(QPACK_DECOMPRESSION_FAILED, "header_too_large"); _integerDecoder.setPrefix(8); @@ -139,7 +162,7 @@ public boolean decode(long streamId, ByteBuffer buffer, Handler handler) throws // Decode it straight away if we can, otherwise add it to the list of EncodedFieldSections. if (requiredInsertCount <= insertCount) { - MetaData metaData = encodedFieldSection.decode(_context, _maxHeaderSize); + MetaData metaData = encodedFieldSection.decode(_context, maxHeaderSize); if (LOG.isDebugEnabled()) LOG.debug("Decoded: streamId={}, metadata={}", streamId, metaData); _metaDataNotifications.add(new MetaDataNotification(streamId, metaData, handler)); diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java index ae7ccd24d285..7105b5257bf9 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java @@ -91,7 +91,7 @@ public class QpackEncoder implements Dumpable private final List _instructions = new ArrayList<>(); private final Instruction.Handler _handler; private final QpackContext _context; - private final int _maxBlockedStreams; + private int _maxBlockedStreams; private final Map _streamInfoMap = new HashMap<>(); private final EncoderInstructionParser _parser; private final InstructionHandler _instructionHandler = new InstructionHandler(); @@ -106,6 +106,21 @@ public QpackEncoder(Instruction.Handler handler, int maxBlockedStreams) _parser = new EncoderInstructionParser(_instructionHandler); } + public int getMaxBlockedStreams() + { + return _maxBlockedStreams; + } + + public void setMaxBlockedStreams(int maxBlockedStreams) + { + _maxBlockedStreams = maxBlockedStreams; + } + + public int getCapacity() + { + return _context.getDynamicTable().getCapacity(); + } + /** * Set the capacity of the DynamicTable and send a instruction to set the capacity on the remote Decoder. * @@ -411,7 +426,7 @@ private boolean referenceEntry(Entry entry, StreamInfo streamInfo) return true; } - if (_blockedStreams < _maxBlockedStreams) + if (_blockedStreams < getMaxBlockedStreams()) { _blockedStreams++; sectionInfo.block(); diff --git a/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HTTP3SessionServer.java b/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HTTP3SessionServer.java index 504688b3648a..02af205db982 100644 --- a/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HTTP3SessionServer.java +++ b/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HTTP3SessionServer.java @@ -95,6 +95,26 @@ protected GoAwayFrame newGoAwayFrame(boolean graceful) return super.newGoAwayFrame(graceful); } + @Override + protected void onSettingMaxTableCapacity(long value) + { + getProtocolSession().getQpackEncoder().setCapacity((int)value); + } + + @Override + protected void onSettingMaxFieldSectionSize(long value) + { + getProtocolSession().getQpackDecoder().setMaxHeaderSize((int)value); + } + + @Override + protected void onSettingMaxBlockedStreams(long value) + { + ServerHTTP3Session session = getProtocolSession(); + session.getQpackDecoder().setMaxBlockedStreams((int)value); + session.getQpackEncoder().setMaxBlockedStreams((int)value); + } + private void notifyAccept() { Server.Listener listener = getListener(); diff --git a/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3Session.java b/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3Session.java index ac52b5a42802..c62103d28d59 100644 --- a/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3Session.java +++ b/jetty-http3/http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3Session.java @@ -91,6 +91,11 @@ public QpackDecoder getQpackDecoder() return decoder; } + public QpackEncoder getQpackEncoder() + { + return encoder; + } + public HTTP3SessionServer getSessionServer() { return session; diff --git a/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ClientServerTest.java b/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ClientServerTest.java index b5b7c3c42647..e6a9b7bf687c 100644 --- a/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ClientServerTest.java +++ b/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ClientServerTest.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http3.tests; import java.nio.ByteBuffer; +import java.util.AbstractMap; import java.util.Map; import java.util.Random; import java.util.concurrent.CountDownLatch; @@ -34,6 +35,7 @@ import org.eclipse.jetty.http3.internal.HTTP3ErrorCode; import org.eclipse.jetty.http3.internal.HTTP3Session; import org.eclipse.jetty.http3.server.AbstractHTTP3ServerConnectionFactory; +import org.eclipse.jetty.http3.server.internal.HTTP3SessionServer; import org.eclipse.jetty.quic.client.ClientQuicSession; import org.eclipse.jetty.quic.common.QuicSession; import org.junit.jupiter.api.Test; @@ -41,6 +43,7 @@ import org.junit.jupiter.params.provider.ValueSource; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -90,6 +93,59 @@ public void onSettings(Session session, SettingsFrame frame) assertTrue(clientSettingsLatch.await(5, TimeUnit.SECONDS)); } + @Test + public void testSettings() throws Exception + { + Map.Entry maxTableCapacity = new AbstractMap.SimpleEntry<>(SettingsFrame.MAX_TABLE_CAPACITY, 1024L); + Map.Entry maxHeaderSize = new AbstractMap.SimpleEntry<>(SettingsFrame.MAX_FIELD_SECTION_SIZE, 2048L); + Map.Entry maxBlockedStreams = new AbstractMap.SimpleEntry<>(SettingsFrame.MAX_BLOCKED_STREAMS, 16L); + CountDownLatch settingsLatch = new CountDownLatch(2); + AtomicReference serverSessionRef = new AtomicReference<>(); + start(new Session.Server.Listener() + { + @Override + public Map onPreface(Session session) + { + serverSessionRef.set((HTTP3SessionServer)session); + return Map.ofEntries(maxTableCapacity, maxHeaderSize, maxBlockedStreams); + } + + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + }); + + HTTP3SessionClient clientSession = (HTTP3SessionClient)newSession(new Session.Client.Listener() + { + @Override + public Map onPreface(Session session) + { + return Map.ofEntries(maxTableCapacity, maxHeaderSize, maxBlockedStreams); + } + + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + }); + + assertTrue(settingsLatch.await(5, TimeUnit.SECONDS)); + + HTTP3SessionServer serverSession = serverSessionRef.get(); + assertEquals(maxTableCapacity.getValue(), serverSession.getProtocolSession().getQpackEncoder().getCapacity()); + assertEquals(maxBlockedStreams.getValue(), serverSession.getProtocolSession().getQpackEncoder().getMaxBlockedStreams()); + assertEquals(maxBlockedStreams.getValue(), serverSession.getProtocolSession().getQpackDecoder().getMaxBlockedStreams()); + assertEquals(maxHeaderSize.getValue(), serverSession.getProtocolSession().getQpackDecoder().getMaxHeaderSize()); + + assertEquals(maxTableCapacity.getValue(), clientSession.getProtocolSession().getQpackEncoder().getCapacity()); + assertEquals(maxBlockedStreams.getValue(), clientSession.getProtocolSession().getQpackEncoder().getMaxBlockedStreams()); + assertEquals(maxBlockedStreams.getValue(), clientSession.getProtocolSession().getQpackDecoder().getMaxBlockedStreams()); + assertEquals(maxHeaderSize.getValue(), clientSession.getProtocolSession().getQpackDecoder().getMaxHeaderSize()); + } + @Test public void testGETThenResponseWithoutContent() throws Exception { diff --git a/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ExternalServerTest.java b/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ExternalServerTest.java index d5fcf90d9244..42737e2bf3f1 100644 --- a/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ExternalServerTest.java +++ b/jetty-http3/http3-tests/src/test/java/org/eclipse/jetty/http3/tests/ExternalServerTest.java @@ -44,8 +44,9 @@ public void testExternalServer() throws Exception client.start(); try { + HostPort hostPort = new HostPort("google.com:443"); // HostPort hostPort = new HostPort("nghttp2.org:4433"); - HostPort hostPort = new HostPort("quic.tech:8443"); +// HostPort hostPort = new HostPort("quic.tech:8443"); // HostPort hostPort = new HostPort("h2o.examp1e.net:443"); // HostPort hostPort = new HostPort("test.privateoctopus.com:4433"); Session.Client session = client.connect(new InetSocketAddress(hostPort.getHost(), hostPort.getPort()), new Session.Client.Listener() {})