traeak commented on code in PR #12593:
URL: https://github.com/apache/trafficserver/pull/12593#discussion_r4168412993
##########
src/api/InkAPI.cc:
##########
@@ -5472,57 +5472,142 @@ TSHttpTxnServerRequestBodySet(TSHttpTxn txnp, char
*buf, int64_t buflength)
s->internal_msg_buffer_fast_allocator_size = -1;
}
-void const *
+TSStrategy
TSHttpTxnNextHopStrategyGet(TSHttpTxn txnp)
{
sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
auto sm = reinterpret_cast<HttpSM const *>(txnp);
- return static_cast<void *>(sm->t_state.next_hop_strategy);
+ return reinterpret_cast<TSStrategy>(sm->t_state.next_hop_strategy);
}
-void
-TSHttpTxnNextHopStrategySet(TSHttpTxn txnp, void const *stratptr)
+TSStrategy
+TSHttpTxnNextHopStrategyFind(TSHttpTxn txnp, const char *name)
{
sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
- // null strategy falls back to parent.config
- // sdk_assert(sdk_sanity_check_null_ptr(strategy) == TS_SUCCESS);
+ sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
+
+ auto sm = reinterpret_cast<HttpSM const *>(txnp);
- auto sm = reinterpret_cast<HttpSM *>(txnp);
- auto strategy = reinterpret_cast<NextHopSelectionStrategy const *>(stratptr);
+ // m_remap is null if no rewrite table existed when the transaction started.
+ if (sm->m_remap.get() == nullptr || sm->m_remap->strategyFactory == nullptr)
{
+ return nullptr;
+ }
- sm->t_state.next_hop_strategy = const_cast<NextHopSelectionStrategy
*>(strategy);
+ // HttpSM has a reference count handle to UrlRewrite which has a
+ // pointer to NextHopStrategyFactory
+ NextHopSelectionStrategy *const strategy =
sm->m_remap->strategyFactory->strategyInstance(name);
+
+ return reinterpret_cast<TSStrategy>(strategy);
}
-char const *
-TSHttpNextHopStrategyNameGet(void const *stratptr)
+void
+TSHttpTxnNextHopStrategySet(TSHttpTxn txnp, TSStrategy stratptr)
{
- char const *name = nullptr;
- if (nullptr != stratptr) {
- auto strategy = reinterpret_cast<NextHopSelectionStrategy const
*>(stratptr);
- name = strategy->strategy_name.c_str();
+ sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+
+ auto sm = reinterpret_cast<HttpSM *>(txnp);
+
+ // A null strategy falls back to parent.config.
+ if (stratptr == nullptr) {
+ sm->t_state.next_hop_strategy = nullptr;
+ return;
}
- return name;
+ // Provenance guard: the core dereferences this pointer during parent
+ // selection, so reject handles that are not live strategies in this
+ // transaction's own factory (e.g. one cached across a config reload).
+ auto const candidate = reinterpret_cast<NextHopSelectionStrategy
*>(stratptr);
+ if (sm->m_remap == nullptr || sm->m_remap->strategyFactory == nullptr ||
!sm->m_remap->strategyFactory->contains(candidate)) {
+ TSError("%s: strategy %p is not present in the live strategy factory;
ignoring", __func__, stratptr);
+ return;
+ }
+
+ sm->t_state.next_hop_strategy = candidate;
}
-void const *
-TSHttpTxnNextHopNamedStrategyGet(TSHttpTxn txnp, const char *name)
+namespace
+{
+// Logs once per call site; a misbehaving plugin may call on every transaction.
+void
+log_outside_remap_init(char const *func, std::atomic<bool> &logged)
+{
+ if (!logged.exchange(true, std::memory_order_relaxed)) {
+ TSError("%s: must be called during remap plugin initialization (no loading
remap rule); returning nullptr", func);
+ }
+}
+} // namespace
+
+TSStrategy
+TSRemapNextHopStrategyFind(const char *name)
{
- sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
- auto sm = reinterpret_cast<HttpSM const *>(txnp);
+ // Outside of remap rule initialization (e.g. a globally loaded plugin)
+ // there is no loading mapping on this thread.
+ auto const um = url_mapping::instance;
+ if (um == nullptr) {
+ static std::atomic<bool> logged{false};
+ log_outside_remap_init(__func__, logged);
+ return nullptr;
+ }
- sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap.get()) ==
TS_SUCCESS);
- sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap->strategyFactory)
== TS_SUCCESS);
+ // No strategies.yaml is loaded.
+ if (um->strategyFactory == nullptr) {
+ return nullptr;
+ }
- // HttpSM has a reference count handle to UrlRewrite which has a
- // pointer to NextHopStrategyFactory
- NextHopSelectionStrategy const *const strat =
sm->m_remap->strategyFactory->strategyInstance(name);
+ // The loading UrlRewrite manages the NextHopStrategyFactory pointer.
+ return
reinterpret_cast<TSStrategy>(um->strategyFactory->strategyInstance(name));
+}
+
+void
+TSRemapNextHopStrategySet(TSStrategy strategy)
+{
+ auto const um = url_mapping::instance;
+ if (um == nullptr) {
+ TSError("%s: must be called during remap plugin initialization (no loading
remap rule)", __func__);
+ return;
+ }
+
+ if (strategy == nullptr) {
+ um->strategy = nullptr;
+ return;
+ }
- return static_cast<void const *>(strat);
+ // Compare by pointer only: a stale handle may point at freed memory.
+ auto const candidate = reinterpret_cast<NextHopSelectionStrategy
*>(strategy);
+ if (um->strategyFactory == nullptr ||
!um->strategyFactory->contains(candidate)) {
+ TSError("%s: strategy %p is not present in the loading strategy factory;
ignoring", __func__, strategy);
+ return;
+ }
+
+ um->strategy = candidate;
+}
+
+TSStrategy
+TSRemapNextHopStrategyGet()
+{
+ auto const um = url_mapping::instance;
+ if (um == nullptr) {
+ static std::atomic<bool> logged{false};
+ log_outside_remap_init(__func__, logged);
+ return nullptr;
+ }
+ return reinterpret_cast<TSStrategy>(um->strategy);
+}
+
+char const *
+TSNextHopStrategyNameGet(TSStrategy stratptr)
+{
+ // A null handle has no name; the config-layer "null" sentinel is a
+ // separate concern and must not be conflated with a missing strategy.
+ if (stratptr == nullptr) {
+ return nullptr;
Review Comment:
will be using nullptr if no name matches.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
