diff --git a/include/tscore/PluginUserArgs.h b/include/tscore/PluginUserArgs.h index 057e37c1b3a..4dd637b2158 100644 --- a/include/tscore/PluginUserArgs.h +++ b/include/tscore/PluginUserArgs.h @@ -35,6 +35,41 @@ static constexpr std::array MAX_USER_ARGS = {{ 128 /* max number of user arguments for GLB */ }}; +/** Stagger each user argument value so we can detect mismatched + * indices. + * + * For example, say a plugin associates data with both sessions and + * transactions and that its session index is 2 and its transaction index is 4. + * In this case, we'll hand back to the plugin 2002 for its session index and + * 1004 for its transaction index. If it then accidentally uses its session + * index to reference its transaction index, it will pass back 2002 instead of + * 1004, which we will identify as belonging to the wrong user argument type + * because it is in the 2000 session block rather than the expected 1000 + * transaction block. + * + * Note that these higher value 1000 block indices are only used when + * interfacing with the plugin. Internally the lower valued index is used. + * That is, for a transaction, expect internally a value of 3 instead of 1003. + */ +static constexpr size_t +get_user_arg_offset(TSUserArgType type) +{ + // TS_USER_ARGS_TXN indices begin at 1000, TS_USER_ARGS_SSN begin at 2000, + // etc. + return (static_cast(type) + 1) * 1000; +} + +/** Verify that the user passed in an index whose value corresponds with the + * type. See the comment above the declaration of get_user_arg_offset for the + * intention behind this. + */ +static constexpr inline bool +SanityCheckUserIndex(TSUserArgType type, int idx) +{ + int const block_start = get_user_arg_offset(type); + return idx >= block_start && idx < block_start + 1000; +} + /** This is a mixin class (sort of), implementing the appropriate APIs and data storage for a particular user arg table. Used by VConn / Ssn / Txn user arg data. @@ -53,6 +88,8 @@ template class PluginUserArgs : public virtual PluginUserArgsM void * get_user_arg(size_t ix) const { + ink_release_assert(SanityCheckUserIndex(I, ix)); + ix -= get_user_arg_offset(I); ink_release_assert(ix < user_args.size()); return this->user_args[ix]; }; @@ -60,6 +97,8 @@ template class PluginUserArgs : public virtual PluginUserArgsM void set_user_arg(size_t ix, void *arg) { + ink_release_assert(SanityCheckUserIndex(I, ix)); + ix -= get_user_arg_offset(I); ink_release_assert(ix < user_args.size()); user_args[ix] = arg; }; diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index f43796cad4f..0235ac54379 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -6312,6 +6312,9 @@ TSUserArgIndexReserve(TSUserArgType type, const char *name, const char *descript if (TS_SUCCESS == TSUserArgIndexNameLookup(type, name, &idx, &desc)) { // Found existing index. + + // No need to add get_user_arg_offset(type) here since + // TSUserArgIndexNameLookup already does so. *ptr_idx = idx; return TS_SUCCESS; } @@ -6325,7 +6328,7 @@ TSUserArgIndexReserve(TSUserArgType type, const char *name, const char *descript if (description) { arg.description = description; } - *ptr_idx = idx; + *ptr_idx = idx + get_user_arg_offset(type); return TS_SUCCESS; } @@ -6336,6 +6339,8 @@ TSReturnCode TSUserArgIndexLookup(TSUserArgType type, int idx, const char **name, const char **description) { sdk_assert(0 <= type && type < TS_USER_ARGS_COUNT); + sdk_assert(SanityCheckUserIndex(type, idx)); + idx -= get_user_arg_offset(type); if (sdk_sanity_check_null_ptr(name) == TS_SUCCESS) { if (idx < UserArgIdx[type]) { UserArg &arg(UserArgTable[type][idx]); @@ -6363,7 +6368,7 @@ TSUserArgIndexNameLookup(TSUserArgType type, const char *name, int *arg_idx, con if (description) { *description = arg->description.c_str(); } - *arg_idx = arg - UserArgTable[type]; + *arg_idx = arg - UserArgTable[type] + get_user_arg_offset(type); return TS_SUCCESS; } } @@ -6398,6 +6403,7 @@ TSUserArgGet(void *data, int arg_idx) } // ------------- +/* These are deprecated as of v9.0.0, and will be removed in v10.0.0 */ TSReturnCode TSHttpTxnArgIndexReserve(const char *name, const char *description, int *arg_idx) { @@ -6456,7 +6462,6 @@ void TSHttpTxnArgSet(TSHttpTxn txnp, int arg_idx, void *arg) { sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS); - sdk_assert(arg_idx >= 0 && static_cast(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_TXN]); HttpSM *sm = reinterpret_cast(txnp); @@ -6467,7 +6472,6 @@ void * TSHttpTxnArgGet(TSHttpTxn txnp, int arg_idx) { sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS); - sdk_assert(arg_idx >= 0 && static_cast(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_TXN]); HttpSM *sm = reinterpret_cast(txnp); return sm->get_user_arg(arg_idx); @@ -6477,7 +6481,6 @@ void TSHttpSsnArgSet(TSHttpSsn ssnp, int arg_idx, void *arg) { sdk_assert(sdk_sanity_check_http_ssn(ssnp) == TS_SUCCESS); - sdk_assert(arg_idx >= 0 && static_cast(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_SSN]); ProxySession *cs = reinterpret_cast(ssnp); @@ -6488,7 +6491,6 @@ void * TSHttpSsnArgGet(TSHttpSsn ssnp, int arg_idx) { sdk_assert(sdk_sanity_check_http_ssn(ssnp) == TS_SUCCESS); - sdk_assert(arg_idx >= 0 && static_cast(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_SSN]); ProxySession *cs = reinterpret_cast(ssnp); return cs->get_user_arg(arg_idx); @@ -6498,7 +6500,6 @@ void TSVConnArgSet(TSVConn connp, int arg_idx, void *arg) { sdk_assert(sdk_sanity_check_iocore_structure(connp) == TS_SUCCESS); - sdk_assert(arg_idx >= 0 && static_cast(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_VCONN]); PluginUserArgsMixin *user_args = dynamic_cast(reinterpret_cast(connp)); sdk_assert(user_args); @@ -6509,13 +6510,14 @@ void * TSVConnArgGet(TSVConn connp, int arg_idx) { sdk_assert(sdk_sanity_check_iocore_structure(connp) == TS_SUCCESS); - sdk_assert(arg_idx >= 0 && static_cast(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_VCONN]); PluginUserArgsMixin *user_args = dynamic_cast(reinterpret_cast(connp)); sdk_assert(user_args); return user_args->get_user_arg(arg_idx); } +/* End deprecated Arg functions. */ + void TSHttpTxnStatusSet(TSHttpTxn txnp, TSHttpStatus status) { diff --git a/src/tscore/Makefile.am b/src/tscore/Makefile.am index c0ca76c670e..8861b86afbd 100644 --- a/src/tscore/Makefile.am +++ b/src/tscore/Makefile.am @@ -183,6 +183,7 @@ test_tscore_SOURCES = \ unit_tests/test_MemArena.cc \ unit_tests/test_MT_hashtable.cc \ unit_tests/test_ParseRules.cc \ + unit_tests/test_PluginUserArgs.cc \ unit_tests/test_PriorityQueue.cc \ unit_tests/test_Ptr.cc \ unit_tests/test_Regex.cc \ diff --git a/src/tscore/unit_tests/test_PluginUserArgs.cc b/src/tscore/unit_tests/test_PluginUserArgs.cc new file mode 100644 index 00000000000..72dc7eec711 --- /dev/null +++ b/src/tscore/unit_tests/test_PluginUserArgs.cc @@ -0,0 +1,56 @@ +/** + @file Test for Regex.cc + + @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 "tscore/PluginUserArgs.h" +#include "catch.hpp" + +TEST_CASE("get_user_arg_offset", "[libts][PluginUserArgs]") +{ + CHECK(get_user_arg_offset(TS_USER_ARGS_TXN) == 1000); + CHECK(get_user_arg_offset(TS_USER_ARGS_SSN) == 2000); + CHECK(get_user_arg_offset(TS_USER_ARGS_VCONN) == 3000); + CHECK(get_user_arg_offset(TS_USER_ARGS_GLB) == 4000); +} + +TEST_CASE("SanityCheckUserIndex", "[libts][PluginUserArgs]") +{ + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_TXN, 0)); + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_TXN, 1)); + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_TXN, 999)); + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_TXN, 2000)); + + CHECK(SanityCheckUserIndex(TS_USER_ARGS_TXN, 1000)); + CHECK(SanityCheckUserIndex(TS_USER_ARGS_TXN, 1001)); + CHECK(SanityCheckUserIndex(TS_USER_ARGS_TXN, 1999)); + + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_SSN, 1000)); + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_SSN, 3000)); + CHECK(SanityCheckUserIndex(TS_USER_ARGS_SSN, 2000)); + CHECK(SanityCheckUserIndex(TS_USER_ARGS_SSN, 2001)); + CHECK(SanityCheckUserIndex(TS_USER_ARGS_SSN, 2999)); + + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_VCONN, 2000)); + CHECK(SanityCheckUserIndex(TS_USER_ARGS_VCONN, 3000)); + + CHECK_FALSE(SanityCheckUserIndex(TS_USER_ARGS_GLB, 3000)); + CHECK(SanityCheckUserIndex(TS_USER_ARGS_GLB, 4000)); +}