slice: Fix a crash caused by use-after-free (#13113)

Author: maskit

plugins/slice/Data.h

@@ -25,6 +25,7 @@
 #include "Stage.h"
 
 #include <netinet/in.h>
+#include <string>
 #include <unordered_map>
 
 struct Config;
@@ -47,8 +48,8 @@ struct Data {
 
   sockaddr_storage m_client_ip;
 
-  // transaction pointer
-  TSHttpTxn m_txnp{nullptr};
+  // cached effective URL for use during async intercept processing
+  std::string m_effective_url;
 
   // for pristine/effective url coming in
   TSMBuffer m_urlbuf{nullptr};

plugins/slice/server.cc

@@ -90,28 +90,24 @@ enum HeaderState {
 };
 
 static void
-update_object_size(TSHttpTxn txnp, int64_t size, Config &config)
+update_object_size(std::string_view const url, int64_t size, Config &config)
 {
-  int   urllen = 0;
-  char *urlstr = TSHttpTxnEffectiveUrlStringGet(txnp, &urllen);
-  if (urlstr != nullptr) {
-    if (size <= 0) {
-      DEBUG_LOG("Ignoring invalid content length for %.*s: %" PRId64, urllen, urlstr, size);
-      TSfree(urlstr);
-      return;
-    }
+  if (url.empty()) {
+    ERROR_LOG("Could not get URL from transaction.");
+    return;
+  }
 
-    if (static_cast<uint64_t>(size) >= config.m_min_size_to_slice) {
-      config.sizeCacheAdd({urlstr, static_cast<size_t>(urllen)}, static_cast<uint64_t>(size));
-      TSStatIntIncrement(config.stat_TP, 1);
-    } else {
-      config.sizeCacheRemove({urlstr, static_cast<size_t>(urllen)});
-      TSStatIntIncrement(config.stat_FP, 1);
-    }
+  if (size <= 0) {
+    DEBUG_LOG("Ignoring invalid content length for %.*s: %" PRId64, static_cast<int>(url.size()), url.data(), size);
+    return;
+  }
 
-    TSfree(urlstr);
+  if (static_cast<uint64_t>(size) >= config.m_min_size_to_slice) {
+    config.sizeCacheAdd(url, static_cast<uint64_t>(size));
+    TSStatIntIncrement(config.stat_TP, 1);
   } else {
-    ERROR_LOG("Could not get URL from transaction.");
+    config.sizeCacheRemove(url);
+    TSStatIntIncrement(config.stat_FP, 1);
   }
 }
 
@@ -151,7 +147,7 @@ handleFirstServerHeader(Data *const data, TSCont const contp)
     }
     DEBUG_LOG("Passthru bytes: header: %" PRId64 " body: %" PRId64, hlen, clen);
     if (clen != INT64_MAX) {
-      update_object_size(data->m_txnp, clen, *data->m_config);
+      update_object_size(data->m_effective_url, clen, *data->m_config);
       TSVIONBytesSet(output_vio, hlen + clen);
     } else {
       TSVIONBytesSet(output_vio, clen);
@@ -171,7 +167,7 @@ handleFirstServerHeader(Data *const data, TSCont const contp)
     return HeaderState::Fail;
   }
 
-  update_object_size(data->m_txnp, blockcr.m_length, *data->m_config);
+  update_object_size(data->m_effective_url, blockcr.m_length, *data->m_config);
 
   // set the resource content length from block response
   data->m_contentlen = blockcr.m_length;

plugins/slice/slice.cc

@@ -108,7 +108,14 @@ read_request(TSHttpTxn txnp, Config *const config, TSCont read_resp_hdr_contp)
       std::unique_ptr<Data> data = std::make_unique<Data>(config);
 
       data->m_method_type = header.method();
-      data->m_txnp        = txnp;
+
+      // Cache the effective URL now while txnp is still valid
+      int   efflen = 0;
+      char *effstr = TSHttpTxnEffectiveUrlStringGet(txnp, &efflen);
+      if (effstr != nullptr) {
+        data->m_effective_url.assign(effstr, efflen);
+        TSfree(effstr);
+      }
 
       // set up feedback connect
       if (AF_INET == ip->sa_family) {
@@ -200,8 +207,8 @@ read_request(TSHttpTxn txnp, Config *const config, TSCont read_resp_hdr_contp)
         }
       }
 
-      data->m_buffer_index      = TSPluginVCIOBufferIndexGet(data->m_txnp);     // default of m_buffer_index = 32KB
-      data->m_buffer_water_mark = TSPluginVCIOBufferWaterMarkGet(data->m_txnp); // default of m_buffer_water_mark = 0
+      data->m_buffer_index      = TSPluginVCIOBufferIndexGet(txnp);     // default of m_buffer_index = 32KB
+      data->m_buffer_water_mark = TSPluginVCIOBufferWaterMarkGet(txnp); // default of m_buffer_water_mark = 0
 
       if (dbg_ctl.on()) {
         int         len    = 0;