fix: Preserve percent-encoding in HttpRequest targets and query construction

[kinyoklion]

Summary

The shared network::HttpRequest constructor built the Beast request target from Boost.URL's path() and query(), both of which percent-decode. The asio/Beast backend sends that string verbatim as the request target, so every percent-encoded byte in a URL reached the wire decoded: a basis selector state containing CR LF split the request line, and &, space, # or % inside a value corrupted the query. The curl backend was not affected because it sends Url(), which is already encoded. The server SDK's FDv2 polling and streaming sources have sent the server-supplied basis state through this constructor since server v3.11.0, so the bug is live on the asio backend today.

The FDv1 sources appended their own parameter with a literal ?, so a base URL that already carried a query produced ...?token=x?withReasons=true (client poller) and ...?token=x?filter=key (server poller and server streaming source).

Contracts

Each URL helper now takes one kind of input and states it:

• HttpRequest(url, ...) takes a percent-encoded URL. Path() is the encoded request target, built from encoded_path() and encoded_query(). (Boost.URL's encoded_target(), which the SSE client uses, asserts on the 1.81 release after normalize_path(), so it is not used here.)
• AppendUrl(url, path) takes a raw path. / separates segments and every other character that is not allowed in a segment is encoded, so a % in the input becomes %25. It no longer tries to detect input that is already encoded. Every SDK call site passes a path constant or base64url text, so their output is unchanged.
• AppendQueryParam(url, key, value) (new) takes a raw key and value and encodes every character outside the unreserved set. The encoding is done explicitly because Boost.URL's params().append changed its output between releases (1.89 and later encode a space as +).
• The asio redirect handler was the only caller that passed already-encoded input to AppendUrl. It now resolves the Location header against the request URL with boost::urls::resolve, as the SSE client does. Relative locations resolve per RFC 3986 instead of being appended as a path suffix, and a Location that is not a valid URI reference fails the request.

Changes

• HttpRequest: normalize_path(), then the target is the encoded path plus ? and the encoded query when the query is not empty.
• AppendUrl: segment-based append through Boost.URL segments(), then normalize_path() for dot segments.
• AppendQueryParam: encode(..., unreserved_chars) plus encoded_params().append. Used by the client FDv1 poller (withReasons), the server FDv1 poller (filter), and the server FDv1 streaming source (filter).
• asio_requester.hpp: MakeRedirectRequest uses parse_uri_reference and resolve; IsAbsolute is removed.
• Tests
  • http_requester_test.cpp: encoded target, raw-path encoding (space, CR LF, %), base64url padding, URL encoding kept, query parameter encoding for every reserved character.
  • asio_requester_test.cpp (asio only): encoded query and path on the wire; relative, relative-without-slash, absolute and malformed Location handling through a loopback Beast server.
  • polling_data_source_request_test.cpp (client) and fdv1_source_request_test.cpp (server): the FDv1 poller and streaming source send &withReasons=true / &filter= when the base URL already has a query, checked at the wire.
  • fdv2_polling_impl_test.cpp: a selector state with CR LF, & and # stays encoded in Path().

Behavior changes

• Path() and Url() keep the percent-encoding the caller supplied. Path normalization still decodes escapes of characters that are legal in a path; the Boost 1.81 release used in CI also decodes %2F there (1.83 preserves it), which matches the previous behavior for that character.
• A % in the path passed to AppendUrl is encoded as %25. Previously an escape in that argument was kept.
• A redirect Location that is not a valid URI reference fails the request with "The request was malformed and could not be made." Previously it was sent with the % re-encoded.
• A user-configured base URL that carries its own query string keeps it, and the SDK's parameters are joined with &.

Follow-up

The FDv2 poll and stream request builders (fdv2_polling_impl.cpp, streaming_synchronizer.cpp) still add the basis state with params().append / set, whose output depends on the Boost.URL release (1.89 and later encode a space as +). This PR does not change them; moving them to AppendQueryParam would make the bytes identical on every release.

Verification

Linux GCC 13 Debug, system Boost 1.83: asio backend internal 283/283, client 184/184, server 596/596; curl backend internal 281/281, client 184/184, server 596/596. The six asio wire tests pass 20/20 shuffled repeats. The constructor, AppendUrl, AppendQueryParam and redirect logic were also compiled and run against the Boost.URL 1.81 headers with assertions enabled. The wire and constructor tests fail on main.

Prepared with Claude Code assistance.

---

Note

Overview
Fixes asio/Beast HTTP request targets so percent-encoded path and query bytes are sent on the wire unchanged. HttpRequest now builds Path() from encoded_path() and encoded_query() (only normalize_path() on the path), which stops decoded selector/query values (e.g. CR LF in FDv2 basis) from breaking the request line.

URL helpers are tightened: AppendUrl appends raw path segments via Boost.URL (encodes illegal segment chars; % in input becomes %25). New AppendQueryParam adds raw key/value with explicit unreserved encoding and & when the base URL already has a query. Client FDv1 polling (withReasons) and server FDv1 polling/streaming (filter) use it instead of appending ?…, fixing double-? URLs when the configured base already carries query params.

Redirects in asio_requester resolve Location with boost::urls::resolve (relative and absolute); invalid references fail the request instead of guessing.

Broad unit and loopback wire tests cover encoding preservation, query joining, redirects, and end-to-end poller/stream targets.

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

[beekld]

This isn't wrong, but I wanna push back on it a little bit. It feels like a very claude solution: rather than defining clear expectations at API boundaries, it's trying to kinda brute force these functions to work with all of their specific callers expectations. If we have to do it this way because of other constraints, that's fine. But I think it would be more maintainable if these URL APIs were explicit about whether their inputs are expected to be escaped or not, and we ensured callers conform to those contracts. Otherwise, I worry about more and more edge cases cropping up.

[beekld]

This feels error-prone to me. I mean, "characters not allowed in a path are percent-encoded" implies that if you put % in there, it would be encoded as %25. But this also says "Percent-encoding in the URL and in the appended path is preserved". So, it'll encode things except for percent signs? I generally think it's a good idea for APIs to either take an encoded string or a raw string, and be clear about it.

[kinyoklion]

I think it should be that it accepts non-encoded parameters and then they are encoded. As that is already what happens when it makes its way into the curl path. At least from the computed URLs I could see. Now when things happen at different points in time I could see needed internally to know that something was already encoded earlier. There aren't really any cases where we should be using anything that actually needs encoded.

[beekld]

So then can we simplify this by not having it try to detect and special case things that are already encoded?

[kinyoklion]

Yeah, I've not got back to another adjustment pass yet.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ 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 ed57865. Configure here.}

[cursor]

Empty path with query omits slash

Medium Severity

Switching from normalize() to normalize_path() drops the empty-path-to-/ rewrite that full normalization applied when an authority is present. encoded_target() then yields a query-only target, and MakeBeastRequest only substitutes / when Path() is completely empty, so a query-bearing empty path is sent as-is. That is not valid origin-form and can break follow-up requests after a Location that is host-plus-query with no path.

Additional Locations (1)

• libs/internal/include/launchdarkly/network/asio_requester.hpp#L78-L83

 

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