fixed hpack literal encoding bug

This commit is contained in:
Greg Wilkins 2014-06-10 12:55:21 +02:00
parent ad034f4d54
commit ab5461d73e
6 changed files with 171 additions and 9 deletions

View File

@ -30,6 +30,7 @@ import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.util.ArrayQueue;
import org.eclipse.jetty.util.ArrayTernaryTrie;
import org.eclipse.jetty.util.ArrayTrie;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.Trie;
@ -104,7 +105,7 @@ public class HpackContext
};
private static final Map<HttpField,Entry> __staticFieldMap = new HashMap<>();
private static final Trie<Entry> __staticNameMap = new ArrayTrie<>(24);
private static final Trie<Entry> __staticNameMap = new ArrayTernaryTrie<>(true,512);
private static final Entry[] __staticTable=new Entry[STATIC_TABLE.length];
static
@ -140,15 +141,18 @@ public class HpackContext
default:
entry=new StaticEntry(i,new HttpField(STATIC_TABLE[i][0],STATIC_TABLE[i][1]));
}
__staticTable[i]=entry;
if (entry._field.getValue()!=null)
__staticFieldMap.put(entry._field,entry);
if (!added.contains(entry._field.getName()))
{
added.add(entry._field.getName());
__staticNameMap.put(entry._field.getName(),entry);
if (__staticNameMap.get(entry._field.getName())==null)
throw new IllegalStateException("name trie too small");
}
}
}

View File

@ -27,6 +27,8 @@ import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http2.hpack.HpackContext.Entry;
import org.eclipse.jetty.io.ByteBufferPool.Lease;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.TypeUtil;
public class HpackEncoder
{
@ -130,6 +132,13 @@ public class HpackEncoder
private void encode(ByteBuffer buffer, HttpField field)
{
/*
System.err.println("encode "+field);
int p=buffer.position();
try{
*/
// TODO currently we do not check if there is enough space, so we will always
// return true or fail nastily.
@ -142,6 +151,7 @@ public class HpackEncoder
if (entry.isInReferenceSet())
{
entry.used();
// System.err.println("In Reference Set");
return;
}
@ -163,8 +173,11 @@ public class HpackEncoder
// Add the value
buffer.put(entry.getStaticHuffmanValue());
// System.err.println("Literal without Indexing, indexed name");
return;
}
// System.err.println("Indexed from Header Set");
// So we can add the entry to the reference Set and emit the index;
_context.addToRefSet(entry);
@ -200,7 +213,7 @@ public class HpackEncoder
{
reference=true;
never_index=false;
huffman=__DO_NOT_HUFFMAN.contains(header);
huffman=!__DO_NOT_HUFFMAN.contains(header);
name_bits = 6;
mask=(byte)0x40;
}
@ -208,17 +221,19 @@ public class HpackEncoder
{
reference=false;
never_index=__NEVER_INDEX.contains(header);
huffman=__DO_NOT_HUFFMAN.contains(header);
huffman=!__DO_NOT_HUFFMAN.contains(header);
name_bits = 4;
mask=never_index?(byte)0x01:(byte)0x00;
}
// Add the mask bits
buffer.put(mask);
// Look for a name Index
Entry name_entry = _context.get(field.getName());
// System.err.printf("Literal huff=%b refed=%b neverIdx=%b nameIdx=%b b=%d m=0x%02x%n",huffman,reference,never_index,name_entry!=null,name_bits,mask);
if (name_entry!=null)
NBitInteger.encode(buffer,name_bits,_context.index(name_entry));
else
@ -241,7 +256,7 @@ public class HpackEncoder
else
{
// add literal assuming iso_8859_1
buffer.put((byte)0x80);
buffer.put((byte)0x00);
NBitInteger.encode(buffer,7,value.length());
for (int i=0;i<value.length();i++)
{
@ -262,6 +277,15 @@ public class HpackEncoder
}
return;
/*
}
finally
{
int e=buffer.position();
System.err.println("'"+TypeUtil.toHexString(buffer.array(),buffer.arrayOffset()+p,e-p)+"'");
}
*/
}

View File

@ -20,6 +20,7 @@
package org.eclipse.jetty.http2.hpack;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@ -67,6 +68,37 @@ public class MetaData implements Iterable<HttpField>
return list;
}
@Override
public boolean equals(Object o)
{
if (!(o instanceof MetaData))
return false;
MetaData m = (MetaData)o;
List<HttpField> lm=m.getFields();
int s=0;
for (HttpField field: this)
{
s++;
if (!lm.contains(field))
return false;
}
if (s!=lm.size())
return false;
return true;
}
@Override
public String toString()
{
StringBuilder out = new StringBuilder();
for (HttpField field: this)
out.append(field).append('\n');
return out.toString();
}
/* -------------------------------------------------------- */
/* -------------------------------------------------------- */
@ -132,6 +164,27 @@ public class MetaData implements Iterable<HttpField>
{
return _path;
}
@Override
public boolean equals(Object o)
{
if (!(o instanceof Request))
return false;
Request r = (Request)o;
if (!_method.equals(r._method) ||
!_scheme.equals(r._scheme) ||
!_authority.equals(r._authority) ||
!_path.equals(r._path))
return false;
return super.equals(o);
}
@Override
public String toString()
{
return _method+" "+_scheme+"://"+_authority+_path+" HTTP/2\n"+super.toString();
}
}
/* -------------------------------------------------------- */
@ -169,5 +222,21 @@ public class MetaData implements Iterable<HttpField>
{
return _status;
}
@Override
public boolean equals(Object o)
{
if (!(o instanceof Response))
return false;
Response r = (Response)o;
if (_status!=r._status)
return false;
return super.equals(o);
}
@Override
public String toString()
{
return "HTTP/2 "+_status+"\n"+super.toString();
}
}
}

