Guard against a null hook list in HttpHookState::Scope::init

[bryancall]

FeatureAPIHooks::operator[] returns nullptr for an out of range hook id:

template <typename ID, int N>
APIHooks const *
FeatureAPIHooks<ID, N>::operator[](ID id) const
{
  return likely(is_valid(id)) ? &(m_hooks[id]) : nullptr;
}

but HttpHookState::Scope::init dereferenced the result without testing it:

_hooks = (*feature_hooks)[id];

_p = nullptr;
_c = _hooks->head();

HttpSM::do_api_callout_internal has a default: case that sets the hook id to -1 and then falls through to HttpHookState::init:

default:
  cur_hook_id = static_cast<TSHttpHookID>(-1);
  ink_assert(!"not reached");
}

hook_state.init(cur_hook_id, http_global_hooks, ...);

ink_assert compiles out in a release build, so a release binary that reaches that case does not stop there. It calls init with an invalid id, operator[] hands back nullptr, and the dereference follows. ProxySession::do_api_callout has the same debug-only-assert-then-init shape, so the guard covers two call sites.

What this does and does not fix

To be precise about the benefit: this does not make a release build survive that path. With the scope left empty, getNext() returns nullptr, state_api_callout takes the no-hooks path, and handle_api_return switches on the same api_next_action that fell through in the first place, landing on default: ink_release_assert(!"Not reached");. The process still aborts a few frames later.

What it does deliver is removing the undefined behavior: a null dereference in Scope::init becomes a deterministic, named abort that points at the actual bad state. That is a better crash to triage, and it makes Scope::init honor the nullptr contract operator[] already documents.

Scope

This is defense in depth, not a fix for an observed crash. StateMachineAction_t has 13 relevant values and the switch handles all of them, so the default: is unreachable given non-corrupt state. I have no reproduction, and I am not claiming this explains any crash I have seen.

A reasonable alternative is to make the failure explicit at its source instead, by promoting the ink_assert in HttpSM::do_api_callout_internal to ink_release_assert. That would make the invariant local rather than depending on two switch case lists staying in sync. Happy to go that way instead if maintainers prefer it.

Testing

Built with the dev-asan preset (-fsanitize=address,undefined) on Fedora 44 / gcc 16, -Werror clean.

• unit (ctest): 174/174 passed
• in-binary regression (traffic_server -R 1): 67 passed, REGRESSION_TEST DONE: PASSED
• autest: 476 passed / 65 failed / 40 skipped, over 561 tests (remap_acl and remap_acl_yaml excluded for runtime)

The 65 autest failures are pre-existing environment failures on this box, not regressions. I built unmodified master at the same base commit (8aebe2c706) and ran the identical 561-test selection: it produces the same 476/65/40 and the same 61 distinct failing tests, so the failing set matches the baseline exactly. They are concentrated in TLS, QUIC/HTTP3, and config-reload tests.