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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd04894
Issue #4890 Large HTTP2 Fields encoded
gregw Jun 1, 2020
2667974
Issue #4890 Large HTTP2 Fields encoded
gregw Jun 1, 2020
916817c
Issue #4890 Large HTTP2 Fields encoded
gregw Jun 1, 2020
8c9f539
Issue #4890 Large HTTP2 Fields encoded
gregw Jun 1, 2020
1590a9a
Issue #4890 Large HTTP2 Fields encoded
gregw Jun 1, 2020
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
23 changes: 11 additions & 12 deletions jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java
View file
Open in desktop
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 its name?
HttpHeader header = field.getHeader();

// Select encoding strategy
Expand All @@ -342,12 +342,11 @@ 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)
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 All @@ -356,7 +355,7 @@ else if (name == null)
}
else
{
// known custom name, but unknown value.
// Known name, but different value.
// This is probably a custom field with changing value, so don't index.
indexed = false;
encodeName(buffer, (byte)0x00, 4, field.getName(), null);
Expand Down Expand Up @@ -395,9 +394,9 @@ else if (DO_NOT_INDEX.contains(header))
(huffman ? "HuffV" : "LitV") +
(neverIndex ? "!!Idx" : "!Idx");
}
else if (fieldSize >= _context.getMaxDynamicTableSize() || header == HttpHeader.CONTENT_LENGTH && field.getValue().length() > 2)
else if (fieldSize >= _context.getMaxDynamicTableSize() || header == HttpHeader.CONTENT_LENGTH && !"0".equals(field.getValue()))
{
// Non indexed if field too large or a content length for 3 digits or more
// The field is too large or a non zero content length, so do not index.
indexed = false;
encodeName(buffer, (byte)0x00, 4, header.asString(), name);
encodeValue(buffer, true, field.getValue());
Expand Down
49 changes: 49 additions & 0 deletions jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.util.BufferUtil;
Expand Down Expand Up @@ -145,6 +146,54 @@ 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 testIndexContentLength()
{
HpackEncoder encoder = new HpackEncoder(38 * 5);
HpackContext ctx = encoder.getHpackContext();

ByteBuffer buffer = BufferUtil.allocate(4096);

// Index zero content length
int pos = BufferUtil.flipToFill(buffer);
encoder.encode(buffer, new HttpField(HttpHeader.CONTENT_LENGTH, "0"));
BufferUtil.flipToFlush(buffer, pos);
int dynamicTableSize = ctx.getDynamicTableSize();
assertThat(dynamicTableSize, Matchers.greaterThan(0));

// Do not index non zero content length
pos = BufferUtil.flipToFill(buffer);
encoder.encode(buffer, new HttpField(HttpHeader.CONTENT_LENGTH, "42"));
BufferUtil.flipToFlush(buffer, pos);
assertThat(ctx.getDynamicTableSize(), Matchers.is(dynamicTableSize));
}

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