Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #4890 - Do not index large HTTP2 Fields #4927

Merged
merged 5 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -299,11 +299,11 @@ public void encode(ByteBuffer buffer, HttpField field)

String encoding = null;

// Is there an entry for the field?
// Is there an index entry for the field?
Entry entry = _context.get(field);
if (entry != null)
{
// Known field entry, so encode it as indexed
// This is a known indexed field, send as static or dynamic indexed.
if (entry.isStatic())
{
buffer.put(((StaticEntry)entry).getEncodedField());
Expand All @@ -321,10 +321,10 @@ public void encode(ByteBuffer buffer, HttpField field)
}
else
{
// Unknown field entry, so we will have to send literally.
// Unknown field entry, so we will have to send literally, but perhaps add an index.
final boolean indexed;

// But do we know it's name?
// Do we know it's name?
gregw marked this conversation as resolved.
Show resolved Hide resolved
HttpHeader header = field.getHeader();

// Select encoding strategy
Expand All @@ -342,12 +342,12 @@ public void encode(ByteBuffer buffer, HttpField field)
if (_debug)
encoding = indexed ? "PreEncodedIdx" : "PreEncoded";
}
// has the custom header name been seen before?
else if (name == null)
// has the custom header name been seen before and will it fit in dynamic table?
gregw marked this conversation as resolved.
Show resolved Hide resolved
else if (name == null && fieldSize < _context.getMaxDynamicTableSize())
{
// unknown name and value, so let's index this just in case it is
// the first time we have seen a custom name or a custom field.
// unless the name is changing, this is worthwhile
// unknown name and value that will fit in dynamic table, so let's index
// this just in case it is the first time we have seen a custom name or a
// custom field. Unless the name is once only, this is worthwhile
indexed = true;
encodeName(buffer, (byte)0x40, 6, field.getName(), null);
encodeValue(buffer, true, field.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,32 @@ public void testUnknownFieldsContextManagement() throws Exception
assertEquals(5, encoder.getHpackContext().size());
}

@Test
public void testLargeFieldsNotIndexed()
{
HpackEncoder encoder = new HpackEncoder(38 * 5);
HpackContext ctx = encoder.getHpackContext();

ByteBuffer buffer = BufferUtil.allocate(4096);

// Index little fields
int pos = BufferUtil.flipToFill(buffer);
encoder.encode(buffer, new HttpField("Name", "Value"));
BufferUtil.flipToFlush(buffer, pos);
int dynamicTableSize = ctx.getDynamicTableSize();
assertThat(dynamicTableSize, Matchers.greaterThan(0));

// Do not index big field
StringBuilder largeName = new StringBuilder("largeName-");
String filler = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX";
while (largeName.length() < ctx.getMaxDynamicTableSize())
largeName.append(filler, 0, Math.min(filler.length(), ctx.getMaxDynamicTableSize() - largeName.length()));
pos = BufferUtil.flipToFill(buffer);
encoder.encode(buffer, new HttpField(largeName.toString(), "Value"));
BufferUtil.flipToFlush(buffer, pos);
assertThat(ctx.getDynamicTableSize(), Matchers.is(dynamicTableSize));
}

@Test
public void testNeverIndexSetCookie() throws Exception
{
Expand Down