TSUserArg: add value type checking

include/tscore/PluginUserArgs.h

@@ -35,6 +35,41 @@ static constexpr std::array<size_t, TS_USER_ARGS_COUNT> 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<size_t>(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,13 +88,17 @@ template <TSUserArgType I> 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];
   };
 
   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;
   };

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<size_t>(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_TXN]);
 
   HttpSM *sm = reinterpret_cast<HttpSM *>(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<size_t>(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_TXN]);
 
   HttpSM *sm = reinterpret_cast<HttpSM *>(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<size_t>(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_SSN]);
 
   ProxySession *cs = reinterpret_cast<ProxySession *>(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<size_t>(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_SSN]);
 
   ProxySession *cs = reinterpret_cast<ProxySession *>(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<size_t>(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_VCONN]);
   PluginUserArgsMixin *user_args = dynamic_cast<PluginUserArgsMixin *>(reinterpret_cast<VConnection *>(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<size_t>(arg_idx) < MAX_USER_ARGS[TS_USER_ARGS_VCONN]);
   PluginUserArgsMixin *user_args = dynamic_cast<PluginUserArgsMixin *>(reinterpret_cast<VConnection *>(connp));
   sdk_assert(user_args);
 
   return user_args->get_user_arg(arg_idx);
 }
 
+/* End deprecated Arg functions. */
+
 void
 TSHttpTxnStatusSet(TSHttpTxn txnp, TSHttpStatus status)
 {

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 \

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));
+}