Fix overridable configuration testing

include/proxy/http/HttpConfig.h

@@ -669,9 +669,12 @@ struct OverridableHttpConfigParams {
   //////////////////////////////
   // server verification mode //
   //////////////////////////////
-  char *ssl_client_verify_server_policy     = nullptr;
-  char *ssl_client_verify_server_properties = nullptr;
-  char *ssl_client_sni_policy               = nullptr;
+  char  *ssl_client_verify_server_policy         = nullptr;
+  size_t ssl_client_verify_server_policy_len     = 0;
+  char  *ssl_client_verify_server_properties     = nullptr;
+  size_t ssl_client_verify_server_properties_len = 0;
+  char  *ssl_client_sni_policy                   = nullptr;
+  size_t ssl_client_sni_policy_len               = 0;
 
   MgmtInt proxy_response_hsts_max_age = -1;
 
@@ -786,11 +789,16 @@ struct OverridableHttpConfigParams {
   MgmtFloat background_fill_threshold = 0.5;
 
   // Various strings, good place for them here ...
-  char *ssl_client_cert_filename        = nullptr;
-  char *ssl_client_private_key_filename = nullptr;
-  char *ssl_client_ca_cert_filename     = nullptr;
-  char *ssl_client_ca_cert_path         = nullptr;
-  char *ssl_client_alpn_protocols       = nullptr;
+  char  *ssl_client_cert_filename            = nullptr;
+  size_t ssl_client_cert_filename_len        = 0;
+  char  *ssl_client_private_key_filename     = nullptr;
+  size_t ssl_client_private_key_filename_len = 0;
+  char  *ssl_client_ca_cert_filename         = nullptr;
+  size_t ssl_client_ca_cert_filename_len     = 0;
+  char  *ssl_client_ca_cert_path             = nullptr;
+  size_t ssl_client_ca_cert_path_len         = 0;
+  char  *ssl_client_alpn_protocols           = nullptr;
+  size_t ssl_client_alpn_protocols_len       = 0;
 
   // Host Resolution order
   HostResData host_res_data;

include/proxy/http/OverridableConfigDefs.h

@@ -31,7 +31,7 @@
   - Lua plugin enum and variable array (ts_lua_http_config.cc)
   - String-to-enum mapping (overridable_txn_vars.cc)
   - The _conf_to_memberp switch statement (InkAPI.cc)
-  - SDK_Overridable_Configs test array (InkAPITest.cc)
+  - Config descriptor test array (test_HttpOverridableConfig.cc)
 
   @section xmacro_format X-Macro Format
 

src/api/CMakeLists.txt

@@ -16,7 +16,15 @@
 #######################
 
 # plugin api *only*
-add_library(tsapi SHARED InkAPI.cc InkAPIInternal.cc InkIOCoreAPI.cc)
+add_library(tsapi_objects OBJECT InkAPI.cc InkAPIInternal.cc InkIOCoreAPI.cc)
+set_target_properties(tsapi_objects PROPERTIES POSITION_INDEPENDENT_CODE ON)
+target_link_libraries(tsapi_objects PRIVATE libswoc::libswoc yaml-cpp::yaml-cpp OpenSSL::SSL)
+if(ENABLE_PROBES)
+  target_link_libraries(tsapi_objects PRIVATE systemtap::systemtap)
+endif()
+
+add_library(tsapi SHARED)
+target_link_libraries(tsapi PRIVATE tsapi_objects)
 if(BUILD_REGRESSION_TESTING)
   target_sources(tsapi PRIVATE InkAPITest.cc)
 endif()
@@ -63,4 +71,37 @@ if(APPLE)
   target_link_options(tsapi PRIVATE -undefined dynamic_lookup)
 endif()
 
-clang_tidy_check(tsapi)
+if(BUILD_TESTING)
+  add_executable(test_api unit_tests/test_HttpOverridableConfig.cc)
+  target_link_libraries(
+    test_api
+    PRIVATE Catch2::Catch2WithMain
+            ts::tscore
+            tsapi_objects
+            ts::overridable_txn_vars
+            ts::tsutil
+            ts::http
+            ts::http_remap
+            ts::http2
+            ts::logging
+            ts::hdrs
+            ts::diagsconfig
+            ts::inkutils
+            ts::inkdns
+            ts::inkhostdb
+            ts::inkcache
+            ts::aio
+            ts::proxy
+            ts::inknet
+            ts::records
+            ts::inkevent
+            libswoc::libswoc
+            ts::jsonrpc_protocol
+            ts::jsonrpc_server
+            ts::rpcpublichandlers
+            ts::configmanager
+  )
+  add_catch2_test(NAME test_api COMMAND test_api)
+endif()
+
+clang_tidy_check(tsapi_objects)

src/api/InkAPI.cc

@@ -7521,6 +7521,16 @@ TSHttpTxnConfigStringSet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
 
   s->t_state.setup_per_txn_configs();
 
+  auto set_txn_string = [s, value, length](char *&destination, size_t &destination_length) {
+    if (value && length > 0) {
+      destination        = s->t_state.arena.str_store(value, length);
+      destination_length = length;
+    } else {
+      destination        = nullptr;
+      destination_length = 0;
+    }
+  };
+
   switch (conf) {
   case TS_CONFIG_HTTP_RESPONSE_SERVER_STR:
     if (value && length > 0) {
@@ -7563,44 +7573,31 @@ TSHttpTxnConfigStringSet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
     }
     break;
   case TS_CONFIG_SSL_CLIENT_VERIFY_SERVER_POLICY:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_verify_server_policy = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_verify_server_policy,
+                   s->t_state.my_txn_conf().ssl_client_verify_server_policy_len);
     break;
   case TS_CONFIG_SSL_CLIENT_VERIFY_SERVER_PROPERTIES:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_verify_server_properties = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_verify_server_properties,
+                   s->t_state.my_txn_conf().ssl_client_verify_server_properties_len);
     break;
   case TS_CONFIG_SSL_CLIENT_SNI_POLICY:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_sni_policy = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_sni_policy, s->t_state.my_txn_conf().ssl_client_sni_policy_len);
     break;
   case TS_CONFIG_SSL_CLIENT_CERT_FILENAME:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_cert_filename = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_cert_filename, s->t_state.my_txn_conf().ssl_client_cert_filename_len);
     break;
   case TS_CONFIG_SSL_CLIENT_PRIVATE_KEY_FILENAME:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_private_key_filename = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_private_key_filename,
