HPACK: do not allow header block to end with table size update

CHANGELOG

@@ -1,3 +1,7 @@
+2018-04-27
+
+    - HPACK: do not allow header block to end with table size update.
+
 2018-04-25
 
     - [BUGFIX] Do not create gap in sent packnos when squeezing delayed

src/liblsquic/lsquic_frame_reader.c

@@ -938,19 +938,15 @@ decode_and_pass_payload (struct lsquic_frame_reader *fr)
         s = lsquic_hdec_decode(fr->fr_hdec, &comp, end,
                                 hwc.buf, hwc.buf + 16 * 1024,
                                 &hwc.name_len, &hwc.val_len);
-        if (s > 0)
+        if (s == 0)
         {
             err = add_header_to_uh(fr, &hwc);
-            if (0 != err)
+            if (err == 0)
-                goto stream_error;
+                continue;
-        }
-        else if (s < 0)
-        {
-            err = FR_ERR_DECOMPRESS;
-            goto stream_error;
         }
         else
-            break;
+            err = FR_ERR_DECOMPRESS;
+        goto stream_error;
     }
     assert(comp == end);
 

src/liblsquic/lsquic_hpack_dec.c

@@ -290,7 +290,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec,
     int indexed_type, len;
 
     if ((*src) == src_end)
-        return 0;
+        return -1;
 
     while ((*(*src) & 0xe0) == 0x20)    //001 xxxxx
     {
@@ -300,7 +300,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec,
             return -1;
         hdec_update_max_capacity(dec, new_capacity);
         if (*src == src_end)
-            return 0;
+            return -1;
     }
 
     /* lsquic_hdec_dec_int() sets `index' and advances `src'.  If we do not call
@@ -373,7 +373,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec,
                     return -1;
                 *val_len = lsquic_hpack_stx_tab[index - 1].val_len;
                 memcpy(name + *name_len, lsquic_hpack_stx_tab[index - 1].val, *val_len);
-                return 1;
+                return 0;
             }
         }
         else
@@ -392,7 +392,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec,
                     return -1;
                 *val_len = entry->dte_val_len;
                 memcpy(name + *name_len, DTE_VALUE(entry), *val_len);
-                return 1;
+                return 0;
             }
         }
     }
@@ -421,7 +421,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec,
             return -1;  //error
     }
 
-    return 1;
+    return 0;
 }
 
 

src/liblsquic/lsquic_hpack_dec.h

@@ -22,16 +22,11 @@ lsquic_hdec_init (struct lsquic_hdec *);
 void
 lsquic_hdec_cleanup (struct lsquic_hdec *);
 
-/** @lsquic_hdecode
+/*
- * @brief HPACK decode one name/value item
+ * Returns 0 on success, a negative value on failure.
- *  @param[in,out] dec - A pointer to a valid HPACK API struct
+ *
- *  @param[in,out] src - Address of pointer to source buffer
+ * If 0 is returned, `src' is advanced.  Calling with a zero-length input
- *  @param[in] src_end - A pointer to end of source buffer
+ * buffer results in an error.
- *  @param[out] dst - A pointer to destination buffer
- *  @param[out] dst_end - A pointer to end of destination buffer
- *  @param[out] name_len - The item name's length
- *  @param[out] value_len - The item value's length
- *  @return 1: OK, 0: end, -1: FAIL
  */
 int
 lsquic_hdec_decode (struct lsquic_hdec *dec,

test/unittests/test_hpack.c

@@ -344,9 +344,11 @@ test_hpack_test_RFC_Example (void)
     char out[2048];
     uint16_t name_len = 1024;
     uint16_t val_len = 1024;
-    while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len,
+    while (pSrc < bufEnd)
-                    &val_len)) > 0)
     {
+        rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out),
+                                &name_len, &val_len);
+        assert(rc == 0);
         char *name = out;
         char *val = name + name_len;
         printf("%.*s: %.*s\n", name_len, name, val_len, val);
@@ -358,9 +360,11 @@ test_hpack_test_RFC_Example (void)
         "\x82\x86\x84\xbe\x58\x86\xa8\xeb\x10\x64\x9c\xbf";
     pSrc = bufSamp;
     bufEnd =  bufSamp + strlen((const char *)bufSamp);
-    while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len,
+    while (pSrc < bufEnd)
-                    &val_len)) > 0)
     {
+        rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out),
+                                &name_len, &val_len);
+        assert(rc == 0);
         char *name = out;
         char *val = name + name_len;
         printf("%.*s: %.*s\n", name_len, name, val_len, val);
@@ -370,9 +374,11 @@ test_hpack_test_RFC_Example (void)
         "\x82\x87\x85\xbf\x40\x88\x25\xa8\x49\xe9\x5b\xa9\x7d\x7f\x89\x25\xa8\x49\xe9\x5b\xb8\xe8\xb4\xbf";
     pSrc = bufSamp;
     bufEnd =  bufSamp + strlen((const char *)bufSamp);
-    while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len,
+    while (pSrc < bufEnd)
-                    &val_len)) > 0)
     {
+        rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out),
+                                &name_len, &val_len);
+        assert(rc == 0);
         char *name = out;
         char *val = name + name_len;
         printf("%.*s: %.*s\n", name_len, name, val_len, val);
@@ -420,7 +426,7 @@ test_decode_limits (void)
         src = comp;
         s = lsquic_hdec_decode(&hdec, &src, end, out, out + enough[n],
                                 &name_len, &val_len);
-        assert(1 == s);
+        assert(0 == s);
         assert(src == end);
         assert(16 == name_len);
         assert(17 == val_len);
