This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 01d733ce99ea0d1c575308d207a967b814be8c82 Author: opencode <[email protected]> AuthorDate: Tue Oct 13 15:05:01 2026 +0200 Correct the dynamic table index calculation after HPACK encoder roll-over When the encoder's entry position counter reached Integer.MAX_VALUE, preventPositionRollover() reset the counter to zero but rebased each surviving dynamic table entry's stored position to the value returned by getPosition(). For dynamic entries that value is the absolute HPACK index, already including the static table offset, so the offset was applied twice on every later lookup: each surviving entry's index became 61 too large (for example an entry at index 62 would be referenced as index 123). A conformant peer would treat those dynamic table references as invalid, resulting in a COMPRESSION_ERROR connection close. Rebase the stored position to the entry's position relative to the start of the dynamic table (the absolute index minus the static table length) so the offset is only applied once by getPosition(). The trigger requires around 2^31 incremental insertions on a single connection so this is effectively unreachable in practice. Add a regression test that moves the counter close to the roll-over point via reflection and verifies the encoder's dynamic table references are valid for the decoder after the roll-over prevention logic runs. --- java/org/apache/coyote/http2/HpackEncoder.java | 9 ++++-- test/org/apache/coyote/http2/TestHpack.java | 43 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/java/org/apache/coyote/http2/HpackEncoder.java b/java/org/apache/coyote/http2/HpackEncoder.java index af7d0e606c..e93c08e2ea 100644 --- a/java/org/apache/coyote/http2/HpackEncoder.java +++ b/java/org/apache/coyote/http2/HpackEncoder.java @@ -288,11 +288,14 @@ class HpackEncoder { private void preventPositionRollover() { - // if the position counter is about to roll over we iterate all the table entries - // and set their position to their actual position + // if the position counter is about to roll over we iterate all the + // table entries and set their position to their position relative to + // the start of the dynamic table (i.e. excluding the static table + // offset) since that, plus the (reset) counter value plus the static + // table length, is the value from which their index is calculated for (List<TableEntry> tableEntries : dynamicTable.values()) { for (TableEntry t : tableEntries) { - t.position = t.getPosition(); + t.position = t.getPosition() - Hpack.STATIC_TABLE_LENGTH; } } entryPositionCounter = 0; diff --git a/test/org/apache/coyote/http2/TestHpack.java b/test/org/apache/coyote/http2/TestHpack.java index 6525f214ce..fea5250a88 100644 --- a/test/org/apache/coyote/http2/TestHpack.java +++ b/test/org/apache/coyote/http2/TestHpack.java @@ -16,6 +16,7 @@ */ package org.apache.coyote.http2; +import java.lang.reflect.Field; import java.nio.ByteBuffer; import org.junit.Assert; @@ -85,6 +86,48 @@ public class TestHpack { Assert.assertEquals("value2", headers2.getHeader("header2")); } + + @Test + public void testEncodeAfterEntryPositionCounterRollover() throws Exception { + // Test for correct index computation for dynamic table entries after + // the position counter roll-over prevention logic is triggered. The + // roll-over requires ~2^31 insertions to trigger organically so + // reflection is used to move the counter close to the roll-over point. + MimeHeaders headers = new MimeHeaders(); + headers.setValue("header1").setString("value1"); + headers.setValue("header2").setString("value2"); + ByteBuffer output = ByteBuffer.allocate(512); + HpackEncoder encoder = new HpackEncoder(); + + Field field = HpackEncoder.class.getDeclaredField("entryPositionCounter"); + field.setAccessible(true); + // The insertion of header2 below pushes the counter to + // Integer.MAX_VALUE which triggers the roll-over prevention logic + field.setInt(encoder, Integer.MAX_VALUE - 2); + + HpackDecoder decoder = new HpackDecoder(); + MimeHeaders headers2 = new MimeHeaders(); + decoder.setHeaderEmitter(new HeadersListener(headers2)); + + // Encode and decode. Both dynamic tables now contain the two entries + // with header2 at index 62 and header1 at index 63 + encoder.encode(headers, output); + output.flip(); + decoder.decode(output); + output.clear(); + + // Re-send the headers. The encoder should reference the dynamic table + // entries using the same indexes the decoder has for them + encoder.encode(headers, output); + output.flip(); + headers2.recycle(); + decoder.decode(output); + output.clear(); + + Assert.assertEquals("value1", headers2.getHeader("header1")); + Assert.assertEquals("value2", headers2.getHeader("header2")); + } + private static class HeadersListener implements HpackDecoder.HeaderEmitter { private final MimeHeaders headers; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
