Skip to content

Commit

Permalink
Jetty 9.4.x #4890 do not index large fields (#4927)
Browse files Browse the repository at this point in the history 
* Issue #4890 Large HTTP2 Fields encoded

When choosing a strategy to encode a HTTP2 field, do not use indexed
if the field is larger than the dynamic table.

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #4890 Large HTTP2 Fields encoded

Fixed checkstyle in test

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #4890 Large HTTP2 Fields encoded

Only index 0 content-length

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #4890 Large HTTP2 Fields encoded

Fixed comments

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #4890 Large HTTP2 Fields encoded

Don't parse int

Signed-off-by: Greg Wilkins <[email protected]>
  • Loading branch information
@gregw
gregw authored Jun 1, 2020
1 parent ab0d5e7 commit 646010e
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 12 deletions.
23 changes: 11 additions & 12 deletions jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java
View file
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
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

0 comments on commit 646010e

Please sign in to comment.