You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2020/05/19 06:28:20 UTC

[trafficserver] branch master updated: Fix HPACK Dynamic Table Cleanup

This is an automated email from the ASF dual-hosted git repository.

masaori pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 3376d43  Fix HPACK Dynamic Table Cleanup
3376d43 is described below

commit 3376d438b4a6410187e1ddedd87d2e89279ec196
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Mon May 11 08:56:54 2020 +0900

    Fix HPACK Dynamic Table Cleanup
---
 proxy/http2/HPACK.cc                              | 22 +++------
 proxy/http2/HPACK.h                               |  6 +--
 proxy/http2/unit_tests/test_HpackIndexingTable.cc | 59 +++++++++++++++++++++++
 3 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/proxy/http2/HPACK.cc b/proxy/http2/HPACK.cc
index eefdef2..97a2626 100644
--- a/proxy/http2/HPACK.cc
+++ b/proxy/http2/HPACK.cc
@@ -327,10 +327,10 @@ HpackIndexingTable::size() const
   return _dynamic_table->size();
 }
 
-bool
+void
 HpackIndexingTable::update_maximum_size(uint32_t new_size)
 {
-  return _dynamic_table->update_maximum_size(new_size);
+  _dynamic_table->update_maximum_size(new_size);
 }
 
 //
@@ -403,11 +403,11 @@ HpackDynamicTable::size() const
 // are evicted from the end of the header table until the size of the
 // header table is less than or equal to the maximum size.
 //
-bool
+void
 HpackDynamicTable::update_maximum_size(uint32_t new_size)
 {
   this->_maximum_size = new_size;
-  return this->_evict_overflowed_entries();
+  this->_evict_overflowed_entries();
 }
 
 uint32_t
@@ -416,12 +416,12 @@ HpackDynamicTable::length() const
   return this->_headers.size();
 }
 
-bool
+void
 HpackDynamicTable::_evict_overflowed_entries()
 {
   if (this->_current_size <= this->_maximum_size) {
     // Do nothing
-    return true;
+    return;
   }
 
   for (auto h = this->_headers.rbegin(); h != this->_headers.rend(); ++h) {
@@ -438,13 +438,7 @@ HpackDynamicTable::_evict_overflowed_entries()
     }
   }
 
-  if (this->_headers.size() == 0) {
-    return false;
-  }
-
   this->_mime_hdr_gc();
-
-  return true;
 }
 
 /**
@@ -774,9 +768,7 @@ update_dynamic_table_size(const uint8_t *buf_start, const uint8_t *buf_end, Hpac
     return HPACK_ERROR_COMPRESSION_ERROR;
   }
 
-  if (indexing_table.update_maximum_size(size) == false) {
-    return HPACK_ERROR_COMPRESSION_ERROR;
-  }
+  indexing_table.update_maximum_size(size);
 
   return len;
 }
diff --git a/proxy/http2/HPACK.h b/proxy/http2/HPACK.h
index dc0b517..70f95ed 100644
--- a/proxy/http2/HPACK.h
+++ b/proxy/http2/HPACK.h
@@ -123,12 +123,12 @@ public:
 
   uint32_t maximum_size() const;
   uint32_t size() const;
-  bool update_maximum_size(uint32_t new_size);
+  void update_maximum_size(uint32_t new_size);
 
   uint32_t length() const;
 
 private:
-  bool _evict_overflowed_entries();
+  void _evict_overflowed_entries();
   void _mime_hdr_gc();
 
   uint32_t _current_size = 0;
@@ -157,7 +157,7 @@ public:
   void add_header_field(const MIMEField *field);
   uint32_t maximum_size() const;
   uint32_t size() const;
-  bool update_maximum_size(uint32_t new_size);
+  void update_maximum_size(uint32_t new_size);
 
 private:
   HpackDynamicTable *_dynamic_table;
diff --git a/proxy/http2/unit_tests/test_HpackIndexingTable.cc b/proxy/http2/unit_tests/test_HpackIndexingTable.cc
index 44aabd2..ff470c7 100644
--- a/proxy/http2/unit_tests/test_HpackIndexingTable.cc
+++ b/proxy/http2/unit_tests/test_HpackIndexingTable.cc
@@ -484,4 +484,63 @@ TEST_CASE("HPACK high level APIs", "[hpack]")
       }
     }
   }
+
+  SECTION("dynamic table size update")
+  {
+    HpackIndexingTable indexing_table(4096);
+    REQUIRE(indexing_table.maximum_size() == 4096);
+    REQUIRE(indexing_table.size() == 0);
+
+    // add entries in dynamic table
+    {
+      ats_scoped_obj<HTTPHdr> headers(new HTTPHdr);
+      headers->create(HTTP_TYPE_REQUEST);
+
+      // C.3.1.  First Request
+      uint8_t data[] = {0x82, 0x86, 0x84, 0x41, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x65,
+                        0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d};
+
+      int64_t len = hpack_decode_header_block(indexing_table, headers, data, sizeof(data), MAX_REQUEST_HEADER_SIZE, MAX_TABLE_SIZE);
+      CHECK(len == sizeof(data));
+      CHECK(indexing_table.maximum_size() == 4096);
+      CHECK(indexing_table.size() == 57);
+    }
+
+    // clear all entries by setting a maximum size of 0
+    {
+      ats_scoped_obj<HTTPHdr> headers(new HTTPHdr);
+      headers->create(HTTP_TYPE_REQUEST);
+
+      uint8_t data[] = {0x20};
+
+      int64_t len = hpack_decode_header_block(indexing_table, headers, data, sizeof(data), MAX_REQUEST_HEADER_SIZE, MAX_TABLE_SIZE);
+      CHECK(len == sizeof(data));
+      CHECK(indexing_table.maximum_size() == 0);
+      CHECK(indexing_table.size() == 0);
+    }
+
+    // make the maximum size back to 4096
+    {
+      ats_scoped_obj<HTTPHdr> headers(new HTTPHdr);
+      headers->create(HTTP_TYPE_REQUEST);
+
+      uint8_t data[] = {0x3f, 0xe1, 0x1f};
+
+      int64_t len = hpack_decode_header_block(indexing_table, headers, data, sizeof(data), MAX_REQUEST_HEADER_SIZE, MAX_TABLE_SIZE);
+      CHECK(len == sizeof(data));
+      CHECK(indexing_table.maximum_size() == 4096);
+      CHECK(indexing_table.size() == 0);
+    }
+
+    // error with exceeding the limit (MAX_TABLE_SIZE)
+    {
+      ats_scoped_obj<HTTPHdr> headers(new HTTPHdr);
+      headers->create(HTTP_TYPE_REQUEST);
+
+      uint8_t data[] = {0x3f, 0xe2, 0x1f};
+
+      int64_t len = hpack_decode_header_block(indexing_table, headers, data, sizeof(data), MAX_REQUEST_HEADER_SIZE, MAX_TABLE_SIZE);
+      CHECK(len == HPACK_ERROR_COMPRESSION_ERROR);
+    }
+  }
 }