Skip to content

Commit 32087de

Browse files
committed
Fix set-body at remap and escalate plugin interaction with internal_msg_buffer
In EndRemapRequest, the previous fix unconditionally cleared internal_msg_buffer in the else branch (remap success path). This destroyed the body set by set-body at remap hooks when no set-status was used, causing ATS to contact the origin instead of serving the synthetic response. Fix: Only clear internal_msg_buffer when a plugin tunnel is active (the original intent - clearing stale error bodies from build_error_response during remap failures for plugin tunnels). Also clear internal_msg_buffer in redirect_request() to prevent stale error bodies from build_error_response (e.g., connection failures) from being mistaken for plugin-set synthetic bodies by how_to_open_connection() during redirect/retry flows like the escalate plugin.
1 parent 281ecf8 commit 32087de

2 files changed

Lines changed: 17 additions & 3 deletions

File tree

src/proxy/http/HttpSM.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8558,6 +8558,10 @@ HttpSM::redirect_request(const char *arg_redirect_url, const int arg_redirect_le
85588558
// XXX - doing a destroy() for now, we can do a fileds_clear() if we have performance issue
85598559
t_state.hdr_info.client_response.destroy();
85608560
}
8561+
// Clear any error body from a previous failed connection attempt (e.g., from
8562+
// build_error_response) so that how_to_open_connection() does not mistake it
8563+
// for a plugin-set synthetic body and short-circuit the retry.
8564+
t_state.free_internal_msg_buffer();
85618565

85628566
int scheme = t_state.next_hop_scheme;
85638567
int scheme_len = hdrtoken_index_to_length(scheme);

src/proxy/http/HttpTransact.cc

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,9 +1233,19 @@ HttpTransact::EndRemapRequest(State *s)
12331233
Metrics::Counter::increment(http_rsb.invalid_client_requests);
12341234
TRANSACT_RETURN(StateMachineAction_t::SEND_ERROR_CACHE_NOOP, nullptr);
12351235
} else {
1236-
s->hdr_info.client_response.destroy(); // release the underlying memory.
1237-
s->hdr_info.client_response.clear(); // clear the pointers.
1238-
s->free_internal_msg_buffer(); // clear error body so plugin tunnel is not bypassed.
1236+
// This else branch handles two cases:
1237+
// 1. Remap succeeded (reverse_proxy == true) - normal request processing
1238+
// 2. Remap failed but plugin tunnel exists - plugin overrides error
1239+
//
1240+
// For case 2, clear the stale error response that build_error_response()
1241+
// created during remap failure, because the plugin tunnel will provide
1242+
// its own response. For case 1, preserve any plugin-set internal_msg_buffer
1243+
// (e.g., from set-body at remap) so how_to_open_connection() can short-circuit.
1244+
if (s->state_machine->plugin_tunnel_type != HttpPluginTunnel_t::NONE) {
1245+
s->hdr_info.client_response.destroy(); // release the underlying memory.
1246+
s->hdr_info.client_response.clear(); // clear the pointers.
1247+
s->free_internal_msg_buffer(); // clear error body so plugin tunnel is not bypassed.
1248+
}
12391249
TxnDbg(dbg_ctl_http_trans, "END HttpTransact::EndRemapRequest");
12401250
12411251
if (s->is_upgrade_request && s->post_remap_upgrade_return_point) {

0 commit comments

Comments
 (0)