Repository: trafficserver Updated Branches: refs/heads/master ee5debcf5 -> caafcbbe0
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. This closes #542 Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/caafcbbe Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/caafcbbe Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/caafcbbe Branch: refs/heads/master Commit: caafcbbe0cad5525b337f99514a4bc0e8c71a7ae Parents: ee5debc Author: Masakazu Kitajo <[email protected]> Authored: Wed Mar 30 17:56:31 2016 +0900 Committer: Masakazu Kitajo <[email protected]> Committed: Thu Apr 7 11:29:56 2016 +0900 ---------------------------------------------------------------------- .gitignore | 1 + lib/ts/TestBox.h | 1 + proxy/hdrs/MIME.cc | 39 ++++++++++++++++++++------- proxy/hdrs/MIME.h | 1 + proxy/hdrs/Makefile.am | 11 ++++++++ proxy/hdrs/test_mime.cc | 64 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 107 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/caafcbbe/.gitignore ---------------------------------------------------------------------- diff --git a/.gitignore b/.gitignore index 493d95c..ceb32ae 100644 --- a/.gitignore +++ b/.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 http://git-wip-us.apache.org/repos/asf/trafficserver/blob/caafcbbe/lib/ts/TestBox.h ---------------------------------------------------------------------- diff --git a/lib/ts/TestBox.h b/lib/ts/TestBox.h index 4020fc3..d8140e2 100644 --- a/lib/ts/TestBox.h +++ b/lib/ts/TestBox.h @@ -25,6 +25,7 @@ */ #include <stdarg.h> +#include "ts/ink_apidefs.h" #include "ts/Regression.h" namespace http://git-wip-us.apache.org/repos/asf/trafficserver/blob/caafcbbe/proxy/hdrs/MIME.cc ---------------------------------------------------------------------- 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) { http://git-wip-us.apache.org/repos/asf/trafficserver/blob/caafcbbe/proxy/hdrs/MIME.h ---------------------------------------------------------------------- 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); http://git-wip-us.apache.org/repos/asf/trafficserver/blob/caafcbbe/proxy/hdrs/Makefile.am ---------------------------------------------------------------------- diff --git a/proxy/hdrs/Makefile.am b/proxy/hdrs/Makefile.am index f0a6515..1154aa6 100644 --- a/proxy/hdrs/Makefile.am +++ b/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 http://git-wip-us.apache.org/repos/asf/trafficserver/blob/caafcbbe/proxy/hdrs/test_mime.cc ---------------------------------------------------------------------- diff --git a/proxy/hdrs/test_mime.cc b/proxy/hdrs/test_mime.cc new file mode 100644 index 0000000..c4752ae --- /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 "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; +}