@@ -481,7 +487,7 @@ test_hpack_self_enc_dec_test (void)
     pSrc = respBuf;
     bufEnd =  pBuf;
     while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len,
-                    &val_len)) > 0)
+                    &val_len)) == 0)
     {
         char *name = out;
         char *val = name + name_len;
@@ -515,7 +521,7 @@ test_hpack_self_enc_dec_test (void)
     pSrc = respBuf;
     bufEnd =  pBuf;
     while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len,
-                    &val_len)) > 0)
+                    &val_len)) == 0)
     {
         char *name = out;
         char *val = name + name_len;
@@ -561,7 +567,7 @@ test_hpack_self_enc_dec_test (void)
     pSrc = respBuf;
     bufEnd =  pBuf;
     while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len,
-                    &val_len)) > 0)
+                    &val_len)) == 0)
     {
         char *name = out;
         char *val = name + name_len;
@@ -602,7 +608,7 @@ test_hpack_self_enc_dec_test (void)
     pSrc = respBuf;
     bufEnd =  pBuf;
     while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len,
-                    &val_len)) > 0)
+                    &val_len)) == 0)
     {
         char *name = out;
         char *val = name + name_len;
@@ -640,7 +646,7 @@ test_hpack_encode_and_decode (void)
     uint16_t name_len, val_len;
     int rc = lsquic_hdec_decode(&hdec, &dec, enc, out, out + sizeof(out), &name_len,
                                                                 &val_len);
-    assert(rc > 0);
+    assert(rc == 0);
     assert(name_len == 10);
     assert(val_len == 15);
     assert(dec == enc);
@@ -715,7 +721,7 @@ test_hpack_self_enc_dec_test_firefox_error (void)
     while (pSrc < bufEnd)
     {
         rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, &val_len);
-        assert(rc > 0);
+        assert(rc == 0);
 
         char *name = out;
         char *val = name + name_len;
@@ -748,7 +754,7 @@ test_hdec_table_size_updates (void)
      * error.
      */
     {
-        unsigned const char buf[] = { 0x20 | 0x1E };
+        unsigned const char buf[] = { 0x20 | 0x1E, 0x88 };
         src = buf;
         lsquic_hdec_init(&hdec);
         lsquic_hdec_set_max_capacity(&hdec, 0x11);
@@ -762,7 +768,7 @@ test_hdec_table_size_updates (void)
     /* Test 2: inline update of capacity smaller than max succeeds.
      */
     {
-        unsigned const char buf[] = { 0x20 | 0x1E };
+        unsigned const char buf[] = { 0x20 | 0x1E, 0x88 };
         src = buf;
         lsquic_hdec_init(&hdec);
         s = lsquic_hdec_decode(&hdec, &src, src + sizeof(buf), outbuf,
@@ -784,6 +790,7 @@ test_hdec_table_size_updates (void)
         unsigned const char buf[] = {
                 0x20 | 0x00,
                 0x20 | 0x14,
+                0x88,
         };
         src = buf;
         lsquic_hdec_init(&hdec);
@@ -795,6 +802,18 @@ test_hdec_table_size_updates (void)
         lsquic_hdec_cleanup(&hdec);
     }
 
+    /* Test 4: header block with table update at the end fails.
+     */
+    {
+        unsigned const char buf[] = { 0x20 | 0x1E, };
+        src = buf;
+        lsquic_hdec_init(&hdec);
+        s = lsquic_hdec_decode(&hdec, &src, src + sizeof(buf), outbuf,
+                            outbuf + sizeof(outbuf), &name_len, &val_len);
+        assert(s < 0);
+        lsquic_hdec_cleanup(&hdec);
+    }
+
 }
 
 
@@ -886,7 +905,7 @@ test_henc_nonascii (void)
     src = comp;
     s = lsquic_hdec_decode(&hdec, &src, end, uncomp, uncomp + sizeof(uncomp),
                                                         &name_len, &val_len);
-    assert(s == 1);
+    assert(s == 0);
     assert(sizeof(value) == val_len);
     assert(0 == memcmp(value, uncomp + name_len, val_len));
     lsquic_hdec_cleanup(&hdec);
@@ -927,7 +946,7 @@ test_henc_long_compressable (void)
     src = comp;
     s = lsquic_hdec_decode(&hdec, &src, end, uncomp, uncomp + sizeof(uncomp),
                                                         &name_len, &val_len);
-    assert(s == 1);
+    assert(s == 0);
     assert(sizeof(value) == val_len);
     assert(0 == memcmp(value, uncomp + name_len, val_len));
     lsquic_hdec_cleanup(&hdec);
@@ -975,7 +994,7 @@ test_henc_long_uncompressable (void)
     src = comp;
     s = lsquic_hdec_decode(&hdec, &src, end, uncomp, uncomp + sizeof(uncomp),
                                                         &name_len, &val_len);
-    assert(s == 1);
+    assert(s == 0);
     assert(sizeof(value) == val_len);
     assert(0 == memcmp(value, uncomp + name_len, val_len));
     lsquic_hdec_cleanup(&hdec);
@@ -1210,7 +1229,7 @@ main (int argc, char **argv)
     for (i = 0; comp < end; ++i)
     {
         s = lsquic_hdec_decode(&hdec, &comp, end, out, out + sizeof(out), &name_len, &val_len);
-        assert(s > 0);
+        assert(s == 0);
         assert(name_len == header_arr[i].name.iov_len);
         assert(0 == memcmp(header_arr[i].name.iov_base, out, name_len));
         assert(val_len == header_arr[i].value.iov_len);