This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new cbd094c TS-4313: Fix a bug in MIME caused by improper type
conversion
cbd094c is described below
commit cbd094c34d465edbfc637efc10c90da7fe4648f6
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Wed Mar 30 17:56:31 2016 +0900
TS-4313: Fix a bug in MIME caused by improper type conversion
The old logic picks up a wrong block due to overflow that is caused by
improper type conversion.
This fixes the type conversion and make sure that the block contains the
field.
---
.gitignore | 1 +
proxy/hdrs/MIME.cc | 39 ++++++++++++++++++++++--------
proxy/hdrs/MIME.h | 1 +
proxy/hdrs/Makefile.am | 14 +++++++++++
proxy/hdrs/test_mime.cc | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 109 insertions(+), 10 deletions(-)
diff --git a/.gitignore b/.gitignore
index bf65b4e..4a7b81c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -87,6 +87,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
diff --git a/proxy/hdrs/MIME.cc b/proxy/hdrs/MIME.cc
index 2907d35..b0dbc82 100644
--- a/proxy/hdrs/MIME.cc
+++ b/proxy/hdrs/MIME.cc
@@ -473,7 +473,8 @@ 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;
@@ -481,11 +482,20 @@ mime_hdr_set_accelerators_and_presence_bits(MIMEHdrImpl
*mh, MIMEField *field)
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)
{
diff --git a/proxy/hdrs/MIME.h b/proxy/hdrs/MIME.h
index f882df1..5e4f4e3 100644
--- a/proxy/hdrs/MIME.h
+++ b/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);
diff --git a/proxy/hdrs/Makefile.am b/proxy/hdrs/Makefile.am
index f0a6515..428ba1c 100644
--- a/proxy/hdrs/Makefile.am
+++ b/proxy/hdrs/Makefile.am
@@ -59,6 +59,20 @@ 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 \
+ $(top_builddir)/lib/records/librecords_p.a \
+ $(top_builddir)/mgmt/libmgmt_p.la \
+ $(top_builddir)/proxy/shared/libUglyLogStubs.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
diff --git a/proxy/hdrs/test_mime.cc b/proxy/hdrs/test_mime.cc
new file mode 100644
index 0000000..b27235b
--- /dev/null
+++ b/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 "I_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;
+}
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