+                   s->t_state.my_txn_conf().ssl_client_private_key_filename_len);
     break;
   case TS_CONFIG_SSL_CLIENT_CA_CERT_FILENAME:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_ca_cert_filename = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_ca_cert_filename, s->t_state.my_txn_conf().ssl_client_ca_cert_filename_len);
     break;
   case TS_CONFIG_SSL_CLIENT_CA_CERT_PATH:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_ca_cert_path = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_ca_cert_path, s->t_state.my_txn_conf().ssl_client_ca_cert_path_len);
     break;
   case TS_CONFIG_SSL_CLIENT_ALPN_PROTOCOLS:
-    if (value && length > 0) {
-      s->t_state.my_txn_conf().ssl_client_alpn_protocols = const_cast<char *>(value);
-    }
+    set_txn_string(s->t_state.my_txn_conf().ssl_client_alpn_protocols, s->t_state.my_txn_conf().ssl_client_alpn_protocols_len);
     break;
   case TS_CONFIG_SSL_CERT_FILEPATH:
     /* noop */
@@ -7676,9 +7673,37 @@ TSHttpTxnConfigStringGet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
     *value  = sm->t_state.txn_conf->server_session_sharing_match_str;
     *length = *value ? strlen(*value) : 0;
     break;
+  case TS_CONFIG_SSL_CLIENT_VERIFY_SERVER_POLICY:
+    *value  = sm->t_state.txn_conf->ssl_client_verify_server_policy;
+    *length = sm->t_state.txn_conf->ssl_client_verify_server_policy_len;
+    break;
+  case TS_CONFIG_SSL_CLIENT_VERIFY_SERVER_PROPERTIES:
+    *value  = sm->t_state.txn_conf->ssl_client_verify_server_properties;
+    *length = sm->t_state.txn_conf->ssl_client_verify_server_properties_len;
+    break;
+  case TS_CONFIG_SSL_CLIENT_SNI_POLICY:
+    *value  = sm->t_state.txn_conf->ssl_client_sni_policy;
+    *length = sm->t_state.txn_conf->ssl_client_sni_policy_len;
+    break;
+  case TS_CONFIG_SSL_CLIENT_CERT_FILENAME:
+    *value  = sm->t_state.txn_conf->ssl_client_cert_filename;
+    *length = sm->t_state.txn_conf->ssl_client_cert_filename_len;
+    break;
+  case TS_CONFIG_SSL_CLIENT_PRIVATE_KEY_FILENAME:
+    *value  = sm->t_state.txn_conf->ssl_client_private_key_filename;
+    *length = sm->t_state.txn_conf->ssl_client_private_key_filename_len;
+    break;
+  case TS_CONFIG_SSL_CLIENT_CA_CERT_FILENAME:
+    *value  = sm->t_state.txn_conf->ssl_client_ca_cert_filename;
+    *length = sm->t_state.txn_conf->ssl_client_ca_cert_filename_len;
+    break;
   case TS_CONFIG_SSL_CLIENT_CA_CERT_PATH:
     *value  = sm->t_state.txn_conf->ssl_client_ca_cert_path;
