From 45ead1bb29394871010bf70977426cf319a529a5 Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@webtide.com>
Date: Fri, 15 Jul 2016 13:46:08 +1000
Subject: [PATCH] jetty http2 client parse error #726

---
 .../org/eclipse/jetty/http/HttpField.java     |  7 +-
 .../org/eclipse/jetty/http/HttpFields.java    |  2 +-
 .../jetty/http2/hpack/HpackContext.java       | 96 +++++++++----------
 .../jetty/http2/hpack/HpackDecoder.java       |  5 +-
 .../jetty/http2/hpack/HpackEncoder.java       |  3 +
 .../jetty/http2/hpack/HpackDecoderTest.java   | 28 ++++++
 .../eclipse/jetty/http2/hpack/HpackTest.java  |  8 +-
 7 files changed, 92 insertions(+), 57 deletions(-)

diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java
index 584ed74269d..ab9c142dac6 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java
@@ -206,6 +206,8 @@ public class HttpField
             return false;
         if (_value==null)
             return false;
+        if (search.equals(_value))
+            return true;
         
         search = StringUtil.asciiToLowerCase(search);
 
@@ -410,9 +412,10 @@ public class HttpField
     @Override
     public int hashCode()
     {
+        int vhc = _value==null?0:_value.hashCode();
         if (_header==null)
-            return _value.hashCode() ^ nameHashCode();
-        return _value.hashCode() ^ _header.hashCode();
+            return vhc ^ nameHashCode();
+        return vhc ^ _header.hashCode();
     }
 
     @Override
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java
index 0951a6de515..283cfc5f45c 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java
@@ -161,7 +161,7 @@ public class HttpFields implements Iterable<HttpField>
         for (int i=_size;i-->0;)
         {
             HttpField f=_fields[i];
-            if (f.isSameName(field) && f.contains(field.getValue()))
+            if (f.isSameName(field) && (f.equals(field)||f.contains(field.getValue())))
                 return true;
         }
         return false;
diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java
index 89fbf00e4ad..2d2b30e2702 100644
--- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java
+++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java
@@ -47,11 +47,11 @@ import org.eclipse.jetty.util.log.Logger;
 public class HpackContext
 {
     public static final Logger LOG = Log.getLogger(HpackContext.class);
-    
+    private static final String EMPTY = "";
     public static final String[][] STATIC_TABLE = 
     {
         {null,null},
-        /* 1  */ {":authority",null},
+        /* 1  */ {":authority",EMPTY},
         /* 2  */ {":method","GET"},
         /* 3  */ {":method","POST"},
         /* 4  */ {":path","/"},
@@ -65,53 +65,53 @@ public class HpackContext
         /* 12 */ {":status","400"},
         /* 13 */ {":status","404"},
         /* 14 */ {":status","500"},
-        /* 15 */ {"accept-charset",null},
+        /* 15 */ {"accept-charset",EMPTY},
         /* 16 */ {"accept-encoding","gzip, deflate"},
-        /* 17 */ {"accept-language",null},
-        /* 18 */ {"accept-ranges",null},
-        /* 19 */ {"accept",null},
-        /* 20 */ {"access-control-allow-origin",null},
-        /* 21 */ {"age",null},
-        /* 22 */ {"allow",null},
-        /* 23 */ {"authorization",null},
-        /* 24 */ {"cache-control",null},
-        /* 25 */ {"content-disposition",null},
-        /* 26 */ {"content-encoding",null},
-        /* 27 */ {"content-language",null},
-        /* 28 */ {"content-length",null},
-        /* 29 */ {"content-location",null},
-        /* 30 */ {"content-range",null},
-        /* 31 */ {"content-type",null},
-        /* 32 */ {"cookie",null},
-        /* 33 */ {"date",null},
-        /* 34 */ {"etag",null},
-        /* 35 */ {"expect",null},
-        /* 36 */ {"expires",null},
-        /* 37 */ {"from",null},
-        /* 38 */ {"host",null},
-        /* 39 */ {"if-match",null},
-        /* 40 */ {"if-modified-since",null},
-        /* 41 */ {"if-none-match",null},
-        /* 42 */ {"if-range",null},
-        /* 43 */ {"if-unmodified-since",null},
-        /* 44 */ {"last-modified",null},
-        /* 45 */ {"link",null},
-        /* 46 */ {"location",null},
-        /* 47 */ {"max-forwards",null},
-        /* 48 */ {"proxy-authenticate",null},
-        /* 49 */ {"proxy-authorization",null},
-        /* 50 */ {"range",null},
-        /* 51 */ {"referer",null},
-        /* 52 */ {"refresh",null},
-        /* 53 */ {"retry-after",null},
-        /* 54 */ {"server",null},
-        /* 55 */ {"set-cookie",null},
-        /* 56 */ {"strict-transport-security",null},
-        /* 57 */ {"transfer-encoding",null},
-        /* 58 */ {"user-agent",null},
-        /* 59 */ {"vary",null},
-        /* 60 */ {"via",null},
-        /* 61 */ {"www-authenticate",null},
+        /* 17 */ {"accept-language",EMPTY},
+        /* 18 */ {"accept-ranges",EMPTY},
+        /* 19 */ {"accept",EMPTY},
+        /* 20 */ {"access-control-allow-origin",EMPTY},
+        /* 21 */ {"age",EMPTY},
+        /* 22 */ {"allow",EMPTY},
+        /* 23 */ {"authorization",EMPTY},
+        /* 24 */ {"cache-control",EMPTY},
+        /* 25 */ {"content-disposition",EMPTY},
+        /* 26 */ {"content-encoding",EMPTY},
+        /* 27 */ {"content-language",EMPTY},
+        /* 28 */ {"content-length",EMPTY},
+        /* 29 */ {"content-location",EMPTY},
+        /* 30 */ {"content-range",EMPTY},
+        /* 31 */ {"content-type",EMPTY},
+        /* 32 */ {"cookie",EMPTY},
+        /* 33 */ {"date",EMPTY},
+        /* 34 */ {"etag",EMPTY},
+        /* 35 */ {"expect",EMPTY},
+        /* 36 */ {"expires",EMPTY},
+        /* 37 */ {"from",EMPTY},
+        /* 38 */ {"host",EMPTY},
+        /* 39 */ {"if-match",EMPTY},
+        /* 40 */ {"if-modified-since",EMPTY},
+        /* 41 */ {"if-none-match",EMPTY},
+        /* 42 */ {"if-range",EMPTY},
+        /* 43 */ {"if-unmodified-since",EMPTY},
+        /* 44 */ {"last-modified",EMPTY},
+        /* 45 */ {"link",EMPTY},
+        /* 46 */ {"location",EMPTY},
+        /* 47 */ {"max-forwards",EMPTY},
+        /* 48 */ {"proxy-authenticate",EMPTY},
+        /* 49 */ {"proxy-authorization",EMPTY},
+        /* 50 */ {"range",EMPTY},
+        /* 51 */ {"referer",EMPTY},
+        /* 52 */ {"refresh",EMPTY},
+        /* 53 */ {"retry-after",EMPTY},
+        /* 54 */ {"server",EMPTY},
+        /* 55 */ {"set-cookie",EMPTY},
+        /* 56 */ {"strict-transport-security",EMPTY},
+        /* 57 */ {"transfer-encoding",EMPTY},
+        /* 58 */ {"user-agent",EMPTY},
+        /* 59 */ {"vary",EMPTY},
+        /* 60 */ {"via",EMPTY},
+        /* 61 */ {"www-authenticate",EMPTY},
     };
     
     private static final Map<HttpField,Entry> __staticFieldMap = new HashMap<>();
diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java
index 38dc8567f1f..c0564daa9ca 100644
--- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java
+++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java
@@ -78,10 +78,9 @@ public class HpackDecoder
 
         while(buffer.hasRemaining())
         {
-            if (LOG.isDebugEnabled())
+            if (LOG.isDebugEnabled() && buffer.hasArray())
             {
-                int l=Math.min(buffer.remaining(),16);
-                // TODO: not guaranteed the buffer has a backing array !
+                int l=Math.min(buffer.remaining(),32);
                 LOG.debug("decode {}{}",
                         TypeUtil.toHexString(buffer.array(),buffer.arrayOffset()+buffer.position(),l),
                         l<buffer.remaining()?"...":"");
diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java
index e42ffb86b54..098cec41e20 100644
--- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java
+++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java
@@ -175,6 +175,9 @@ public class HpackEncoder
 
     public void encode(ByteBuffer buffer, HttpField field)
     {
+        if (field.getValue()==null)
+            field = new HttpField(field.getHeader(),field.getName(),"");
+        
         final int p=_debug?buffer.position():-1;
 
         String encoding=null;
diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java
index 8b40fd7acf9..6cb86619f0e 100644
--- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java
+++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java
@@ -23,14 +23,22 @@ import java.nio.ByteBuffer;
 import java.util.Iterator;
 
 import org.eclipse.jetty.http.HttpField;
+import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpScheme;
+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.eclipse.jetty.util.log.Log;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
 import org.junit.Test;
 
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 public class HpackDecoderTest
@@ -159,4 +167,24 @@ public class HpackDecoderTest
         assertEquals("www.example.com",request.getURI().getHost());
         assertFalse(request.iterator().hasNext());
     }
+    
+    @Test
+    public void testNghttpx()
+    {        
+        // Response encoded by nghttpx
+        String encoded="886196C361Be940b6a65B6850400B8A00571972e080a62D1Bf5f87497cA589D34d1f9a0f0d0234327690Aa69D29aFcA954D3A5358980Ae112e0f7c880aE152A9A74a6bF3";
+        ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
+
+        HpackDecoder decoder = new HpackDecoder(4096,8192);
+        MetaData.Response response = (MetaData.Response)decoder.decode(buffer);
+
+        assertThat(response.getStatus(),is(200));
+        assertThat(response.getFields().size(),is(6));
+        assertTrue(response.getFields().contains(new HttpField(HttpHeader.DATE,"Fri, 15 Jul 2016 02:36:20 GMT")));
+        assertTrue(response.getFields().contains(new HttpField(HttpHeader.CONTENT_TYPE,"text/html")));
+        assertTrue(response.getFields().contains(new HttpField(HttpHeader.CONTENT_ENCODING,"")));
+        assertTrue(response.getFields().contains(new HttpField(HttpHeader.CONTENT_LENGTH,"42")));
+        assertTrue(response.getFields().contains(new HttpField(HttpHeader.SERVER,"nghttpx nghttp2/1.12.0")));
+        assertTrue(response.getFields().contains(new HttpField(HttpHeader.VIA,"1.1 nghttpx")));
+    }
 }
diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java
index c1b1d0d5b2f..abd8ab7472f 100644
--- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java
+++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java
@@ -54,6 +54,7 @@ public class HpackTest
         HttpFields fields0 = new HttpFields();
         fields0.add(HttpHeader.CONTENT_TYPE,"text/html");
         fields0.add(HttpHeader.CONTENT_LENGTH,"1024");
+        fields0.add(new HttpField(HttpHeader.CONTENT_ENCODING,(String)null));
         fields0.add(ServerJetty);
         fields0.add(XPowerJetty);
         fields0.add(Date);
@@ -65,7 +66,7 @@ public class HpackTest
         encoder.encode(buffer,original0);
         BufferUtil.flipToFlush(buffer,0);
         Response decoded0 = (Response)decoder.decode(buffer);
-
+        original0.getFields().put(new HttpField(HttpHeader.CONTENT_ENCODING,""));
         assertMetadataSame(original0,decoded0);
         
         // Same again?
@@ -79,9 +80,10 @@ public class HpackTest
         HttpFields fields1 = new HttpFields();
         fields1.add(HttpHeader.CONTENT_TYPE,"text/plain");
         fields1.add(HttpHeader.CONTENT_LENGTH,"1234");
+        fields1.add(HttpHeader.CONTENT_ENCODING," ");
         fields1.add(ServerJetty);
-        fields0.add(XPowerJetty);
-        fields0.add(Date);
+        fields1.add(XPowerJetty);
+        fields1.add(Date);
         fields1.add("Custom-Key","Other-Value");
         Response original1 = new MetaData.Response(HttpVersion.HTTP_2,200,fields1);