refactor: add SSE Builder::on_response hook

[beekld]

Summary

Adds an on_response hook to sse::Builder, invoked once per (re)connect after the HTTP response headers have been received, before any branching on status. Fires for every response (any status, including redirects and errors); does not fire if the connection failed before any response (TCP error, read timeout).

Prep for upcoming FDv2 work that needs to inspect the X-LD-FD-Fallback response header.

Design notes

cURL backend accumulates header lines into a per-transfer http::response_header<> on RequestContext, because HeaderCallback is invoked once per CRLF-terminated line. The hook fires on the empty terminator and is posted to the client strand.

Test plan

• gtest_launchdarkly-sse-client — green under both Foxy and cURL backends
• New ClientTest.OnResponseHookFiresWithCustomHeader

---

Note

Low Risk
Additive API and hook timing only; existing connect/error/backoff paths unchanged aside from cURL header parsing refactor covered by tests.

Overview
Adds Builder::on_response, a callback that runs once per connect/reconnect after response headers are complete and before status-based handling (success, redirect, errors). It receives http::response_header<> and is documented to run on the client executor without blocking; it does not run when no HTTP response was received (e.g. TCP failure or read timeout).

Foxy invokes the hook from on_headers when parsing finishes. cURL gains header-line parsing into current_response (status line, OWS-trimmed fields, emit on blank line) so the same hook can fire per response in a redirect chain, with delivery posted to the client strand.

A new test asserts status 200 and case-insensitive read of X-LD-FD-Fallback—supporting upcoming FDv2 fallback signaling without changing stream behavior yet.

^{Reviewed by Cursor Bugbot for commit 0659239. Bugbot is set up for automated code reviews on this repo. Configure here.}

[kinyoklion]

H1 — response_header<>::result(unsigned) throws on code > 999; thrown across the libcurl C frame → UB (minimal fix; skip if applying the comprehensive M1+M3 fix below).

Beast message.hpp:372: @throws std::invalid_argument if v > 999.

HeaderCallback runs inside curl_multi_perform, a C frame. Throwing a C++ exception across a C frame is undefined behavior — in practice the process terminates. A server (or transparent proxy with naive re-stamping) emitting HTTP/1.1 1234 ... is enough to crash the SDK: from_chars parses 4 ASCII digits cleanly, the code != 0 guard passes, and result(1234) throws.

Tightening the guard to the valid HTTP status range (RFC 7231 §6: status codes are three-digit) avoids the throw.

