Skip to content

tunnel_handler_post assertion crashes on VC timeout/error events (same class as #12958) #12969

@bryancall

Description

@bryancall

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 below

After 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 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions