Add STL compliant field iteration to MIMEHdr.

proxy/hdrs/MIME.h

@@ -987,6 +987,43 @@ struct MIMEFieldIter {
 class MIMEHdr : public HdrHeapSDKHandle
 {
 public:
+  /** Iterator over fields in the header.
+   * This iterator should be stable over field deletes, but not insertions.
+   */
+  class iterator
+  {
+    using self_type = iterator; ///< Self reference types.
+
+  public:
+    iterator() = default;
+
+    // STL iterator compliance types.
+    using difference_type   = void;
+    using value_type        = MIMEField;
+    using pointer           = value_type *;
+    using reference         = value_type &;
+    using iterator_category = std::forward_iterator_tag;
+
+    pointer operator->();
+    reference operator*();
+
+    self_type &operator++();
+    self_type operator++(int);
+
+    bool operator==(self_type const &that);
+    bool operator!=(self_type const &that);
+
+  protected:
+    MIMEFieldBlockImpl *_block = nullptr; ///< Current block.
+    unsigned _slot             = 0;       ///< Slot in @a _block
+
+    /// Internal method to move to a valid slot.
+    /// If the current location is valid, this is a no-op.
+    self_type &step();
+
+    friend class MIMEHdr;
+  };
+
   MIMEHdrImpl *m_mime = nullptr;
 
   MIMEHdr() = default; // Force the creation of the default constructor
@@ -1009,9 +1046,14 @@ class MIMEHdr : public HdrHeapSDKHandle
   void field_delete(MIMEField *field, bool delete_all_dups = true);
   void field_delete(const char *name, int name_length);
 
+  iterator begin();
+  iterator end();
+
+  /*
   MIMEField *iter_get_first(MIMEFieldIter *iter);
   MIMEField *iter_get(MIMEFieldIter *iter);
   MIMEField *iter_get_next(MIMEFieldIter *iter);
+   */
 
   uint64_t presence(uint64_t mask);
 
@@ -1229,52 +1271,86 @@ MIMEHdr::field_delete(MIMEField *field, bool delete_all_dups)
   mime_hdr_field_delete(m_heap, m_mime, field, delete_all_dups);
 }
 
-inline void
-MIMEHdr::field_delete(const char *name, int name_length)
+inline auto
+MIMEHdr::begin() -> iterator
 {
-  MIMEField *field = field_find(name, name_length);
-  if (field)
-    field_delete(field);
+  iterator spot;
+  spot._block = &m_mime->m_first_fblock;
+  spot._slot  = 0;
+  return spot.step();
 }
 
-inline MIMEField *
-MIMEHdr::iter_get_first(MIMEFieldIter *iter)
+inline auto
+MIMEHdr::end() -> iterator
 {
-  iter->m_block = &m_mime->m_first_fblock;
-  iter->m_slot  = 0;
-  return iter_get(iter);
+  return {}; // default constructed iterator.
 }
 
-inline MIMEField *
-MIMEHdr::iter_get(MIMEFieldIter *iter)
+inline auto
+MIMEHdr::iterator::step() -> self_type &
 {
-  MIMEField *f;
-  MIMEFieldBlockImpl *b = iter->m_block;
-
-  int slot = iter->m_slot;
-
-  while (b) {
-    for (; slot < (int)b->m_freetop; slot++) {
-      f = &(b->m_field_slots[slot]);
-      if (f->is_live()) {
-        iter->m_slot  = slot;
-        iter->m_block = b;
-        return f;
+  while (_block) {
+    auto limit = _block->m_freetop;
+    while (_slot < limit) {
+      if (_block->m_field_slots[_slot].is_live()) {
+        return *this;
       }
+      ++_slot;
     }
-    b    = b->m_next;
-    slot = 0;
+    _block = _block->m_next;
+    _slot  = 0;
   }
+  return *this;
+}
 
-  iter->m_block = nullptr;
-  return nullptr;
+inline auto
+MIMEHdr::iterator::operator*() -> reference
+{
+  return _block->m_field_slots[_slot];
 }
 
-inline MIMEField *
-MIMEHdr::iter_get_next(MIMEFieldIter *iter)
+inline auto
+MIMEHdr::iterator::operator->() -> pointer
+{
+  return &(_block->m_field_slots[_slot]);
+}
+
+inline bool
+MIMEHdr::iterator::operator==(const self_type &that)
+{
+  return _block == that._block && _slot == that._slot;
+}
+
+inline bool
+MIMEHdr::iterator::operator!=(const self_type &that)
+{
+  return _block != that._block || _slot != that._slot;
+}
+
+inline auto
+MIMEHdr::iterator::operator++() -> self_type &
 {
-  iter->m_slot++;
-  return iter_get(iter);
+  if (_block) {
+    ++_slot;
+    this->step();
+  }
+  return *this;
+}
+
+inline auto
+MIMEHdr::iterator::operator++(int) -> self_type
+{
+  self_type zret{*this};
+  this->step();
+  return zret;
+}
+
+inline void
+MIMEHdr::field_delete(const char *name, int name_length)
+{
+  MIMEField *field = field_find(name, name_length);
+  if (field)
+    field_delete(field);
 }
 
 /*-------------------------------------------------------------------------

proxy/http/HttpTransact.cc

@@ -4951,17 +4951,14 @@ HttpTransact::set_headers_for_cache_write(State *s, HTTPInfo *cache_info, HTTPHd
 void
 HttpTransact::merge_response_header_with_cached_header(HTTPHdr *cached_header, HTTPHdr *response_header)
 {
-  MIMEField *field;
   MIMEField *new_field;
-  MIMEFieldIter fiter;
   const char *name;
   bool dups_seen = false;
 
-  field = response_header->iter_get_first(&fiter);
-
-  for (; field != nullptr; field = response_header->iter_get_next(&fiter)) {
+  for (auto spot = response_header->begin(), limit = response_header->end(); spot != limit; ++spot) {
+    MIMEField &field{*spot};
     int name_len;
-    name = field->name_get(&name_len);
+    name = field.name_get(&name_len);
 
     ///////////////////////////
     // is hop-by-hop header? //
@@ -5011,31 +5008,20 @@ HttpTransact::merge_response_header_with_cached_header(HTTPHdr *cached_header, H
     //   the remaining fields one by one from the
     //   response header
     //
-    if (field->m_next_dup) {
+    if (field.m_next_dup) {
       if (dups_seen == false) {
-        MIMEField *dfield;
         // use a second iterator to delete the
         // remaining response headers in the cached response,
         // so that they will be added in the next iterations.
-        MIMEFieldIter fiter2 = fiter;
-        const char *dname    = name;
-        int dlen             = name_len;
-
-        while (dname) {
-          cached_header->field_delete(dname, dlen);
-          dfield = response_header->iter_get_next(&fiter2);
-          if (dfield) {
-            dname = dfield->name_get(&dlen);
-          } else {
-            dname = nullptr;
-          }
+        for (auto spot2 = spot; spot2 != limit; ++spot2) {
+          cached_header->field_delete(&*spot2, true);
         }
         dups_seen = true;
       }
     }
 
     int value_len;
-    const char *value = field->value_get(&value_len);
+    const char *value = field.value_get(&value_len);
 
     if (dups_seen == false) {
       cached_header->value_set(name, name_len, value, value_len);

proxy/http/HttpTransactHeaders.cc

@@ -200,8 +200,6 @@ HttpTransactHeaders::copy_header_fields(HTTPHdr *src_hdr, HTTPHdr *new_hdr, bool
   ink_assert(src_hdr->valid());
   ink_assert(!new_hdr->valid());
 
-  MIMEField *field;
-  MIMEFieldIter field_iter;
   bool date_hdr = false;
 
   // Start with an exact duplicate
@@ -222,19 +220,19 @@ HttpTransactHeaders::copy_header_fields(HTTPHdr *src_hdr, HTTPHdr *new_hdr, bool
   //         is changed for example by dechunking, the transfer encoding
   //         should be modified when when the decision is made to dechunk it
 
-  for (field = new_hdr->iter_get_first(&field_iter); field != nullptr; field = new_hdr->iter_get_next(&field_iter)) {
-    if (field->m_wks_idx == -1) {
+  for (auto &field : *new_hdr) {
+    if (field.m_wks_idx == -1) {
       continue;
     }
 
-    int field_flags = hdrtoken_index_to_flags(field->m_wks_idx);
+    int field_flags = hdrtoken_index_to_flags(field.m_wks_idx);
 
     if (field_flags & HTIF_HOPBYHOP) {
       // Delete header if not in special proxy_auth retention mode
       if ((!retain_proxy_auth_hdrs) || (!(field_flags & HTIF_PROXYAUTH))) {
-        new_hdr->field_delete(field);
+        new_hdr->field_delete(&field);
       }
-    } else if (field->m_wks_idx == MIME_WKSIDX_DATE) {
+    } else if (field.m_wks_idx == MIME_WKSIDX_DATE) {
       date_hdr = true;
     }
   }

proxy/http2/HPACK.cc

@@ -908,11 +908,10 @@ hpack_encode_header_block(HpackIndexingTable &indexing_table, uint8_t *out_buf,
     cursor += written;
   }
 
-  MIMEFieldIter field_iter;
-  for (MIMEField *field = hdr->iter_get_first(&field_iter); field != nullptr; field = hdr->iter_get_next(&field_iter)) {
+  for (auto &field : *hdr) {
     // Convert field name to lower case to follow HTTP2 spec
     // This conversion is needed because WKSs in MIMEFields is old fashioned
-    std::string_view original_name = field->name_get();
+    std::string_view original_name = field.name_get();
     int name_len                   = original_name.size();
     ts::LocalBuffer<char> local_buffer(name_len);
     char *lower_name = local_buffer.data();
@@ -921,7 +920,7 @@ hpack_encode_header_block(HpackIndexingTable &indexing_table, uint8_t *out_buf,
     }
 
     std::string_view name{lower_name, static_cast<size_t>(name_len)};
-    std::string_view value = field->value_get();
+    std::string_view value = field.value_get();
 
     // Choose field representation (See RFC7541 7.1.3)
     // - Authorization header obviously should not be indexed

proxy/http2/HTTP2.cc

@@ -498,9 +498,8 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
   }
 
   // Check validity of all names and values
-  MIMEFieldIter iter;
-  for (auto *mf = headers->iter_get_first(&iter); mf != nullptr; mf = headers->iter_get_next(&iter)) {
-    if (!mf->name_is_valid() || !mf->value_is_valid()) {
+  for (auto &mf : *headers) {
+    if (!mf.name_is_valid() || !mf.value_is_valid()) {
       return PARSE_RESULT_ERROR;
     }
   }
@@ -707,8 +706,8 @@ http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t *buf_start, const uint32_
   if (is_trailing_header) {
     expected_pseudo_header_count = 0;
   }
-  for (field = hdr->iter_get_first(&iter); field != nullptr; field = hdr->iter_get_next(&iter)) {
-    value = field->name_get(&len);
+  for (auto &field : *hdr) {
+    value = field.name_get(&len);
     // Pseudo headers must appear before regular headers
     if (len && value[0] == ':') {
       ++pseudo_header_count;

proxy/http2/test_HPACK.cc

@@ -133,34 +133,31 @@ compare_header_fields(HTTPHdr *a, HTTPHdr *b)
     return -1;
   }
 
-  MIMEFieldIter a_iter, b_iter;
+  auto a_spot = a->begin(), a_limit = a->end();
+  auto b_spot = b->begin(), b_limit = b->end();
 
-  const MIMEField *a_field = a->iter_get_first(&a_iter);
-  const MIMEField *b_field = b->iter_get_first(&b_iter);
-
-  while (a_field != nullptr && b_field != nullptr) {
+  while (a_spot != a_limit && b_spot != b_limit) {
     int a_str_len, b_str_len;
     // compare header name
-    const char *a_str = a_field->name_get(&a_str_len);
-    const char *b_str = b_field->name_get(&b_str_len);
+    const char *a_str = a_spot->name_get(&a_str_len);
+    const char *b_str = b_spot->name_get(&b_str_len);
     if (a_str_len != b_str_len) {
       if (memcmp(a_str, b_str, a_str_len) != 0) {
         print_difference(a_str, a_str_len, b_str, b_str_len);
         return -1;
       }
     }
     // compare header value
-    a_str = a_field->value_get(&a_str_len);
-    b_str = b_field->value_get(&b_str_len);
+    a_str = a_spot->value_get(&a_str_len);
+    b_str = b_spot->value_get(&b_str_len);
     if (a_str_len != b_str_len) {
       if (memcmp(a_str, b_str, a_str_len) != 0) {
         print_difference(a_str, a_str_len, b_str, b_str_len);
         return -1;
       }
     }
-
-    a_field = a->iter_get_next(&a_iter);
-    b_field = b->iter_get_next(&b_iter);
+    ++a_spot;
+    ++b_spot;
   }
 
   return 0;

proxy/http3/QPACK.cc

@@ -191,9 +191,8 @@ QPACK::encode(uint64_t stream_id, HTTPHdr &header_set, MIOBuffer *header_block,
   IOBufferBlock *compressed_headers = new_IOBufferBlock();
   compressed_headers->alloc(BUFFER_SIZE_INDEX_2K);
 
-  MIMEFieldIter field_iter;
-  for (MIMEField *field = header_set.iter_get_first(&field_iter); field != nullptr; field = header_set.iter_get_next(&field_iter)) {
-    int ret            = this->_encode_header(*field, base_index, compressed_headers, referred_index);
+  for (auto &field : header_set) {
+    int ret            = this->_encode_header(field, base_index, compressed_headers, referred_index);
     largest_reference  = std::max(largest_reference, referred_index);
     smallest_reference = std::min(smallest_reference, referred_index);
     if (ret < 0) {

proxy/logging/LogUtils.cc

@@ -670,14 +670,9 @@ marshalMimeHdr(MIMEHdr *hdr, char *buf)
   ts::FixedBufferWriter bw(buf, bwSize);
 
   if (hdr) {
-    MIMEFieldIter mfIter;
-    const MIMEField *mfp = hdr->iter_get_first(&mfIter);
-
-    while (mfp) {
-      marshalStr(bw, *mfp, &MIMEField::name_get);
-      marshalStr(bw, *mfp, &MIMEField::value_get);
-
-      mfp = hdr->iter_get_next(&mfIter);
+    for (auto const &mfp : *hdr) {
+      marshalStr(bw, mfp, &MIMEField::name_get);
+      marshalStr(bw, mfp, &MIMEField::value_get);
     }
   }
 

proxy/logging/unit-tests/test_LogUtils.cc

@@ -52,8 +52,6 @@ test(const MIMEField *pairs, int numPairs, const char *asciiResult, int extraUnm
 
   REQUIRE(binAlignSize < static_cast<int>(sizeof(binBuf)));
 
-  hdr.reset();
-
   REQUIRE(marshalMimeHdr(numPairs ? &hdr : nullptr, binBuf) == binAlignSize);
 
   int binSize{1};