Skip to content

Commit 86cbf49

Browse files
arturobernalgfitekonea
authored andcommitted
Fix HPACK table size update sequencing
Signal the minimum pending table size first and the final size second
1 parent ddaccde commit 86cbf49

2 files changed

Lines changed: 155 additions & 6 deletions

File tree

httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackEncoder.java

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,14 @@ public final class HPackEncoder {
5555
private final CharsetEncoder charsetEncoder;
5656
private ByteBuffer tmpBuf;
5757
private int maxTableSize;
58+
private int minTableSize;
5859

5960
HPackEncoder(final OutboundDynamicTable dynamicTable, final CharsetEncoder charsetEncoder) {
6061
this.dynamicTable = Objects.requireNonNull(dynamicTable);
6162
this.huffmanBuf = new ByteArrayBuffer(128);
6263
this.charsetEncoder = charsetEncoder;
6364
this.maxTableSize = this.dynamicTable.getMaxSize();
65+
this.minTableSize = -1;
6466
}
6567

6668
HPackEncoder(final OutboundDynamicTable dynamicTable, final Charset charset) {
@@ -144,6 +146,19 @@ private void ensureCapacity(final int extra) {
144146
}
145147
}
146148

149+
private void encodeMaxTableSizeUpdates(final ByteArrayBuffer dst) {
150+
if (this.minTableSize == -1) {
151+
return;
152+
}
153+
encodeInt(dst, 5, this.minTableSize, 0x20);
154+
this.dynamicTable.setMaxSize(this.minTableSize);
155+
if (this.maxTableSize != this.minTableSize) {
156+
encodeInt(dst, 5, this.maxTableSize, 0x20);
157+
this.dynamicTable.setMaxSize(this.maxTableSize);
158+
}
159+
this.minTableSize = -1;
160+
}
161+
147162
int encodeString(
148163
final ByteArrayBuffer dst,
149164
final CharSequence charSequence, final int off, final int len,
@@ -262,11 +277,7 @@ void encodeHeader(
262277
void encodeHeader(
263278
final ByteArrayBuffer dst, final String name, final String value, final boolean sensitive,
264279
final boolean noIndexing, final boolean useHuffman) throws CharacterCodingException {
265-
//send receiver the updated dynamic table size
266-
if (maxTableSize != this.dynamicTable.getMaxSize()) {
267-
encodeInt(dst, 5, maxTableSize, 0x20);
268-
this.dynamicTable.setMaxSize(maxTableSize);
269-
}
280+
encodeMaxTableSizeUpdates(dst);
270281

271282
final HPackRepresentation representation;
272283
if (sensitive) {
@@ -342,6 +353,13 @@ public int getMaxTableSize() {
342353
public void setMaxTableSize(final int maxTableSize) {
343354
Args.notNegative(maxTableSize, "Max table size");
344355
this.maxTableSize = maxTableSize;
356+
if (this.minTableSize == -1) {
357+
if (maxTableSize != this.dynamicTable.getMaxSize()) {
358+
this.minTableSize = maxTableSize;
359+
}
360+
} else if (maxTableSize < this.minTableSize) {
361+
this.minTableSize = maxTableSize;
362+
}
345363
}
346364

347-
}
365+
}
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* ====================================================================
3+
* Licensed to the Apache Software Foundation (ASF) under one
4+
* or more contributor license agreements. See the NOTICE file
5+
* distributed with this work for additional information
6+
* regarding copyright ownership. The ASF licenses this file
7+
* to you under the Apache License, Version 2.0 (the
8+
* "License"); you may not use this file except in compliance
9+
* with the License. You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing,
14+
* software distributed under the License is distributed on an
15+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
* KIND, either express or implied. See the License for the
17+
* specific language governing permissions and limitations
18+
* under the License.
19+
* ====================================================================
20+
*
21+
* This software consists of voluntary contributions made by many
22+
* individuals on behalf of the Apache Software Foundation. For more
23+
* information on the Apache Software Foundation, please see
24+
* <http://www.apache.org/>.
25+
*
26+
*/
27+
package org.apache.hc.core5.http2.hpack;
28+
29+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
30+
import static org.junit.jupiter.api.Assertions.assertEquals;
31+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
32+
33+
import java.nio.charset.StandardCharsets;
34+
import java.util.Arrays;
35+
36+
import org.apache.hc.core5.util.ByteArrayBuffer;
37+
import org.junit.jupiter.api.Test;
38+
39+
class TestHPackEncoder {
40+
41+
@Test
42+
void testMultipleTableSizeUpdatesSignalMinimumFirstThenFinal() throws Exception {
43+
final OutboundDynamicTable dynamicTable = new OutboundDynamicTable(4096);
44+
final HPackEncoder encoder = new HPackEncoder(dynamicTable, StandardCharsets.US_ASCII);
45+
final ByteArrayBuffer dst = new ByteArrayBuffer(128);
46+
47+
encoder.setMaxTableSize(1024);
48+
encoder.setMaxTableSize(2048);
49+
50+
// No local resize until the next header block is encoded.
51+
assertEquals(4096, dynamicTable.getMaxSize());
52+
assertEquals(2048, encoder.getMaxTableSize());
53+
54+
encoder.encodeHeader(dst, "x-test", "abc", false, false, false);
55+
56+
// First 6 bytes must be:
57+
// 1024 => 0x3f 0xe1 0x07
58+
// 2048 => 0x3f 0xe1 0x0f
59+
final byte[] actual = Arrays.copyOf(dst.array(), dst.length());
60+
assertArrayEquals(new byte[]{
61+
0x3f, (byte) 0xe1, 0x07,
62+
0x3f, (byte) 0xe1, 0x0f
63+
}, Arrays.copyOf(actual, 6));
64+
65+
// Local encoder state must also end at the final advertised size.
66+
assertEquals(2048, dynamicTable.getMaxSize());
67+
}
68+
69+
@Test
70+
void testShrinkThenRestoreOriginalStillEmitsTwoUpdates() throws Exception {
71+
final OutboundDynamicTable dynamicTable = new OutboundDynamicTable(4096);
72+
final HPackEncoder encoder = new HPackEncoder(dynamicTable, StandardCharsets.US_ASCII);
73+
final ByteArrayBuffer dst = new ByteArrayBuffer(128);
74+
75+
encoder.setMaxTableSize(1024);
76+
encoder.setMaxTableSize(4096);
77+
78+
encoder.encodeHeader(dst, "x-test", "abc", false, false, false);
79+
80+
final byte[] actual = Arrays.copyOf(dst.array(), dst.length());
81+
assertArrayEquals(new byte[]{
82+
0x3f, (byte) 0xe1, 0x07,
83+
0x3f, (byte) 0xe1, 0x1f
84+
}, Arrays.copyOf(actual, 6));
85+
86+
assertEquals(4096, dynamicTable.getMaxSize());
87+
}
88+
89+
@Test
90+
void testTableSizeUpdateZeroThenRestore() throws Exception {
91+
final OutboundDynamicTable dynamicTable = new OutboundDynamicTable(4096);
92+
final HPackEncoder encoder = new HPackEncoder(dynamicTable, StandardCharsets.US_ASCII);
93+
final ByteArrayBuffer dst = new ByteArrayBuffer(128);
94+
95+
encoder.setMaxTableSize(0);
96+
encoder.setMaxTableSize(4096);
97+
98+
encoder.encodeHeader(dst, "x-test", "abc", false, false, false);
99+
100+
final byte[] actual = Arrays.copyOf(dst.array(), dst.length());
101+
assertArrayEquals(new byte[]{
102+
0x20,
103+
0x3f, (byte) 0xe1, 0x1f
104+
}, Arrays.copyOf(actual, 4));
105+
106+
assertEquals(4096, dynamicTable.getMaxSize());
107+
}
108+
109+
@Test
110+
void testTableSizeUpdateIsNotRepeatedAcrossHeaderBlocks() throws Exception {
111+
final OutboundDynamicTable dynamicTable = new OutboundDynamicTable(4096);
112+
final HPackEncoder encoder = new HPackEncoder(dynamicTable, StandardCharsets.US_ASCII);
113+
114+
encoder.setMaxTableSize(1024);
115+
116+
final ByteArrayBuffer dst1 = new ByteArrayBuffer(128);
117+
encoder.encodeHeader(dst1, "x-a", "1", false, false, false);
118+
119+
final ByteArrayBuffer dst2 = new ByteArrayBuffer(128);
120+
encoder.encodeHeader(dst2, "x-b", "2", false, false, false);
121+
122+
final byte[] first = Arrays.copyOf(dst1.array(), dst1.length());
123+
assertArrayEquals(new byte[]{
124+
0x3f, (byte) 0xe1, 0x07
125+
}, Arrays.copyOf(first, 3));
126+
127+
final byte[] second = Arrays.copyOf(dst2.array(), dst2.length());
128+
assertNotEquals(0x20, second[0] & 0xe0);
129+
}
130+
131+
}

0 commit comments

Comments
 (0)