bryancall opened a new pull request, #13542:
URL: https://github.com/apache/trafficserver/pull/13542
`FeatureAPIHooks::operator[]` returns `nullptr` for an out of range hook id:
```c++
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:
```c++
_hooks = (*feature_hooks)[id];
_p = nullptr;
_c = _hooks->head();
```
`HttpSM::state_api_callout` has a `default:` case that sets the hook id to
-1 and then falls through to `HttpHookState::init`:
```c++
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. It calls `init` with an invalid id,
`operator[]` hands back `nullptr`, and the dereference follows.
`Scope::candidate` already tests `_hooks` for null before using it, so the
rest of the class is written to tolerate an empty scope. This change makes
`init` agree with `candidate`.
### Honest scope
This is a latent defect found by inspection while looking at an unrelated
crash. I am not claiming it explains any crash I have seen, and I do not have a
reproduction: it needs the `default:` case to be reached, which is asserted to
be unreachable. It is a cheap correctness fix that makes a release build fail
safe where today it dereferences null, and it removes a null dereference that
sits between an assertion that does not run and code that already expects null.
Happy to close it if the preference is to make the unreachable case explicit
some other way, for example by returning early rather than continuing with an
invalid id.
### Testing
Not yet built locally. Opened as a draft to let CI compile and run it first.
--
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]