View File

@ -686,4 +686,19 @@ public class HpackContextTest
}
}
@Test
public void testNameInsensitivity()
{
HpackContext ctx = new HpackContext(4096);
assertEquals("content-length",ctx.get("content-length").getHttpField().getName());
assertEquals("content-length",ctx.get("Content-Length").getHttpField().getName());
ctx.add(new HttpField("Wibble","Wobble"));
assertEquals("Wibble",ctx.get("wibble").getHttpField().getName());
assertEquals("Wibble",ctx.get("Wibble").getHttpField().getName());
}
}

View File

@ -267,7 +267,6 @@ public class HpackEncoderTest
}

View File

@ -19,16 +19,67 @@
package org.eclipse.jetty.http2.hpack;
import java.nio.ByteBuffer;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.junit.Assert;
import org.junit.Test;
import org.eclipse.jetty.http2.hpack.MetaData.Response;
import org.eclipse.jetty.http2.hpack.MetaData.Request;
import org.eclipse.jetty.util.BufferUtil;
public class HpackTest
{
@Test
public void encodeDecodeTest()
public void encodeDecodeResponseTest()
{
HttpFields fields = new HttpFields();
HpackEncoder encoder = new HpackEncoder();
HpackDecoder decoder = new HpackDecoder();
ByteBuffer buffer = BufferUtil.allocate(16*1024);
HttpFields fields0 = new HttpFields();
fields0.add(HttpHeader.CONTENT_TYPE,"text/html");
fields0.add(HttpHeader.CONTENT_LENGTH,"1024");
fields0.add(HttpHeader.SERVER,"jetty");
fields0.add(HttpHeader.SET_COOKIE,"abcdefghijklmnopqrstuvwxyz");
fields0.add("custom-key","custom-value");
Response original0 = new Response(200,fields0);
BufferUtil.clearToFill(buffer);
encoder.encode(buffer,original0);
BufferUtil.flipToFlush(buffer,0);
Response decoded0 = (Response)decoder.decode(buffer);
System.err.println(decoded0);
Assert.assertEquals(original0,decoded0);
// Same again?
BufferUtil.clearToFill(buffer);
encoder.encode(buffer,original0);
BufferUtil.flipToFlush(buffer,0);
Response decoded0b = (Response)decoder.decode(buffer);
System.err.println(decoded0b);
Assert.assertEquals(original0,decoded0b);
HttpFields fields1 = new HttpFields();
fields1.add(HttpHeader.CONTENT_TYPE,"text/plain");
fields1.add(HttpHeader.CONTENT_LENGTH,"1234");
fields1.add(HttpHeader.SERVER,"jetty");
fields1.add("custom-key","other-value");
Response original1 = new Response(200,fields1);
// Same again?
BufferUtil.clearToFill(buffer);
encoder.encode(buffer,original1);
BufferUtil.flipToFlush(buffer,0);
Response decoded1 = (Response)decoder.decode(buffer);
System.err.println(decoded1);
Assert.assertEquals(original1,decoded1);
}