[jira] [Created] (TS-3845) HPACK error when trying to delete entries from an empty table

[Bryan Call (JIRA)]

Bryan Call created TS-3845:
------------------------------

             Summary: HPACK error when trying to delete entries from an empty 
table
                 Key: TS-3845
                 URL: https://issues.apache.org/jira/browse/TS-3845
             Project: Traffic Server
          Issue Type: Improvement
          Components: HTTP/2
            Reporter: Bryan Call


This coredump that happens very seldom.  It happens when 
Http2DynamicTable::add_header_field() is called and it tires to delete entries 
from an empty table to make room. 

Here is a patch to stop the core from happening, but it would be better to find 
the root cause.  I am running another patch in production that walks the table 
on every add calculates the used space and compares it to _current_size to see 
if I can diagnose it more.
{code}
diff --git a/proxy/http2/HPACK.cc b/proxy/http2/HPACK.cc
index d65a6b1..76e3877 100644
--- a/proxy/http2/HPACK.cc
+++ b/proxy/http2/HPACK.cc
@@ -230,7 +230,7 @@ Http2DynamicTable::set_dynamic_table_size(uint32_t new_size)
   return true;
 }

-void
+bool
 Http2DynamicTable::add_header_field(const MIMEField *field)
 {
   int name_len, value_len;
@@ -247,7 +247,7 @@ Http2DynamicTable::add_header_field(const MIMEField *field)
     _mhdr->fields_clear();
   } else {
     _current_size += header_size;
-    while (_current_size > _settings_dynamic_table_size) {
+    while (_current_size > _settings_dynamic_table_size && _headers.length() > 
0) {
       int last_name_len, last_value_len;
       MIMEField *last_field = _headers.last();

@@ -259,11 +259,18 @@ Http2DynamicTable::add_header_field(const MIMEField 
*field)
       _mhdr->field_delete(last_field, false);
     }

+    if (_headers.length() == 0 && _current_size - header_size != 0) {
+      Error("No headers in dynamic table, but the size is: %u", _current_size);
+      return false; // this will close the connection
+    }
+
     MIMEField *new_field = _mhdr->field_create(name, name_len);
     new_field->value_set(_mhdr->m_heap, _mhdr->m_mime, value, value_len);
     // XXX Because entire Vec instance is copied, Its too expensive!
     _headers.insert(0, new_field);
   }
+
+  return true;
 }

 // The first byte of an HPACK field unambiguously tells us what
@@ -658,7 +665,9 @@ decode_literal_header_field(MIMEFieldWrapper &header, const 
uint8_t *buf_start,

   // Incremental Indexing adds header to header table as new entry
   if (isIncremental) {
-    dynamic_table.add_header_field(header.field_get());
+    if (dynamic_table.add_header_field(header.field_get()) == false) {
+      return HPACK_ERROR_COMPRESSION_ERROR;
+    }
   }

   // Print decoded header field
diff --git a/proxy/http2/HPACK.h b/proxy/http2/HPACK.h
index 4e63a37..e03ef1a 100644
--- a/proxy/http2/HPACK.h
+++ b/proxy/http2/HPACK.h
@@ -111,7 +111,7 @@ public:
     delete _mhdr;
   }

-  void add_header_field(const MIMEField *field);
+  bool add_header_field(const MIMEField *field);
   int get_header_from_indexing_tables(uint32_t index, MIMEFieldWrapper 
&header_field) const;
   bool set_dynamic_table_size(uint32_t new_size);
{code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)