Copilot commented on code in PR #12593:
URL: https://github.com/apache/trafficserver/pull/12593#discussion_r4396230772
##########
src/api/InkAPI.cc:
##########
@@ -5472,57 +5472,136 @@ 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);
Review Comment:
This lookup can return a strategy from the initial global `m_remap` table
while a global header_rewrite rule runs at `READ_REQUEST_HDR`.
`HttpSM::do_remap_request()` can then replace `m_remap` with a virtual-host
table, releasing the only transaction reference to the table that owns this
handle. A later remap reload can delete that factory while
`t_state.next_hop_strategy` still points into it, and parent selection
dereferences the pointer. The membership check in `TSHttpTxnNextHopStrategySet`
does not pin the factory or protect a handle already stored in the transaction;
retain the source rewrite table/factory for the transaction whenever a strategy
handle is installed, or defer the global lookup until the final remap table is
selected.
--
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]