Suggested change

	if (result.ec == std::errc{} && code != 0) {
	if (result.ec == std::errc{} && code >= 100 && code <= 999) {

[kinyoklion]

H3 — set() collapses duplicate-name headers; introduces a silent backend asymmetry (minimal fix; skip if applying the comprehensive M1+M3 fix below).

Beast's basic_fields::set() semantics: "Set a field value, removing any other instances of that field." For headers that legitimately repeat — Set-Cookie (RFC 6265 §3 forbids merging), Via, Warning, Link, Proxy-Authenticate, WWW-Authenticate, list-valued headers sent as separate lines — only the LAST value survives in current_response.

The Foxy backend uses Beast's parser (insert() semantics) and preserves all duplicates. Same server response → different response_header<> delivered to the hook depending on which backend is compiled. Today's hook caller is FDv2 reading a single-valued X-LD-FD-Fallback, but the public ResponseHook signature gives callers the full response_header<> and the contract makes no statement about lossy collapsing.

Suggested change

	context->current_response.set(std::string(name), std::string(value));
	context->current_response.insert(std::string(name), std::string(value));

[kinyoklion]

M1 + M3 — parser state flag (companion to the cpp suggestion below). (Comprehensive fix path — apply this with the M1+M3 cpp suggestion. Skip if applying the minimal H1/H2/H3 one-liners above.)

M1 (chunked trailers fire a phantom second hook) and M3 (interior HTTP/ line resets accumulated state) share a root cause: HeaderCallback has no concept of "mid-response vs between responses." A single state bit on RequestContext is enough to gate both.

Suggested change

	http::response_header<> current_response;
	http::response_header<> current_response;
	// True while accumulating headers between an `HTTP/` status line and
	// the empty terminator. Gates `HeaderCallback` against chunked
	// trailers (which arrive without a fresh status line) and against
	// interior `HTTP/` lines that would otherwise wipe accumulated state.
	bool reading_headers = false;

[kinyoklion]

M1 + M3 — comprehensive HeaderCallback rewrite (bundles H1, H2, H3, M1, M3, and L1/§2.2 tolerance). (Comprehensive fix path — apply this with the hpp suggestion above. Skip if applying the minimal H1/H2/H3 one-liners individually; the line ranges conflict.)

This introduces a single reading_headers state bit and gates the three branches on it:

• M1 (trailers) — Per libcurl's CURLOPT_HEADERFUNCTION docs, the callback is invoked for chunked trailers and their terminator line. Without a state flag, the trailer block accumulates into a freshly-reset current_response (status 200 by Beast default) and fires the hook a second time. Gating both the terminator and the trailer headers on reading_headers suppresses the phantom fire.

• M3 (interior HTTP/ reset) — A non-leading HTTP/... line currently wipes accumulated state. The Foxy backend doesn't have this issue (Beast rejects interior status lines as a protocol error); aligning the curl backend closes the gap.

• L1 / RFC 9112 §2.2 tolerance — The existing strip at lines 431-434 only removes paired CRLF, so a bare-LF or bare-CR terminator leaves a stray byte in the value. Verified against libcurl source: for HTTP/1.x, libcurl forwards the original wire bytes to HEADERFUNCTION (lib/http.c:4456,4490-4492), so a non-compliant origin can deliver bare LF here. Only HTTP/2 is normalized (lib/http2.c:1532-1565 synthesizes CRLF). A permissive while loop at the top of the suggestion handles bare LF, bare CR, and CRLF uniformly per RFC 9112 §2.2.

• H1, H2, H3 are folded in inline.

Manual follow-up after applying this suggestion: the original 4-line CRLF strip at lines 431-434 (just above this suggestion's range) becomes dead code — the new while loop subsumes it. Delete those lines:

    if (line.size() >= 2 && line[line.size() - 2] == '\r' &&
        line.back() == '\n') {
        line.remove_suffix(2);
    }

Suggested change

	if (line.empty()) {
	// End-of-headers terminator: emit and reset.
	context->response(std::move(context->current_response));
	context->current_response = http::response_header<>{};
	return total_size;
	}
	if (line.substr(0, 5) == "HTTP/") {
	// Status line: "HTTP/X.Y CODE REASON". Start a fresh response.
	context->current_response = http::response_header<>{};
	auto const code_start = line.find(' ');
	if (code_start != std::string_view::npos) {
	unsigned code = 0;
	auto const result = std::from_chars(
	line.data() + code_start + 1, line.data() + line.size(), code);
	if (result.ec == std::errc{} && code != 0) {
	context->current_response.result(code);
	}
	}
	return total_size;
	}
	auto const colon = line.find(':');
	if (colon != std::string_view::npos) {
	std::string_view name = line.substr(0, colon);
	// HTTP optional whitespace (OWS) per RFC 7230 §3.2.3 is SP or HTAB.
	std::string_view value = line.substr(colon + 1);
	while (!value.empty() &&
	(value.front() == ' ' || value.front() == '\t')) {
	value.remove_prefix(1);
	}
	while (!value.empty() &&
	(value.back() == ' ' || value.back() == '\t')) {
	value.remove_suffix(1);
	}
	context->current_response.set(std::string(name), std::string(value));
	if (beast::iequals(name, "Content-Type") &&
	value.find("text/event-stream") == std::string_view::npos) {
	context->log_message("warning: unexpected Content-Type: " +
	std::string(line));
	}
	}
	// Strip the line terminator. Allow bare LF or bare CR per RFC 9112 §2.2;
	// libcurl preserves the original wire bytes for HTTP/1.x (only HTTP/2
	// synthesizes CRLF), so a non-compliant origin can deliver bare LF here.
	while (!line.empty() &&
	(line.back() == '\r' || line.back() == '\n')) {
	line.remove_suffix(1);
	}
	if (line.empty()) {
	// Terminator. If we're between responses (e.g., the line ends a
	// chunked-transfer trailer block), there's nothing to emit.
	if (context->reading_headers) {
	context->response(std::move(context->current_response));
	context->current_response = http::response_header<>{};
	context->reading_headers = false;
	}
	return total_size;
	}
	if (line.substr(0, 5) == "HTTP/") {
	// Status line: "HTTP/X.Y CODE REASON". Only legitimate before any
	// header has been seen for this response — an interior HTTP/ line
	// would otherwise wipe accumulated state.
	if (context->reading_headers) {
	return total_size;
	}
	// Beast default-constructs result_ to status::ok (200); reset to 0
	// so an unparseable status line surfaces as result_int() == 0.
	context->current_response = http::response_header<>{};
	context->current_response.result(0);
	auto const code_start = line.find(' ');
	if (code_start != std::string_view::npos) {
	unsigned code = 0;
	auto const result = std::from_chars(
	line.data() + code_start + 1, line.data() + line.size(), code);
	// Three-digit status per RFC 7231 §6; the tight bound avoids
	// result(unsigned) throwing across the libcurl C frame.
	if (result.ec == std::errc{} && code >= 100 && code <= 999) {
	context->current_response.result(code);
	}
	}
	context->reading_headers = true;
	return total_size;
	}
	if (!context->reading_headers) {
	// Header line received outside an active response — chunked trailer
	// or protocol-level junk. Ignore.
	return total_size;
	}
	auto const colon = line.find(':');
	if (colon != std::string_view::npos) {
	std::string_view name = line.substr(0, colon);
	// HTTP optional whitespace (OWS) per RFC 7230 §3.2.3 is SP or HTAB.
	std::string_view value = line.substr(colon + 1);
	while (!value.empty() &&
	(value.front() == ' ' || value.front() == '\t')) {
	value.remove_prefix(1);
	}
	while (!value.empty() &&
	(value.back() == ' ' || value.back() == '\t')) {
	value.remove_suffix(1);
	}
	// insert() preserves duplicate-name headers (Set-Cookie, Via, …);
	// set() would collapse them and diverge from the Foxy backend.
	context->current_response.insert(std::string(name), std::string(value));
	if (beast::iequals(name, "Content-Type") &&
	value.find("text/event-stream") == std::string_view::npos) {
	context->log_message("warning: unexpected Content-Type: " +
	std::string(line));
	}
	}

[kinyoklion]

I have concerns with this working with RFC 9112 2.2:

Although the line terminator for the start-line and fields is the sequence CRLF, a recipient MAY recognize a single LF as a line terminator and ignore any preceding CR.

The beast code does apply this lenience, but I am checking now if maybe curl normalizes this already. If not it will be in the blob below.

[kinyoklion]

Looks like CURL would only normalize it for HTTP/2 where it synthesizes http/1 lines.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 0659239. Configure here.}

[cursor]

Status code >999 throws exception across C frame causing UB

High Severity

context->current_response.result(code) is called without checking code <= 999. Beast's result(unsigned) throws std::invalid_argument when the value exceeds 999. Since HeaderCallback executes inside libcurl's C frame (curl_multi_perform), a C++ exception propagating through C code is undefined behavior — typically resulting in process termination. A misbehaving proxy or server emitting a 4-digit status code triggers this.

 

^{Reviewed by Cursor Bugbot for commit 0659239. Configure here.}

[cursor]

Chunked trailers fire phantom second on_response hook invocation

Medium Severity

libcurl's CURLOPT_HEADERFUNCTION is also invoked for HTTP chunked trailers (header lines after the body, terminated by an empty line). When the empty terminator after trailers arrives, line.empty() is true and context->response() fires again with only the trailer fields. This violates the documented "invoked once per (re)connect" contract and sends callers a spurious on_response callback with incorrect/incomplete header data.

 

^{Reviewed by Cursor Bugbot for commit 0659239. Configure here.}

[cursor]

Header set() silently discards duplicate headers unlike Foxy backend

Low Severity

Using set() on current_response replaces any previous value for a given header name, meaning only the last occurrence of a repeated header survives. The Foxy backend's response.base() preserves all duplicate headers (Beast's parser uses insert() semantics). This creates a silent backend asymmetry where on_response hook callers receive different data depending on the backend, with the CURL path losing legitimate multi-valued headers like Set-Cookie.

 

^{Reviewed by Cursor Bugbot for commit 0659239. Configure here.}