Issue #2571 HPACK table overflow (#2589)

When an entry that is too large for the dynamic table is added, the entire table should be evicted as per the RFC.  Previously we were throwing a 431 Bad Message.   This will require that the OP should retest #1134.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2018-05-30 09:22:00 +02:00 committed by GitHub
parent 154298af64
commit 4d09806d4e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 41 additions and 27 deletions

View File

@ -19,6 +19,7 @@
package org.eclipse.jetty.http2.hpack;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@ -254,6 +255,7 @@ public class HpackContext
{
if (LOG.isDebugEnabled())
LOG.debug(String.format("HdrTbl[%x] !added size %d>%d",hashCode(),size,_maxDynamicTableSizeInBytes));
_dynamicTable.evictAll();
return null;
}
_dynamicTableSizeInBytes+=size;
@ -390,7 +392,18 @@ public class HpackContext
if (LOG.isDebugEnabled())
LOG.debug(String.format("HdrTbl[%x] entries=%d, size=%d, max=%d",HpackContext.this.hashCode(),_dynamicTable.size(),_dynamicTableSizeInBytes,_maxDynamicTableSizeInBytes));
}
private void evictAll()
{
if (LOG.isDebugEnabled())
LOG.debug(String.format("HdrTbl[%x] evictAll",HpackContext.this.hashCode()));
_fieldMap.clear();
_nameMap.clear();
_offset = 0;
_size = 0;
_dynamicTableSizeInBytes = 0;
Arrays.fill(_entries,null);
}
}
public static class Entry

View File

@ -242,14 +242,9 @@ public class HpackDecoder
// emit the field
_builder.emit(field);
// if indexed
// if indexed add to dynamic table
if (indexed)
{
// add to dynamic table
if (_context.add(field)==null)
throw new BadMessageException(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431,"Indexed field value too large");
}
_context.add(field);
}
}

View File

@ -329,11 +329,9 @@ public class HpackEncoder
}
}
// If we want the field referenced, then we add it to our
// table and reference set.
// If we want the field referenced, then we add it to our table and reference set.
if (indexed)
if (_context.add(field)==null)
throw new IllegalStateException();
_context.add(field);
}
if (_debug)

View File

@ -166,6 +166,11 @@ public class MetaDataBuilder
return new MetaData.Request(_method,_scheme,_authority,_path,HttpVersion.HTTP_2,fields,_contentLength);
if (_status!=0)
return new MetaData.Response(HttpVersion.HTTP_2,_status,fields,_contentLength);
if (_path!=null)
fields.put(HttpHeader.C_PATH,_path);
if (_authority!=null)
fields.put(HttpHeader.HOST,_authority.getValue());
return new MetaData(HttpVersion.HTTP_2,fields,_contentLength);
}
finally

View File

@ -190,7 +190,19 @@ public class HpackDecoderTest
assertTrue(response.getFields().contains(new HttpField(HttpHeader.SERVER,"nghttpx nghttp2/1.12.0")));
assertTrue(response.getFields().contains(new HttpField(HttpHeader.VIA,"1.1 nghttpx")));
}
@Test
public void testResize() throws Exception
{
String encoded = "3f6166871e33A13a47497f205f8841E92b043d492d49";
ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
HpackDecoder decoder = new HpackDecoder(4096, 8192);
MetaData metaData = decoder.decode(buffer);
assertThat(metaData.getFields().get(HttpHeader.HOST),is("aHostName"));
assertThat(metaData.getFields().get(HttpHeader.CONTENT_TYPE),is("some/content"));
assertThat(decoder.getHpackContext().getDynamicTableSize(),is(0));
}
@Test
public void testTooBigToIndex()
{
@ -198,16 +210,10 @@ public class HpackDecoderTest
ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
HpackDecoder decoder = new HpackDecoder(128,8192);
try
{
decoder.decode(buffer);
Assert.fail();
}
catch (BadMessageException e)
{
assertThat(e.getCode(),equalTo(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431));
assertThat(e.getReason(),Matchers.startsWith("Indexed field value too large"));
}
MetaData metaData = decoder.decode(buffer);
assertThat(decoder.getHpackContext().getDynamicTableSize(),is(0));
assertThat(metaData.getFields().get(HttpHeader.C_PATH),Matchers.startsWith("This is a very large field"));
}
@Test

View File

@ -27,10 +27,10 @@ import java.nio.ByteBuffer;
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;
import org.eclipse.jetty.util.TypeUtil;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Test;
@ -249,7 +249,4 @@ public class HpackEncoderTest
context.get(HpackContext.STATIC_SIZE+1).getSize()+context.get(HpackContext.STATIC_SIZE+2).getSize()));
}
}