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:
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 belowAfter the switch, the code immediately hits:
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:
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 buildsThe 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
Summary
HttpSM::tunnel_handler_posthas the same class of bug fixed in #12959 — it correctly handles VC events in its switch body but then falls through to a narrowink_assertthat crashes on debug/ASAN builds.Additionally,
state_common_wait_for_transform_readis missingVC_EVENT_ACTIVE_TIMEOUTwith adefault: 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 handlesVC_EVENT_EOS,VC_EVENT_ERROR,VC_EVENT_WRITE_COMPLETE,VC_EVENT_INACTIVITY_TIMEOUT, andVC_EVENT_ACTIVE_TIMEOUT— the original developer even wrote comments documenting when each arrives:After the switch, the code immediately hits:
When any of those VC events arrive, the assertion fires because
eventis notHTTP_TUNNEL_EVENT_DONE. On ASAN/debug builds this is a fatal crash. On release buildsink_assertis a no-op and the code proceeds correctly — thep_handler_stateswitch handlesUA_FAILviaterminate_sm = true.Fix
Change the
breaktoreturn 0for the VC event cases. The handler state is already set toUA_FAIL, and the tunnel will eventually fireHTTP_TUNNEL_EVENT_DONEto complete cleanup. This avoids falling through to the post-switch logic that assumesHTTP_TUNNEL_EVENT_DONE.state_common_wait_for_transform_read (low severity)
In
state_common_wait_for_transform_read(HttpSM.cc ~line 1313), the switch handlesVC_EVENT_ERROR,VC_EVENT_EOS, andVC_EVENT_INACTIVITY_TIMEOUTbut is missingVC_EVENT_ACTIVE_TIMEOUT:The
default: ink_release_assert(0)means this would crash in ANY build type ifVC_EVENT_ACTIVE_TIMEOUTarrives. 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_TIMEOUTalongsideVC_EVENT_INACTIVITY_TIMEOUT.Root Cause
Same as #12958 — the 2019 InactivityCop change (#5824) introduced
VC_EVENT_ACTIVE_TIMEOUTas a distinct event type, but not all handlers were updated to accept it. Thetunnel_handler_postassertion was also never properly aligned with the VC events it handles in its switch body.Related
tunnel_handlertunnel_handlerVC_EVENT_ACTIVE_TIMEOUT