diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java index 05a2783dfdb..26a8b5eb2b5 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java @@ -69,6 +69,11 @@ public class HttpField return _name; } + public String getLowerCaseName() + { + return _header != null ? _header.lowerCaseName() : StringUtil.asciiToLowerCase(_name); + } + public String getValue() { return _value; diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java index 826e0cc3d45..03e5b7bc50f 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java @@ -149,6 +149,7 @@ public enum HttpHeader } private final String _string; + private final String _lowerCase; private final byte[] _bytes; private final byte[] _bytesColonSpace; private final ByteBuffer _buffer; @@ -156,11 +157,17 @@ public enum HttpHeader HttpHeader(String s) { _string = s; + _lowerCase = StringUtil.asciiToLowerCase(s); _bytes = StringUtil.getBytes(s); _bytesColonSpace = StringUtil.getBytes(s + ": "); _buffer = ByteBuffer.wrap(_bytes); } + public String lowerCaseName() + { + return _lowerCase; + } + public ByteBuffer toBuffer() { return _buffer.asReadOnlyBuffer(); diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java index b5be288a82b..cb6ee314c53 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java @@ -26,6 +26,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Random; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -44,7 +45,9 @@ import org.eclipse.jetty.http2.api.server.ServerSessionListener; 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.hpack.HpackException; import org.eclipse.jetty.http2.parser.RateControl; import org.eclipse.jetty.http2.parser.ServerParser; import org.eclipse.jetty.http2.server.RawHTTP2ServerConnectionFactory; @@ -58,8 +61,12 @@ import org.eclipse.jetty.util.Jetty; import org.eclipse.jetty.util.Promise; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertEquals; 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 HTTP2Test extends AbstractTest @@ -792,6 +799,100 @@ public class HTTP2Test extends AbstractTest assertTrue(goAwayLatch.await(5, TimeUnit.SECONDS)); } + @Test + public void testClientInvalidHeader() throws Exception + { + start(new EmptyHttpServlet()); + + // A bad header in the request should fail on the client. + Session session = newClient(new Session.Listener.Adapter()); + HttpFields requestFields = new HttpFields(); + requestFields.put(":custom", "special"); + MetaData.Request metaData = newRequest("GET", requestFields); + HeadersFrame request = new HeadersFrame(metaData, null, true); + FuturePromise promise = new FuturePromise<>(); + session.newStream(request, promise, new Stream.Listener.Adapter()); + ExecutionException x = assertThrows(ExecutionException.class, () -> promise.get(5, TimeUnit.SECONDS)); + assertThat(x.getCause(), instanceOf(HpackException.StreamException.class)); + } + + @Test + public void testServerInvalidHeader() throws Exception + { + start(new EmptyHttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) + { + response.setHeader(":custom", "special"); + } + }); + + // Good request with bad header in the response. + Session session = newClient(new Session.Listener.Adapter()); + MetaData.Request metaData = newRequest("GET", new HttpFields()); + HeadersFrame request = new HeadersFrame(metaData, null, true); + FuturePromise promise = new FuturePromise<>(); + CountDownLatch resetLatch = new CountDownLatch(1); + session.newStream(request, promise, new Stream.Listener.Adapter() + { + @Override + public void onReset(Stream stream, ResetFrame frame) + { + resetLatch.countDown(); + } + }); + Stream stream = promise.get(5, TimeUnit.SECONDS); + assertNotNull(stream); + + assertTrue(resetLatch.await(5, TimeUnit.SECONDS)); + } + + @Test + public void testServerInvalidHeaderFlushed() throws Exception + { + CountDownLatch serverFailure = new CountDownLatch(1); + start(new EmptyHttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setHeader(":custom", "special"); + try + { + response.flushBuffer(); + } + catch (IOException x) + { + assertThat(x.getCause(), instanceOf(HpackException.StreamException.class)); + serverFailure.countDown(); + throw x; + } + } + }); + + // Good request with bad header in the response. + Session session = newClient(new Session.Listener.Adapter()); + MetaData.Request metaData = newRequest("GET", "/flush", new HttpFields()); + HeadersFrame request = new HeadersFrame(metaData, null, true); + FuturePromise promise = new FuturePromise<>(); + CountDownLatch resetLatch = new CountDownLatch(1); + session.newStream(request, promise, new Stream.Listener.Adapter() + { + @Override + public void onReset(Stream stream, ResetFrame frame) + { + // Cannot receive a 500 because we force the flush on the server, so + // the response is committed even if the server was not able to write it. + resetLatch.countDown(); + } + }); + Stream stream = promise.get(5, TimeUnit.SECONDS); + assertNotNull(stream); + assertTrue(serverFailure.await(5, TimeUnit.SECONDS)); + assertTrue(resetLatch.await(5, TimeUnit.SECONDS)); + } + private static void sleep(long time) { try diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java index 1751968b9da..31e1064e448 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java @@ -35,6 +35,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.HTTP2Session; import org.eclipse.jetty.http2.api.Session; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.api.server.ServerSessionListener; @@ -295,7 +296,34 @@ public class TrailersTest extends AbstractTest } @Test - public void testRequestTrailerInvalidHpack() throws Exception + public void testRequestTrailerInvalidHpackSent() throws Exception + { + start(new EmptyHttpServlet()); + + Session session = newClient(new Session.Listener.Adapter()); + MetaData.Request request = newRequest("POST", new HttpFields()); + HeadersFrame requestFrame = new HeadersFrame(request, null, false); + FuturePromise promise = new FuturePromise<>(); + session.newStream(requestFrame, promise, new Stream.Listener.Adapter()); + Stream stream = promise.get(5, TimeUnit.SECONDS); + ByteBuffer data = ByteBuffer.wrap(StringUtil.getUtf8Bytes("hello")); + Callback.Completable completable = new Callback.Completable(); + stream.data(new DataFrame(stream.getId(), data, false), completable); + CountDownLatch failureLatch = new CountDownLatch(1); + completable.thenRun(() -> + { + // Invalid trailer: cannot contain pseudo headers. + HttpFields trailerFields = new HttpFields(); + trailerFields.put(HttpHeader.C_METHOD, "GET"); + MetaData trailer = new MetaData(HttpVersion.HTTP_2, trailerFields); + HeadersFrame trailerFrame = new HeadersFrame(stream.getId(), trailer, null, true); + stream.headers(trailerFrame, Callback.from(Callback.NOOP::succeeded, x -> failureLatch.countDown())); + }); + assertTrue(failureLatch.await(5, TimeUnit.SECONDS)); + } + + @Test + public void testRequestTrailerInvalidHpackReceived() throws Exception { CountDownLatch serverLatch = new CountDownLatch(1); start(new HttpServlet() @@ -341,6 +369,8 @@ public class TrailersTest extends AbstractTest stream.data(new DataFrame(stream.getId(), data, false), completable); completable.thenRun(() -> { + // Disable checks for invalid headers. + ((HTTP2Session)session).getGenerator().setValidateHpackEncoding(false); // Invalid trailer: cannot contain pseudo headers. HttpFields trailerFields = new HttpFields(); trailerFields.put(HttpHeader.C_METHOD, "GET"); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java index 350751ead3c..c974987a7b8 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java @@ -30,6 +30,7 @@ import java.util.Set; import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.WindowUpdateFrame; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.util.Callback; @@ -207,6 +208,13 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable } } } + catch (HpackException.StreamException failure) + { + if (LOG.isDebugEnabled()) + LOG.debug("Failure generating " + entry, failure); + entry.failed(failure); + pending.remove(); + } catch (Throwable failure) { // Failure to generate the entry is catastrophic. @@ -397,7 +405,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable return 0; } - protected abstract boolean generate(ByteBufferPool.Lease lease); + protected abstract boolean generate(ByteBufferPool.Lease lease) throws HpackException; public abstract long onFlushed(long bytes) throws IOException; diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index b646f349b01..1a55d2e3b97 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -49,6 +49,7 @@ import org.eclipse.jetty.http2.frames.ResetFrame; import org.eclipse.jetty.http2.frames.SettingsFrame; import org.eclipse.jetty.http2.frames.WindowUpdateFrame; import org.eclipse.jetty.http2.generator.Generator; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.http2.parser.Parser; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.EndPoint; @@ -1218,7 +1219,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } @Override - protected boolean generate(ByteBufferPool.Lease lease) + protected boolean generate(ByteBufferPool.Lease lease) throws HpackException { frameBytes = generator.control(lease, frame); beforeSend(); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/FrameGenerator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/FrameGenerator.java index 0194beccc27..915dc7e448d 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/FrameGenerator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/FrameGenerator.java @@ -20,8 +20,11 @@ package org.eclipse.jetty.http2.generator; import java.nio.ByteBuffer; +import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.FrameType; +import org.eclipse.jetty.http2.hpack.HpackEncoder; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.io.ByteBufferPool; public abstract class FrameGenerator @@ -33,7 +36,7 @@ public abstract class FrameGenerator this.headerGenerator = headerGenerator; } - public abstract int generate(ByteBufferPool.Lease lease, Frame frame); + public abstract int generate(ByteBufferPool.Lease lease, Frame frame) throws HpackException; protected ByteBuffer generateHeader(ByteBufferPool.Lease lease, FrameType frameType, int length, int flags, int streamId) { @@ -44,4 +47,19 @@ public abstract class FrameGenerator { return headerGenerator.getMaxFrameSize(); } + + protected ByteBuffer encode(HpackEncoder encoder, ByteBufferPool.Lease lease, MetaData metaData, int maxFrameSize) throws HpackException + { + ByteBuffer hpacked = lease.acquire(maxFrameSize, false); + try + { + encoder.encode(hpacked, metaData); + return hpacked; + } + catch (HpackException x) + { + lease.release(hpacked); + throw x; + } + } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/Generator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/Generator.java index b7288d3897a..005f0b8b6f2 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/Generator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/Generator.java @@ -22,6 +22,7 @@ import org.eclipse.jetty.http2.frames.DataFrame; import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.hpack.HpackEncoder; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.io.ByteBufferPool; public class Generator @@ -65,6 +66,11 @@ public class Generator return byteBufferPool; } + public void setValidateHpackEncoding(boolean validateEncoding) + { + hpackEncoder.setValidateEncoding(validateEncoding); + } + public void setHeaderTableSize(int headerTableSize) { hpackEncoder.setRemoteMaxDynamicTableSize(headerTableSize); @@ -75,7 +81,7 @@ public class Generator headerGenerator.setMaxFrameSize(maxFrameSize); } - public int control(ByteBufferPool.Lease lease, Frame frame) + public int control(ByteBufferPool.Lease lease, Frame frame) throws HpackException { return generators[frame.getType().getType()].generate(lease, frame); } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/HeadersGenerator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/HeadersGenerator.java index 4ff03bd28e5..8388c49a942 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/HeadersGenerator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/HeadersGenerator.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.frames.HeadersFrame; import org.eclipse.jetty.http2.frames.PriorityFrame; import org.eclipse.jetty.http2.hpack.HpackEncoder; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.BufferUtil; @@ -50,13 +51,13 @@ public class HeadersGenerator extends FrameGenerator } @Override - public int generate(ByteBufferPool.Lease lease, Frame frame) + public int generate(ByteBufferPool.Lease lease, Frame frame) throws HpackException { HeadersFrame headersFrame = (HeadersFrame)frame; return generateHeaders(lease, headersFrame.getStreamId(), headersFrame.getMetaData(), headersFrame.getPriority(), headersFrame.isEndStream()); } - public int generateHeaders(ByteBufferPool.Lease lease, int streamId, MetaData metaData, PriorityFrame priority, boolean endStream) + public int generateHeaders(ByteBufferPool.Lease lease, int streamId, MetaData metaData, PriorityFrame priority, boolean endStream) throws HpackException { if (streamId < 0) throw new IllegalArgumentException("Invalid stream id: " + streamId); @@ -66,10 +67,7 @@ public class HeadersGenerator extends FrameGenerator if (priority != null) flags = Flags.PRIORITY; - int maxFrameSize = getMaxFrameSize(); - ByteBuffer hpacked = lease.acquire(maxFrameSize, false); - BufferUtil.clearToFill(hpacked); - encoder.encode(hpacked, metaData); + ByteBuffer hpacked = encode(encoder, lease, metaData, getMaxFrameSize()); int hpackedLength = hpacked.position(); BufferUtil.flipToFlush(hpacked, 0); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java index d4fe2640ef9..dda2da4d588 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.frames.PushPromiseFrame; import org.eclipse.jetty.http2.hpack.HpackEncoder; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.BufferUtil; @@ -40,13 +41,13 @@ public class PushPromiseGenerator extends FrameGenerator } @Override - public int generate(ByteBufferPool.Lease lease, Frame frame) + public int generate(ByteBufferPool.Lease lease, Frame frame) throws HpackException { PushPromiseFrame pushPromiseFrame = (PushPromiseFrame)frame; return generatePushPromise(lease, pushPromiseFrame.getStreamId(), pushPromiseFrame.getPromisedStreamId(), pushPromiseFrame.getMetaData()); } - public int generatePushPromise(ByteBufferPool.Lease lease, int streamId, int promisedStreamId, MetaData metaData) + public int generatePushPromise(ByteBufferPool.Lease lease, int streamId, int promisedStreamId, MetaData metaData) throws HpackException { if (streamId < 0) throw new IllegalArgumentException("Invalid stream id: " + streamId); @@ -58,9 +59,7 @@ public class PushPromiseGenerator extends FrameGenerator int extraSpace = 4; maxFrameSize -= extraSpace; - ByteBuffer hpacked = lease.acquire(maxFrameSize, false); - BufferUtil.clearToFill(hpacked); - encoder.encode(hpacked, metaData); + ByteBuffer hpacked = encode(encoder, lease, metaData, maxFrameSize); int hpackedLength = hpacked.position(); BufferUtil.flipToFlush(hpacked, 0); diff --git a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java index 04685dcb743..4ce0eb7d1b6 100644 --- a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java +++ b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java @@ -74,7 +74,7 @@ public class FrameFloodTest } @Test - public void testInvalidHeadersFrameFlood() + public void testInvalidHeadersFrameFlood() throws Exception { // Invalid MetaData (no method, no scheme, etc). MetaData.Request metadata = new MetaData.Request(null, (String)null, null, null, HttpVersion.HTTP_2, null, -1); diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java index 412f09ebc2a..ec3718ed098 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java @@ -261,7 +261,7 @@ public class HpackContext _dynamicTableSizeInBytes += size; _dynamicTable.add(entry); _fieldMap.put(field, entry); - _nameMap.put(StringUtil.asciiToLowerCase(field.getName()), entry); + _nameMap.put(field.getLowerCaseName(), entry); if (LOG.isDebugEnabled()) LOG.debug(String.format("HdrTbl[%x] added %s", hashCode(), entry)); @@ -383,7 +383,7 @@ public class HpackContext _dynamicTableSizeInBytes -= entry.getSize(); entry._slot = -1; _fieldMap.remove(entry.getHttpField()); - String lc = StringUtil.asciiToLowerCase(entry.getHttpField().getName()); + String lc = entry.getHttpField().getLowerCaseName(); if (entry == _nameMap.get(lc)) _nameMap.remove(lc); } diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java index 2fab4294cb1..57cc4ba3ce6 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.http2.hpack; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.util.EnumMap; import java.util.EnumSet; import java.util.HashSet; import java.util.Set; @@ -27,6 +28,7 @@ import java.util.Set; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; @@ -77,6 +79,9 @@ public class HpackEncoder private static final EnumSet IGNORED_HEADERS = EnumSet.of(HttpHeader.CONNECTION, HttpHeader.KEEP_ALIVE, HttpHeader.PROXY_CONNECTION, HttpHeader.TRANSFER_ENCODING, HttpHeader.UPGRADE); private static final PreEncodedHttpField TE_TRAILERS = new PreEncodedHttpField(HttpHeader.TE, "trailers"); + private static final PreEncodedHttpField C_SCHEME_HTTP = new PreEncodedHttpField(HttpHeader.C_SCHEME, "http"); + private static final PreEncodedHttpField C_SCHEME_HTTPS = new PreEncodedHttpField(HttpHeader.C_SCHEME, "https"); + private static final EnumMap C_METHODS = new EnumMap<>(HttpMethod.class); static { @@ -84,6 +89,10 @@ public class HpackEncoder { STATUSES[code.getCode()] = new PreEncodedHttpField(HttpHeader.C_STATUS, Integer.toString(code.getCode())); } + for (HttpMethod method : HttpMethod.values()) + { + C_METHODS.put(method, new PreEncodedHttpField(HttpHeader.C_METHOD, method.asString())); + } } private final HpackContext _context; @@ -92,6 +101,7 @@ public class HpackEncoder private int _localMaxDynamicTableSize; private int _maxHeaderListSize; private int _headerListSize; + private boolean _validateEncoding = true; public HpackEncoder() { @@ -142,80 +152,118 @@ public class HpackEncoder _localMaxDynamicTableSize = localMaxDynamicTableSize; } - public void encode(ByteBuffer buffer, MetaData metadata) + public boolean isValidateEncoding() { - if (LOG.isDebugEnabled()) - LOG.debug(String.format("CtxTbl[%x] encoding", _context.hashCode())); + return _validateEncoding; + } - _headerListSize = 0; - int pos = buffer.position(); + public void setValidateEncoding(boolean validateEncoding) + { + _validateEncoding = validateEncoding; + } - // Check the dynamic table sizes! - int maxDynamicTableSize = Math.min(_remoteMaxDynamicTableSize, _localMaxDynamicTableSize); - if (maxDynamicTableSize != _context.getMaxDynamicTableSize()) - encodeMaxDynamicTableSize(buffer, maxDynamicTableSize); - - // Add Request/response meta fields - if (metadata.isRequest()) + public void encode(ByteBuffer buffer, MetaData metadata) throws HpackException + { + try { - MetaData.Request request = (MetaData.Request)metadata; - - // TODO optimise these to avoid HttpField creation - String scheme = request.getURI().getScheme(); - encode(buffer, new HttpField(HttpHeader.C_SCHEME, scheme == null ? HttpScheme.HTTP.asString() : scheme)); - encode(buffer, new HttpField(HttpHeader.C_METHOD, request.getMethod())); - encode(buffer, new HttpField(HttpHeader.C_AUTHORITY, request.getURI().getAuthority())); - encode(buffer, new HttpField(HttpHeader.C_PATH, request.getURI().getPathQuery())); - } - else if (metadata.isResponse()) - { - MetaData.Response response = (MetaData.Response)metadata; - int code = response.getStatus(); - HttpField status = code < STATUSES.length ? STATUSES[code] : null; - if (status == null) - status = new HttpField.IntValueHttpField(HttpHeader.C_STATUS, code); - encode(buffer, status); - } - - // Remove fields as specified in RFC 7540, 8.1.2.2. - HttpFields fields = metadata.getFields(); - if (fields != null) - { - // For example: Connection: Close, TE, Upgrade, Custom. - Set hopHeaders = null; - for (String value : fields.getCSV(HttpHeader.CONNECTION, false)) - { - if (hopHeaders == null) - hopHeaders = new HashSet<>(); - hopHeaders.add(StringUtil.asciiToLowerCase(value)); - } - for (HttpField field : fields) - { - HttpHeader header = field.getHeader(); - if (header != null && IGNORED_HEADERS.contains(header)) - continue; - if (header == HttpHeader.TE) - { - if (field.contains("trailers")) - encode(buffer, TE_TRAILERS); - continue; - } - if (hopHeaders != null && hopHeaders.contains(StringUtil.asciiToLowerCase(field.getName()))) - continue; - encode(buffer, field); - } - } - - // Check size - if (_maxHeaderListSize > 0 && _headerListSize > _maxHeaderListSize) - { - LOG.warn("Header list size too large {} > {} for {}", _headerListSize, _maxHeaderListSize); if (LOG.isDebugEnabled()) - LOG.debug("metadata={}", metadata); - } + LOG.debug(String.format("CtxTbl[%x] encoding", _context.hashCode())); - if (LOG.isDebugEnabled()) - LOG.debug(String.format("CtxTbl[%x] encoded %d octets", _context.hashCode(), buffer.position() - pos)); + HttpFields fields = metadata.getFields(); + // Verify that we can encode without errors. + if (isValidateEncoding() && fields != null) + { + for (HttpField field : fields) + { + String name = field.getName(); + char firstChar = name.charAt(0); + if (firstChar <= ' ' || firstChar == ':') + throw new HpackException.StreamException("Invalid header name: '%s'", name); + } + } + + _headerListSize = 0; + int pos = buffer.position(); + + // Check the dynamic table sizes! + int maxDynamicTableSize = Math.min(_remoteMaxDynamicTableSize, _localMaxDynamicTableSize); + if (maxDynamicTableSize != _context.getMaxDynamicTableSize()) + encodeMaxDynamicTableSize(buffer, maxDynamicTableSize); + + // Add Request/response meta fields + if (metadata.isRequest()) + { + MetaData.Request request = (MetaData.Request)metadata; + + String scheme = request.getURI().getScheme(); + encode(buffer, HttpScheme.HTTPS.is(scheme) ? C_SCHEME_HTTPS : C_SCHEME_HTTP); + String method = request.getMethod(); + HttpMethod httpMethod = method == null ? null : HttpMethod.fromString(method); + HttpField methodField = C_METHODS.get(httpMethod); + encode(buffer, methodField == null ? new HttpField(HttpHeader.C_METHOD, method) : methodField); + encode(buffer, new HttpField(HttpHeader.C_AUTHORITY, request.getURI().getAuthority())); + encode(buffer, new HttpField(HttpHeader.C_PATH, request.getURI().getPathQuery())); + } + else if (metadata.isResponse()) + { + MetaData.Response response = (MetaData.Response)metadata; + int code = response.getStatus(); + HttpField status = code < STATUSES.length ? STATUSES[code] : null; + if (status == null) + status = new HttpField.IntValueHttpField(HttpHeader.C_STATUS, code); + encode(buffer, status); + } + + // Remove fields as specified in RFC 7540, 8.1.2.2. + if (fields != null) + { + // For example: Connection: Close, TE, Upgrade, Custom. + Set hopHeaders = null; + for (String value : fields.getCSV(HttpHeader.CONNECTION, false)) + { + if (hopHeaders == null) + hopHeaders = new HashSet<>(); + hopHeaders.add(StringUtil.asciiToLowerCase(value)); + } + for (HttpField field : fields) + { + HttpHeader header = field.getHeader(); + if (header != null && IGNORED_HEADERS.contains(header)) + continue; + if (header == HttpHeader.TE) + { + if (field.contains("trailers")) + encode(buffer, TE_TRAILERS); + continue; + } + String name = field.getLowerCaseName(); + if (hopHeaders != null && hopHeaders.contains(name)) + continue; + encode(buffer, field); + } + } + + // Check size + if (_maxHeaderListSize > 0 && _headerListSize > _maxHeaderListSize) + { + LOG.warn("Header list size too large {} > {} for {}", _headerListSize, _maxHeaderListSize); + if (LOG.isDebugEnabled()) + LOG.debug("metadata={}", metadata); + } + + if (LOG.isDebugEnabled()) + LOG.debug(String.format("CtxTbl[%x] encoded %d octets", _context.hashCode(), buffer.position() - pos)); + } + catch (HpackException x) + { + throw x; + } + catch (Throwable x) + { + HpackException.SessionException failure = new HpackException.SessionException("Could not hpack encode %s", metadata); + failure.initCause(x); + throw failure; + } } public void encodeMaxDynamicTableSize(ByteBuffer buffer, int maxDynamicTableSize) diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java index 2fada9a66c5..bfa0c796bf0 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java @@ -32,13 +32,10 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertEquals; -/** - * - */ public class HpackEncoderTest { @Test - public void testUnknownFieldsContextManagement() + public void testUnknownFieldsContextManagement() throws Exception { HpackEncoder encoder = new HpackEncoder(38 * 5); HttpFields fields = new HttpFields(); @@ -149,7 +146,7 @@ public class HpackEncoderTest } @Test - public void testNeverIndexSetCookie() + public void testNeverIndexSetCookie() throws Exception { HpackEncoder encoder = new HpackEncoder(38 * 5); ByteBuffer buffer = BufferUtil.allocate(4096); @@ -181,7 +178,7 @@ public class HpackEncoderTest } @Test - public void testFieldLargerThanTable() + public void testFieldLargerThanTable() throws Exception { HttpFields fields = new HttpFields(); @@ -199,6 +196,7 @@ public class HpackEncoderTest BufferUtil.flipToFlush(buffer1, pos); encoder = new HpackEncoder(128); + encoder.setValidateEncoding(false); fields.add(new HttpField(":path", "This is a very large field, whose size is larger than the dynamic table so it should not be indexed as it will not fit in the table ever!" + "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX " + @@ -210,6 +208,7 @@ public class HpackEncoderTest BufferUtil.flipToFlush(buffer2, pos); encoder = new HpackEncoder(128); + encoder.setValidateEncoding(false); fields.add(new HttpField("host", "somehost")); ByteBuffer buffer = BufferUtil.allocate(4096); pos = BufferUtil.flipToFill(buffer); @@ -243,7 +242,7 @@ public class HpackEncoderTest } @Test - public void testResize() + public void testResize() throws Exception { HttpFields fields = new HttpFields(); fields.add("host", "localhost0"); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackPerfTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackPerfTest.java index 959a064eafb..faedf6e840d 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackPerfTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackPerfTest.java @@ -20,7 +20,6 @@ package org.eclipse.jetty.http2.hpack; import java.io.File; import java.io.FileReader; -import java.io.FilenameFilter; import java.nio.ByteBuffer; import java.util.Map; @@ -34,6 +33,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertNotNull; + public class HpackPerfTest { int _maxDynamicTableSize = 4 * 1024; @@ -56,28 +57,22 @@ public class HpackPerfTest @Test public void simpleTest() throws Exception { - runStories(_maxDynamicTableSize); + runStories(); } - private void runStories(int maxDynamicTableSize) throws Exception + private void runStories() throws Exception { // Find files File data = MavenTestingUtils.getTestResourceDir("data"); - String[] files = data.list(new FilenameFilter() - { - @Override - public boolean accept(File dir, String name) - { - return name.startsWith("story_"); - } - }); + String[] files = data.list((dir, name) -> name.startsWith("story_")); + assertNotNull(files); // Parse JSON - Map[] stories = new Map[files.length]; + Map[] stories = new Map[files.length]; int i = 0; for (String story : files) { - stories[i++] = (Map)JSON.parse(new FileReader(new File(data, story))); + stories[i++] = (Map)JSON.parse(new FileReader(new File(data, story))); } ByteBuffer buffer = BufferUtil.allocate(256 * 1024); @@ -93,25 +88,27 @@ public class HpackPerfTest encodeStories(buffer, stories, "response"); } - private void encodeStories(ByteBuffer buffer, Map[] stories, String type) throws Exception + private void encodeStories(ByteBuffer buffer, Map[] stories, String type) throws Exception { - for (Map story : stories) + for (Map story : stories) { if (type.equals(story.get("context"))) { HpackEncoder encoder = new HpackEncoder(_maxDynamicTableSize, _maxDynamicTableSize); + encoder.setValidateEncoding(false); // System.err.println(story); Object[] cases = (Object[])story.get("cases"); for (Object c : cases) { // System.err.println(" "+c); - Object[] headers = (Object[])((Map)c).get("headers"); + Object[] headers = (Object[])((Map)c).get("headers"); // System.err.println(" "+headers); HttpFields fields = new HttpFields(); for (Object header : headers) { - Map h = (Map)header; + @SuppressWarnings("unchecked") + Map h = (Map)header; Map.Entry e = h.entrySet().iterator().next(); fields.add(e.getKey(), e.getValue()); _unencodedSize += e.getKey().length() + e.getValue().length(); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java index 245f925cd2f..9abd3af859c 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java @@ -36,6 +36,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; public class HpackTest @@ -251,6 +252,27 @@ public class HpackTest assertEquals(trailerValue, output.get(HttpHeader.TRAILER)); } + @Test + public void testColonHeaders() throws Exception + { + HpackEncoder encoder = new HpackEncoder(); + HpackDecoder decoder = new HpackDecoder(4096, 16384); + + HttpFields input = new HttpFields(); + input.put(":status", "200"); + input.put(":custom", "special"); + + ByteBuffer buffer = BufferUtil.allocate(2048); + BufferUtil.clearToFill(buffer); + assertThrows(HpackException.StreamException.class, () -> encoder.encode(buffer, new MetaData(HttpVersion.HTTP_2, input))); + + encoder.setValidateEncoding(false); + encoder.encode(buffer, new MetaData(HttpVersion.HTTP_2, input)); + + BufferUtil.flipToFlush(buffer, 0); + assertThrows(HpackException.StreamException.class, () -> decoder.decode(buffer)); + } + private void assertMetaDataResponseSame(MetaData.Response expected, MetaData.Response actual) { assertThat("Response.status", actual.getStatus(), is(expected.getStatus())); 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 9304ae519c8..4d2827149c7 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 @@ -61,6 +61,7 @@ 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.generator.Generator; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.http2.parser.RateControl; import org.eclipse.jetty.http2.parser.ServerParser; import org.eclipse.jetty.http2.server.RawHTTP2ServerConnectionFactory; @@ -74,6 +75,7 @@ import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -467,21 +469,35 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest @Override public void onPreface() { - // Server's preface. - generator.control(lease, new SettingsFrame(new HashMap<>(), false)); - // Reply to client's SETTINGS. - generator.control(lease, new SettingsFrame(new HashMap<>(), true)); - writeFrames(); + try + { + // Server's preface. + generator.control(lease, new SettingsFrame(new HashMap<>(), false)); + // Reply to client's SETTINGS. + generator.control(lease, new SettingsFrame(new HashMap<>(), true)); + writeFrames(); + } + catch (HpackException x) + { + x.printStackTrace(); + } } @Override public void onHeaders(HeadersFrame request) { - // Response. - MetaData.Response metaData = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); - HeadersFrame response = new HeadersFrame(request.getStreamId(), metaData, null, true); - generator.control(lease, response); - writeFrames(); + try + { + // Response. + MetaData.Response metaData = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); + HeadersFrame response = new HeadersFrame(request.getStreamId(), metaData, null, true); + generator.control(lease, response); + writeFrames(); + } + catch (HpackException x) + { + x.printStackTrace(); + } } private void writeFrames() @@ -573,6 +589,8 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest @Override public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) { + // Disable checks for invalid headers. + ((HTTP2Session)stream.getSession()).getGenerator().setValidateHpackEncoding(false); // Produce an invalid HPACK block by adding a request pseudo-header to the response. HttpFields fields = new HttpFields(); fields.put(":method", "get"); @@ -601,6 +619,7 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest @Disabled @Test + @Tag("external") public void testExternalServer() throws Exception { HTTP2Client http2Client = new HTTP2Client(); diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java index 3c41615bea8..301699e774b 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java @@ -127,11 +127,16 @@ public interface ByteBufferPool { ByteBuffer buffer = buffers.get(i); if (recycles.get(i)) - byteBufferPool.release(buffer); + release(buffer); } buffers.clear(); recycles.clear(); } + + public void release(ByteBuffer buffer) + { + byteBufferPool.release(buffer); + } } class Bucket