Skip to content

Escape feature and gate keys in HTTP adapter requests#1009

Open
jnunemaker wants to merge 4 commits into
mainfrom
escape-http-adapter-keys
Open

Escape feature and gate keys in HTTP adapter requests#1009
jnunemaker wants to merge 4 commits into
mainfrom
escape-http-adapter-keys

Conversation

@jnunemaker

Copy link
Copy Markdown
Collaborator

Summary

The HTTP adapter interpolated feature and gate keys raw into request paths and query strings ("/features/#{feature.key}", "/features?keys=#{csv_keys}"), so a key containing /, ?, &, #, a comma, or a space produced a malformed request or injected/overrode query parameters — and a key with a space raised URI::InvalidURIError outright.

This escapes each key with URI.encode_www_form_component at every interpolation site (get, get_multi, remove, enable, disable, clear); the API server already decodes keys with Rack::Utils.unescape, so the round-trip is preserved. It also fixes a spurious leading & in Client#uri_for_path when the base URL has no query string.

Added failing-first specs for #get (path escaping) and #get_multi (query escaping); the full HTTP adapter suite (108 examples, including the end-to-end WEBrick + Flipper::Api shared adapter specs) passes.

jnunemaker and others added 4 commits July 8, 2026 00:01
The HTTP adapter interpolated feature/gate keys raw into request paths
and query strings (e.g. "/features/#{feature.key}", "/features?keys=#{csv_keys}").
A key containing /, ?, &, #, a comma, or a space produced a malformed
request or injected/overrode query parameters — and a key with a space
raised URI::InvalidURIError outright.

Escape each key with URI.encode_www_form_component at every interpolation
site (get, get_multi, remove, enable, disable, clear). The API server
already decodes keys with Rack::Utils.unescape, so the round-trip is
preserved. Also fix a spurious leading "&" in Client#uri_for_path when the
base URL has no query string.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant