Skip to content

Commit 52e4a0e

Browse files
committed
Fix dynamic table cpu burn #10472
Signed-off-by: Daniel Kec <[email protected]>
1 parent 31bc817 commit 52e4a0e

File tree

4 files changed

+51
-7
lines changed

4 files changed

+51
-7
lines changed

http/http2/src/main/java/io/helidon/http/http2/Http2Headers.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,7 @@ void write(BufferData buffer) {
11241124

11251125
/**
11261126
* There is one dynamic table for inbound headers and one for outbound headers for each connection.
1127-
* This is to minimize size of headers on the transport.
1127+
* This is to minimize the size of headers on the transport.
11281128
* The table caches header names and values and then uses indexes only when transferring headers over network.
11291129
*/
11301130
public static class DynamicTable {
@@ -1175,8 +1175,8 @@ void maxTableSize(long number) {
11751175
}
11761176
this.maxTableSize = number;
11771177
if (maxTableSize == 0) {
1178-
currentTableSize = 0;
11791178
headers.clear();
1179+
currentTableSize = 0;
11801180
}
11811181
while (maxTableSize < currentTableSize) {
11821182
evict();
@@ -1193,7 +1193,7 @@ int add(HeaderName headerName, String headerValue) {
11931193

11941194
while ((currentTableSize + size) > maxTableSize) {
11951195
evict();
1196-
if (currentTableSize <= 0) {
1196+
if (maxTableSize - currentTableSize < size) {
11971197
throw new Http2Exception(Http2ErrorCode.COMPRESSION,
11981198
"Cannot add header record, max table size too low. "
11991199
+ "current size: " + currentTableSize + ", max size: " + maxTableSize + ","
@@ -1244,6 +1244,7 @@ private IndexedHeaderRecord find(HeaderName headerName, String headerValue) {
12441244

12451245
private void evict() {
12461246
if (headers.isEmpty()) {
1247+
currentTableSize = 0;
12471248
return;
12481249
}
12491250
DynamicHeader removed = headers.remove(headers.size() - 1);

http/http2/src/test/java/io/helidon/http/http2/DynamicTableTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package io.helidon.http.http2;
1818

19+
import java.nio.charset.StandardCharsets;
20+
1921
import io.helidon.http.HeaderNames;
2022

2123
import org.junit.jupiter.api.Test;
@@ -67,6 +69,46 @@ void testIssue10472() {
6769
table.maxTableSize(0L);
6870
}
6971

72+
@Test
73+
void testIssue10472addOnlyOneFit() {
74+
Http2Settings settings = Http2Settings.builder()
75+
.add(Http2Setting.HEADER_TABLE_SIZE, 45L)
76+
.build();
77+
78+
Http2Headers.DynamicTable table = Http2Headers.DynamicTable.create(settings);
79+
table.add(HeaderNames.create("a"), "aa"); // 35
80+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "a", "aa");
81+
table.add(HeaderNames.create("b"), "b"); // 34
82+
83+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "b", "b");
84+
85+
table.add(HeaderNames.create("c"), "c"); // 34
86+
table.add(HeaderNames.create("dddd"), "ddddd"); // 41
87+
88+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "dddd", "ddddd");
89+
}
90+
91+
@Test
92+
void testIssue10472addOnlyTwoFit() {
93+
Http2Settings settings = Http2Settings.builder()
94+
.add(Http2Setting.HEADER_TABLE_SIZE, 75L)
95+
.build();
96+
97+
Http2Headers.DynamicTable table = Http2Headers.DynamicTable.create(settings);
98+
table.add(HeaderNames.create("a"), "aa"); // 35
99+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "a", "aa");
100+
table.add(HeaderNames.create("b"), "b"); // 34
101+
102+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "b", "b");
103+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 2, "a", "aa");
104+
105+
table.add(HeaderNames.create("c"), "c"); // 34
106+
table.add(HeaderNames.create("dddd"), "ddddd"); // 41
107+
108+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "dddd", "ddddd");
109+
testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 2, "c", "c");
110+
}
111+
70112
private void testRecord(Http2Headers.DynamicTable table,
71113
int index,
72114
String expectedName,

tests/integration/h2spec/pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@
4949
</dependency>
5050
<dependency>
5151
<groupId>io.helidon.logging</groupId>
52-
<artifactId>helidon-logging-jul</artifactId>
52+
<artifactId>helidon-logging-slf4j</artifactId>
5353
</dependency>
5454
<dependency>
5555
<groupId>org.slf4j</groupId>
56-
<artifactId>slf4j-jdk14</artifactId>
56+
<artifactId>slf4j-simple</artifactId>
5757
</dependency>
5858
<dependency>
5959
<groupId>org.junit.jupiter</groupId>

tests/integration/h2spec/src/test/java/H2SpecIT.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024 Oracle and/or its affiliates.
2+
* Copyright (c) 2024, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -35,6 +35,7 @@
3535
import org.slf4j.Logger;
3636
import org.slf4j.LoggerFactory;
3737
import org.testcontainers.containers.GenericContainer;
38+
import org.testcontainers.containers.output.Slf4jLogConsumer;
3839
import org.testcontainers.containers.wait.strategy.Wait;
3940
import org.testcontainers.images.PullPolicy;
4041
import org.testcontainers.images.builder.ImageFromDockerfile;
@@ -88,8 +89,8 @@ private static Stream<Arguments> runH2Spec() {
8889
try (var cont = new GenericContainer<>(
8990
new ImageFromDockerfile().withDockerfile(Path.of("./Dockerfile")))
9091
.withAccessToHost(true)
92+
.withLogConsumer(new Slf4jLogConsumer(LoggerFactory.getLogger("h2spec")))
9193
.withImagePullPolicy(PullPolicy.ageBased(Duration.ofDays(365)))
92-
.withLogConsumer(outputFrame -> LOGGER.info(outputFrame.getUtf8StringWithoutLineEnding()))
9394
.waitingFor(Wait.forLogMessage(".*Finished in.*", 1))) {
9495

9596
org.testcontainers.Testcontainers.exposeHostPorts(port);

0 commit comments

Comments
 (0)