-    *length = *value ? strlen(*value) : 0;
+    *length = sm->t_state.txn_conf->ssl_client_ca_cert_path_len;
+    break;
+  case TS_CONFIG_SSL_CLIENT_ALPN_PROTOCOLS:
+    *value  = sm->t_state.txn_conf->ssl_client_alpn_protocols;
+    *length = sm->t_state.txn_conf->ssl_client_alpn_protocols_len;
     break;
   default: {
     MgmtConverter const *conv;

src/api/InkAPITest.cc

@@ -54,8 +54,6 @@
 #include "records/RecHttp.h"
 
 #include "proxy/http/HttpSM.h"
-#include "proxy/http/OverridableConfigDefs.h"
-#include "iocore/net/ConnectionTracker.h"
 #include "tscore/TestBox.h"
 
 namespace
@@ -94,7 +92,8 @@ DbgCtl dbg_ctl_SockClient{"SockClient"};
 #define ERROR_BODY              "TESTING ERROR PAGE"
 #define TRANSFORM_APPEND_STRING "This is a transformed response"
 
-extern int dns_failover_period;
+extern int                    dns_failover_period;
+extern ClassAllocator<HttpSM> httpSMAllocator;
 
 //////////////////////////////////////////////////////////////////////////////
 // STRUCTURES
@@ -8707,140 +8706,6 @@ EXCLUSIVE_REGRESSION_TEST(SDK_API_TSHttpConnectServerIntercept)(RegressionTest *
   return;
 }
 
-////////////////////////////////////////////////
-// SDK_API_OVERRIDABLE_CONFIGS
-//
-// Unit Test for API: TSHttpTxnConfigFind
-//                    TSHttpTxnConfigIntSet
-//                    TSHttpTxnConfigIntGet
-//                    TSHttpTxnConfigFloatSet
-//                    TSHttpTxnConfigFloatGet
-//                    TSHttpTxnConfigStringSet
-//                    TSHttpTxnConfigStringGet
-////////////////////////////////////////////////
-
-// Generate the SDK_Overridable_Configs array from the X-macro.
-// The order MUST match TSOverridableConfigKey enum order (enforced by static_assert).
-// clang-format off
-#define X_SDK_CONFIG(CONFIG_KEY, MEMBER, RECORD_NAME, DATA_TYPE, CONV) RECORD_NAME,
-std::array<std::string_view, TS_CONFIG_LAST_ENTRY> SDK_Overridable_Configs = {{
-  OVERRIDABLE_CONFIGS(X_SDK_CONFIG)
-}};
-#undef X_SDK_CONFIG
-// clang-format on
-
-static_assert(SDK_Overridable_Configs.size() == TS_CONFIG_LAST_ENTRY,
-              "SDK_Overridable_Configs size must match TS_CONFIG_LAST_ENTRY");
-
-extern ClassAllocator<HttpSM> httpSMAllocator;
-
-REGRESSION_TEST(SDK_API_OVERRIDABLE_CONFIGS)(RegressionTest *test, int /* atype ATS_UNUSED */, int *pstatus)
-{
-  TSOverridableConfigKey key;
-  TSRecordDataType       type;
-  HttpSM                *s       = THREAD_ALLOC(httpSMAllocator, this_thread());
-  bool                   success = true;
-  TSHttpTxn              txnp    = reinterpret_cast<TSHttpTxn>(s);
-  InkRand                generator(17);
-  TSMgmtInt              ival_read, ival_rand;
-  TSMgmtFloat            fval_read, fval_rand;
-  const char            *sval_read;
-  const char            *test_string = "The Apache Traffic Server";
-  int                    len;
-
-  s->init();
-  s->mutex = new_ProxyMutex();
-  SCOPED_MUTEX_LOCK(lock, s->mutex, this_ethread());
-
-  HttpCacheSM *c_sm = &(s->get_cache_sm());
-  c_sm->init(s, s->mutex);
-
-  *pstatus = REGRESSION_TEST_INPROGRESS;
-  for (int i = 0; i < static_cast<int>(SDK_Overridable_Configs.size()); ++i) {
-    std::string_view conf{SDK_Overridable_Configs[i]};
-
-    if (TS_SUCCESS == TSHttpTxnConfigFind(conf.data(), -1, &key, &type)) {
-      if (key != i) {
-        SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Failed on %s, expected %d, got %d", conf.data(), i, key);
-        success = false;
-        continue;
-      }
-    } else {
-      SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Call returned unexpected TS_ERROR for %s", conf.data());
-      success = false;
-      continue;
-    }
-
-    if (TS_SUCCESS == TSHttpTxnConfigFind(conf.data(), conf.size(), &key, &type)) {
-      if (key != i) {
-        SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Failed on %s, expected %d, got %d", conf.data(), i, key);
-        success = false;
-        continue;
-      }
-    } else {
-      SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Call returned unexpected TS_ERROR for %s", conf.data());
-      success = false;
-      continue;
-    }
-
-    // Now check the getters / setters
-    switch (type) {
-    case TS_RECORDDATATYPE_INT:
-      ival_rand = generator.random() % 126; // to fit in a signed byte
-      TSHttpTxnConfigIntSet(txnp, key, ival_rand);
-      TSHttpTxnConfigIntGet(txnp, key, &ival_read);
-      if (ival_rand != ival_read) {
-        SDK_RPRINT(test, "TSHttpTxnConfigIntSet", "TestCase1", TC_FAIL, "Failed on %s, %d != %d", conf.data(), ival_read,
-                   ival_rand);
-        success = false;
-        continue;
-      }
-      break;
-
-    case TS_RECORDDATATYPE_FLOAT:
-      fval_rand = generator.random();
-      TSHttpTxnConfigFloatSet(txnp, key, fval_rand);
-      TSHttpTxnConfigFloatGet(txnp, key, &fval_read);
-      if (fval_rand != fval_read) {
-        SDK_RPRINT(test, "TSHttpTxnConfigFloatSet", "TestCase1", TC_FAIL, "Failed on %s, %f != %f", conf.data(), fval_read,
-                   fval_rand);
-        success = false;
-        continue;
-      }
-      break;
-
-    case TS_RECORDDATATYPE_STRING:
-      TSHttpTxnConfigStringSet(txnp, key, test_string, -1);
-      TSHttpTxnConfigStringGet(txnp, key, &sval_read, &len);
-      // Compare string content, not pointers - the implementation may store
-      // a copy of the string (e.g., in ParsedConfigCache for efficiency).
-      if (sval_read == nullptr || std::string_view(test_string) != std::string_view(sval_read, len)) {
-        SDK_RPRINT(test, "TSHttpTxnConfigStringSet", "TestCase1", TC_FAIL, "Failed on %s, %s != %s", conf.data(),
-                   sval_read ? sval_read : "(null)", test_string);
-        success = false;
-        continue;
-      }
-      break;
-
-    default:
-      break;
-    }
-  }
-
-  s->destroy();
-  if (success) {
-    *pstatus = REGRESSION_TEST_PASSED;
-    SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_PASS, "ok");
-    SDK_RPRINT(test, "TSHttpTxnConfigIntSet", "TestCase1", TC_PASS, "ok");
-    SDK_RPRINT(test, "TSHttpTxnConfigFloatSet", "TestCase1", TC_PASS, "ok");
-    SDK_RPRINT(test, "TSHttpTxnConfigStringSet", "TestCase1", TC_PASS, "ok");
-  } else {
-    *pstatus = REGRESSION_TEST_FAILED;
-  }
-
-  return;
-}
-
 ////////////////////////////////////////////////
 // SDK_API_TXN_HTTP_INFO_INFO_GET
 //