bryancall opened a new issue, #12969:
URL: https://github.com/apache/trafficserver/issues/12969
## Summary
`HttpSM::tunnel_handler_post` has the same class of bug fixed in #12959 — it
correctly handles VC events in its switch body but then falls through to a
narrow `ink_assert` that crashes on debug/ASAN builds.
Additionally, `state_common_wait_for_transform_read` is missing
`VC_EVENT_ACTIVE_TIMEOUT` with a `default: ink_release_assert(0)` that would
crash even in release builds.
## tunnel_handler_post (medium severity)
In `tunnel_handler_post` (HttpSM.cc ~line 2856), the switch correctly
handles `VC_EVENT_EOS`, `VC_EVENT_ERROR`, `VC_EVENT_WRITE_COMPLETE`,
`VC_EVENT_INACTIVITY_TIMEOUT`, and `VC_EVENT_ACTIVE_TIMEOUT` — the original
developer even wrote comments documenting when each arrives:
```cpp
case VC_EVENT_EOS: // SSLNetVC may callback EOS during
write error
case VC_EVENT_ERROR: // Send HTTP 408 error
case VC_EVENT_WRITE_COMPLETE: // tunnel_handler_post_ua has sent HTTP
408 response
case VC_EVENT_INACTIVITY_TIMEOUT: // _ua.get_txn() timeout during sending
the HTTP 408 response
case VC_EVENT_ACTIVE_TIMEOUT: // _ua.get_txn() timeout
if (_ua.get_entry()->write_buffer) {
free_MIOBuffer(_ua.get_entry()->write_buffer);
_ua.get_entry()->write_buffer = nullptr;
}
if (p->handler_state == static_cast<int>(HttpSmPost_t::UNKNOWN)) {
p->handler_state = static_cast<int>(HttpSmPost_t::UA_FAIL);
}
break; // <-- falls through to assertion below
```
After the switch, the code immediately hits:
```cpp
ink_assert(event == HTTP_TUNNEL_EVENT_DONE);
ink_assert(data == &tunnel);
```
When any of those VC events arrive, the assertion fires because `event` is
not `HTTP_TUNNEL_EVENT_DONE`. On ASAN/debug builds this is a fatal crash. On
release builds `ink_assert` is a no-op and the code proceeds correctly — the
`p_handler_state` switch handles `UA_FAIL` via `terminate_sm = true`.
### Fix
Change the `break` to `return 0` for the VC event cases. The handler state
is already set to `UA_FAIL`, and the tunnel will eventually fire
`HTTP_TUNNEL_EVENT_DONE` to complete cleanup. This avoids falling through to
the post-switch logic that assumes `HTTP_TUNNEL_EVENT_DONE`.
## state_common_wait_for_transform_read (low severity)
In `state_common_wait_for_transform_read` (HttpSM.cc ~line 1313), the switch
handles `VC_EVENT_ERROR`, `VC_EVENT_EOS`, and `VC_EVENT_INACTIVITY_TIMEOUT` but
is missing `VC_EVENT_ACTIVE_TIMEOUT`:
```cpp
case VC_EVENT_ERROR:
case VC_EVENT_EOS:
case VC_EVENT_INACTIVITY_TIMEOUT:
// cleanup logic...
break;
default:
ink_release_assert(0); // <-- crashes even in release builds
```
The `default: ink_release_assert(0)` means this would crash in ANY build
type if `VC_EVENT_ACTIVE_TIMEOUT` arrives. In practice, transform VCs aren't
monitored by InactivityCop, so this is unlikely to trigger — but it's a latent
crash if event routing ever changes.
### Fix
Add `VC_EVENT_ACTIVE_TIMEOUT` alongside `VC_EVENT_INACTIVITY_TIMEOUT`.
## Root Cause
Same as #12958 — the 2019 InactivityCop change (#5824) introduced
`VC_EVENT_ACTIVE_TIMEOUT` as a distinct event type, but not all handlers were
updated to accept it. The `tunnel_handler_post` assertion was also never
properly aligned with the VC events it handles in its switch body.
## Related
- #12958 — original crash report for `tunnel_handler`
- #12959 — fix for `tunnel_handler`
- #5824 — 2019 InactivityCop change that introduced `VC_EVENT_ACTIVE_TIMEOUT`
--
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]
