diff --git a/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackContext.java b/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackContext.java index d45ce9fec50..c3e28897706 100644 --- a/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackContext.java +++ b/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackContext.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.hpack; +import java.nio.ByteBuffer; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -109,7 +110,7 @@ public class HpackContext Set added = new HashSet<>(); for (int i=1;i _fieldMap = new HashMap<>(); private final Map _nameMap = new HashMap<>(); @@ -183,7 +184,7 @@ public class HpackContext public Entry add(HttpField field) { int i=_headerTable.getNextIndexUnsafe(); - Entry entry=new Entry(i,field,false); + Entry entry=new Entry(i,field); int size = entry.getSize(); if (size>_maxHeaderTableSizeInBytes) return null; @@ -220,6 +221,17 @@ public class HpackContext return referenceSet; } + public void clearReferenceSet() + { + Entry entry = _refSet._refSetNext; + while(entry!=_refSet) + { + Entry next = entry._refSetNext; + entry.removeFromRefSet(); + entry=next; + } + } + public Iterator iterateReferenceSet() { return new Iterator() @@ -339,36 +351,32 @@ public class HpackContext public static class Entry { int _index; - final boolean _static; final HttpField _field; Entry _refSetNext=this; Entry _refSetPrev=this; boolean _refSetUsed; - Entry(boolean isStatic) + Entry() { - _static=isStatic; _index=0; _field=null; } - Entry(int index,String name, String value, boolean isStatic) + Entry(int index,String name, String value) { - _static=isStatic; _index=index; _field=new HttpField(name,value); } - Entry(int index, HttpField field, boolean isStatic) + Entry(int index, HttpField field) { - _static=isStatic; _index=index; _field=field; } private void addToRefSet(HpackContext ctx) { - if (_static) + if (isStatic()) throw new IllegalStateException("static"); if (_index<0) throw new IllegalStateException("evicted"); @@ -380,6 +388,11 @@ public class HpackContext ctx._refSet._refSetPrev._refSetNext=this; ctx._refSet._refSetPrev=this; } + + public boolean isInReferenceSet() + { + return _refSetNext!=this; + } public void removeFromRefSet() { @@ -397,25 +410,63 @@ public class HpackContext return 32+_field.getName().length()+_field.getValue().length(); } - /** - * @return - */ public HttpField getHttpField() { return _field; } - /** - * @return - */ public boolean isStatic() { - return _static; + return false; + } + + public byte[] getStaticHuffmanValue() + { + return null; } public String toString() { - return String.format("{%s,%d,%s,%x}",_static?"S":"D",_index,_field,hashCode()); + return String.format("{%s,%d,%s,%x}",isStatic()?"S":"D",_index,_field,hashCode()); + } + } + + public static class StaticEntry extends Entry + { + final byte[] _huffmanValue; + + StaticEntry(int index,String name, String value) + { + super(index,name,value); + if (value!=null && value.length()>0) + { + int huffmanLen = Huffman.octetsNeeded(value); + int lenLen = NBitInteger.octectsNeeded(7,huffmanLen); + _huffmanValue = new byte[1+lenLen+huffmanLen]; + ByteBuffer buffer = ByteBuffer.wrap(_huffmanValue); + + // Indicate Huffman + buffer.put((byte)0x80); + // Add huffman length + NBitInteger.encode(buffer,7,huffmanLen); + // Encode value + Huffman.encode(buffer,value); + + } + else + _huffmanValue=null; + } + + @Override + public boolean isStatic() + { + return true; + } + + @Override + public byte[] getStaticHuffmanValue() + { + return _huffmanValue; } } diff --git a/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackEncoder.java b/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackEncoder.java index 34edb32281e..a01a0af4e84 100644 --- a/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackEncoder.java +++ b/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/HpackEncoder.java @@ -19,20 +19,68 @@ package org.eclipse.jetty.hpack; +import java.nio.ByteBuffer; + +import org.eclipse.jetty.hpack.HpackContext.Entry; +import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.io.ByteBufferPool; public class HpackEncoder -{ - private final ByteBufferPool byteBufferPool; +{ + private final ByteBufferPool _byteBufferPool; + private final HpackContext _context; public HpackEncoder(ByteBufferPool byteBufferPool) { - this.byteBufferPool = byteBufferPool; + this(byteBufferPool,4096); + } + + public HpackEncoder(ByteBufferPool byteBufferPool,int maxHeaderTableSize) + { + this._byteBufferPool = byteBufferPool; + _context=new HpackContext(maxHeaderTableSize); } public ByteBufferPool.Lease encode(HttpFields fields) { - return new ByteBufferPool.Lease(byteBufferPool); + return new ByteBufferPool.Lease(_byteBufferPool); } + + + private boolean encode(ByteBuffer buffer, HttpField field) + { + // Is there an entry for the field? + Entry entry = _context.get(field); + + if (entry!=null) + { + // if entry is already in the reference set, then nothing more to do. + if (entry.isInReferenceSet()) + return true; + + // Is this as static field + if (entry.isStatic()) + { + // TODO Policy decision to make! + // Should we add to reference set or just always send as indexed? + // Let's always send as indexed to reduce churn in header table! + // BUGGER! Can't do that. Oh well all the static fields have small values, so + // lets send as literal header, indexed name. + // We don't need never indexed because the cookie fields are name only and we can + // huffman encode the value for the same reason. + + // Add the token + buffer.put((byte)0x00); + // Add the name index + NBitInteger.encode(buffer,4,_context.index(entry)); + // Add the value + buffer.put(entry.getStaticHuffmanValue()); + + } + } + } + + + } diff --git a/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/Huffman.java b/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/Huffman.java index e21fbd00e2f..eb3fd6ab9ae 100644 --- a/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/Huffman.java +++ b/jetty-hpack/src/main/java/org/eclipse/jetty/hpack/Huffman.java @@ -341,7 +341,7 @@ public class Huffman } - static public String decode(ByteBuffer buffer) throws IOException + static public String decode(ByteBuffer buffer) { StringBuilder out = new StringBuilder(buffer.remaining()*2); int node = 0; diff --git a/jetty-hpack/src/test/java/org/eclipse/jetty/hpack/HpackContextTest.java b/jetty-hpack/src/test/java/org/eclipse/jetty/hpack/HpackContextTest.java index b4e7db529c6..a61c215e3b0 100644 --- a/jetty-hpack/src/test/java/org/eclipse/jetty/hpack/HpackContextTest.java +++ b/jetty-hpack/src/test/java/org/eclipse/jetty/hpack/HpackContextTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.nio.ByteBuffer; import java.util.HashSet; import java.util.Iterator; import java.util.NoSuchElementException; @@ -397,6 +398,13 @@ public class HpackContextTest ctx.addToRefSet(ctx.get(3)); ctx.addToRefSet(ctx.get(1)); ctx.addToRefSet(ctx.get(5)); + + // check isInReferenceSet + assertTrue(ctx.get(1).isInReferenceSet()); + assertFalse(ctx.get(2).isInReferenceSet()); + assertTrue(ctx.get(3).isInReferenceSet()); + assertFalse(ctx.get(4).isInReferenceSet()); + assertTrue(ctx.get(5).isInReferenceSet()); // iterate ref set HashSet fields = new HashSet<>(); @@ -426,6 +434,13 @@ public class HpackContextTest assertTrue(fields.contains(field[0])); assertTrue(fields.contains(field[4])); + // check isInReferenceSet + assertTrue(ctx.get(1).isInReferenceSet()); + assertFalse(ctx.get(2).isInReferenceSet()); + assertFalse(ctx.get(3).isInReferenceSet()); + assertFalse(ctx.get(4).isInReferenceSet()); + assertTrue(ctx.get(5).isInReferenceSet()); + // iterator remove Iterator iter=ctx.iterateReferenceSet(); iter.next(); @@ -445,6 +460,55 @@ public class HpackContextTest } + @Test + public void testRefSetClear() + { + // Only enough space for 5 entries + HpackContext ctx = new HpackContext(38*5); + + HttpField[] field = + { + new HttpField("fo0","b0r"), + new HttpField("fo1","b1r"), + new HttpField("fo2","b2r"), + new HttpField("fo3","b3r"), + new HttpField("fo4","b4r"), + new HttpField("fo5","b5r"), + new HttpField("fo6","b6r"), + new HttpField("fo7","b7r"), + new HttpField("fo8","b8r"), + new HttpField("fo9","b9r"), + new HttpField("foA","bAr"), + }; + Entry[] entry = new Entry[field.length]; + + // Add 5 entries + for (int i=0;i<=4;i++) + entry[i]=ctx.add(field[i]); + + // Add 3 entries to reference set + ctx.clearReferenceSet(); + ctx.addToRefSet(ctx.get(3)); + ctx.addToRefSet(ctx.get(1)); + ctx.addToRefSet(ctx.get(5)); + + // iterate ref set + HashSet fields = new HashSet<>(); + for (Entry e: ctx.getReferenceSet() ) + fields.add(e.getHttpField()); + assertEquals(3,fields.size()); + assertTrue(fields.contains(field[0])); + assertTrue(fields.contains(field[2])); + assertTrue(fields.contains(field[4])); + + // Clear set + ctx.clearReferenceSet(); + fields.clear(); + for (Entry e: ctx.getReferenceSet() ) + fields.add(e.getHttpField()); + assertEquals(0,fields.size()); + } + @Test public void testResize() { @@ -598,8 +662,28 @@ public class HpackContextTest assertFalse(fields.contains(field[0])); assertFalse(fields.contains(field[2])); assertTrue(fields.contains(field[4])); - - - + } + + @Test + public void testStaticHuffmanValues() + { + HpackContext ctx = new HpackContext(4096); + for (int i=2;i<=14;i++) + { + Entry entry=ctx.get(i); + assertTrue(entry.isStatic()); + + ByteBuffer buffer = ByteBuffer.wrap(entry.getStaticHuffmanValue()); + int huff = 0xff&buffer.get(); + assertTrue((0x80&huff)==0x80); + + int len = NBitInteger.decode(buffer,7); + + assertEquals(len,buffer.remaining()); + String value = Huffman.decode(buffer); + + assertEquals(entry.getHttpField().getValue(),value); + + } } }