BUG/MAJOR: hpack: don't pretend large headers fit in empty table

In hpack_dht_make_room(), we try to fulfill this rule form RFC7541#4.4 :

 "It is not an error to attempt to add an entry that is larger than the
  maximum size; an attempt to add an entry larger than the maximum size
  causes the table to be emptied of all existing entries and results in
  an empty table."

Unfortunately it is not consistent with the way it's used in
hpack_dht_insert() as this last one will consider a success as a
confirmation it can copy the header into the table, and a failure as
an indexing error. This results in the two following issues :
  - if a client sends too large a header into an empty table, this
    header may overflow the table. Fortunately, most clients send
    small headers like :authority first, and never mark headers that
    don't fit into the table as indexable since it is counter-productive ;

  - if a client sends too large a header into a populated table, the
    operation fails after the table is totally flushed and the request
    is not processed.

This patch fixes the two issues at once :
  - a header not fitting into an empty table is always a sign that it
    will never fit ;
  - not fitting into the table is not an error

Thanks to Yves Lafon for reporting detailed traces demonstrating this
issue. This fix must be backported to 1.8.

diff --git a/include/common/hpack-tbl.h b/include/common/hpack-tbl.h
index 824c40018ee642524010ae426e1bda33b2445660..ffa866bb5ae7b7070ddda34d29442bce7993eb2e 100644 (file)
--- a/include/common/hpack-tbl.h
+++ b/include/common/hpack-tbl.h
@@ -216,8 +216,10 @@ static inline struct ist hpack_idx_to_value(const struct hpack_dht *dht, int idx
  */
 static inline int hpack_dht_make_room(struct hpack_dht *dht, unsigned int needed)
 {
-       if (!dht->used || dht->used * 32 + dht->total + needed + 32 <= dht->size)
+       if (dht->used * 32 + dht->total + needed + 32 <= dht->size)
                return 1;
+       else if (!dht->used)
+               return 0;
 
        return __hpack_dht_make_room(dht, needed);
 }

diff --git a/src/hpack-tbl.c b/src/hpack-tbl.c
index 4ca679642278780d7f5808b236749bba937bd99d..06798dc51ab0a6549a4976935c3f612f5aea9acc 100644 (file)
--- a/src/hpack-tbl.c
+++ b/src/hpack-tbl.c
@@ -259,7 +259,7 @@ int hpack_dht_insert(struct hpack_dht *dht, struct ist name, struct ist value)
        uint32_t headroom, tailroom;
 
        if (!hpack_dht_make_room(dht, name.len + value.len))
-               return -1;
+               return 0;
 
        used = dht->used;
        prev = head = dht->head;