TS-4313: Fix improper type conversion

.gitignore

@@ -86,6 +86,7 @@ iocore/eventsystem/test_Buffer
 iocore/eventsystem/test_Event
 proxy/config/records.config.default
 proxy/config/storage.config.default
+proxy/hdrs/test_mime
 proxy/http2/test_Huffmancode
 
 plugins/header_rewrite/header_rewrite_test

lib/ts/TestBox.h

@@ -25,6 +25,7 @@
 */
 
 #include <stdarg.h>
+#include "ts/ink_apidefs.h"
 #include "ts/Regression.h"
 
 namespace

proxy/hdrs/MIME.cc

@@ -473,19 +473,29 @@ mime_hdr_set_accelerator_slotnum(MIMEHdrImpl *mh, int32_t slot_id, uint32_t slot
 inline void
 mime_hdr_set_accelerators_and_presence_bits(MIMEHdrImpl *mh, MIMEField *field)
 {
-  int slot_id, slot_num;
+  int slot_id;
+  ptrdiff_t slot_num;
   if (field->m_wks_idx < 0)
     return;
 
   mime_hdr_presence_set(mh, field->m_wks_idx);
 
   slot_id = hdrtoken_index_to_slotid(field->m_wks_idx);
   if (slot_id != MIME_SLOTID_NONE) {
-    slot_num = (field - &(mh->m_first_fblock.m_field_slots[0]));
-    if ((slot_num >= 0) && (slot_num < MIME_FIELD_SLOTNUM_UNKNOWN))
-      mime_hdr_set_accelerator_slotnum(mh, slot_id, slot_num);
-    else
-      mime_hdr_set_accelerator_slotnum(mh, slot_id, MIME_FIELD_SLOTNUM_UNKNOWN);
+    if (mh->m_first_fblock.contains(field)) {
+      slot_num = (field - &(mh->m_first_fblock.m_field_slots[0]));
+      // constains() assure that the field is in the block, and the calculated
+      // slot_num will be between 0 and 15, which seem valid.
+      // However, strangely, this function regards slot number 14 and 15 as
+      // unknown for some reason that is not clear. It might be a bug.
+      // The block below is left to keep the original behavior. See also TS-4316.
+      if (slot_num >= MIME_FIELD_SLOTNUM_UNKNOWN) {
+        slot_num = MIME_FIELD_SLOTNUM_UNKNOWN;
+      }
+    } else {
+      slot_num = MIME_FIELD_SLOTNUM_UNKNOWN;
+    }
+    mime_hdr_set_accelerator_slotnum(mh, slot_id, slot_num);
   }
 }
 
@@ -1640,10 +1650,11 @@ mime_hdr_field_slotnum(MIMEHdrImpl *mh, MIMEField *field)
 
   slots_so_far = 0;
   for (fblock = &(mh->m_first_fblock); fblock != NULL; fblock = fblock->m_next) {
-    MIMEField *first = &(fblock->m_field_slots[0]);
-    int block_slot = (int)(field - first); // in units of MIMEField
-    if ((block_slot >= 0) && (block_slot < MIME_FIELD_BLOCK_SLOTS))
-      return (slots_so_far + block_slot);
+    if (fblock->contains(field)) {
+      MIMEField *first = &(fblock->m_field_slots[0]);
+      ptrdiff_t block_slot = field - first; // in units of MIMEField
+      return slots_so_far + block_slot;
+    }
     slots_so_far += MIME_FIELD_BLOCK_SLOTS;
   }
   return -1;
@@ -3615,6 +3626,14 @@ MIMEFieldBlockImpl::strings_length()
   return ret;
 }
 
+bool
+MIMEFieldBlockImpl::contains(const MIMEField *field)
+{
+  MIMEField *first = &(m_field_slots[0]);
+  MIMEField *last = &(m_field_slots[MIME_FIELD_BLOCK_SLOTS - 1]);
+  return (field >= first) && (field <= last);
+}
+
 void
 MIMEFieldBlockImpl::check_strings(HeapCheck *heaps, int num_heaps)
 {

proxy/hdrs/MIME.h

@@ -194,6 +194,7 @@ struct MIMEFieldBlockImpl : public HdrHeapObjImpl {
   void unmarshal(intptr_t offset);
   void move_strings(HdrStrHeap *new_heap);
   size_t strings_length();
+  bool contains(const MIMEField *field);
 
   // Sanity Check Functions
   void check_strings(HeapCheck *heaps, int num_heaps);

proxy/hdrs/Makefile.am

@@ -59,6 +59,17 @@ load_http_hdr_LDADD = -L. -lhdrs \
   @LIBTCL@
 load_http_hdr_LDFLAGS = @EXTRA_CXX_LDFLAGS@ @LIBTOOL_LINK_FLAGS@
 
+check_PROGRAMS = test_mime
+
+TESTS = test_mime
+
+test_mime_LDADD = -L. -lhdrs \
+  $(top_builddir)/lib/ts/libtsutil.la \
+  $(top_builddir)/iocore/eventsystem/libinkevent.a
+test_mime_LDFLAGS = @EXTRA_CXX_LDFLAGS@ @LIBTOOL_LINK_FLAGS@
+
+test_mime_SOURCES = test_mime.cc
+
 #test_UNUSED_SOURCES = \
 #  test_header.cc \
 #  test_urlhash.cc

proxy/hdrs/test_mime.cc

@@ -0,0 +1,64 @@
+/** @file
+
+  A brief file description
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include <ts/TestBox.h>
+#include "P_Eventsystem.h"
+#include "MIME.h"
+
+REGRESSION_TEST(MIME)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatus)
+{
+  TestBox box(t, pstatus);
+  box = REGRESSION_TEST_PASSED;
+
+  MIMEField *field;
+  MIMEHdr hdr;
+  hdr.create(NULL);
+
+  hdr.field_create("Test1", 5);
+  hdr.field_create("Test2", 5);
+  hdr.field_create("Test3", 5);
+  hdr.field_create("Test4", 5);
+  field = hdr.field_create("Test5", 5);
+
+  box.check(hdr.m_mime->m_first_fblock.contains(field), "The field block doesn't contain the field but it should");
+  box.check(!hdr.m_mime->m_first_fblock.contains(field + (1L << 32)), "The field block contains the field but it shouldn't");
+
+  int slot_num = mime_hdr_field_slotnum(hdr.m_mime, field);
+  box.check(slot_num == 4, "Slot number is %d but should be 4", slot_num);
+
+  slot_num = mime_hdr_field_slotnum(hdr.m_mime, field + (1L << 32));
+  box.check(slot_num == -1, "Slot number is %d but should be -1", slot_num);
+
+  hdr.destroy();
+}
+
+int
+main(int argc, char *argv[])
+{
+  Thread *main_thread = new EThread;
+  main_thread->set_specific();
+  mime_init();
+
+  RegressionTest::run();
+  return RegressionTest::final_status == REGRESSION_TEST_PASSED ? 0 : 1;
+}