Skip to content

Fix append_query_params corrupting values with percent sequences#229

Open
phani69015 wants to merge 1 commit into
jpmorganchase:masterfrom
phani69015:fix/append-query-params-raw-value-encoding
Open

Fix append_query_params corrupting values with percent sequences#229
phani69015 wants to merge 1 commit into
jpmorganchase:masterfrom
phani69015:fix/append-query-params-raw-value-encoding

Conversation

@phani69015

Copy link
Copy Markdown

append_query_params called unquote() on every value before re-encoding, which assumed all values were already percent-encoded. Callers pass raw values, so this silently decoded them:

  • A file literally named 'a%2Bb.csv' was decoded to 'a+b.csv', colliding with a genuinely different file named 'a+b.csv' and producing the same download URL. Any value containing a percent sequence was corrupted.
  • Values were encoded with the default safe set, so '=' inside values was turned into '%3D', changing 'downloadRange=bytes=0-9' into 'bytes%3D0-9' on the range-download path.

Values are now treated as raw strings and percent-encoded exactly once, and '=' is kept in the safe set so range requests are transmitted as the API expects. The URL-encoding behaviour was introduced in v4.0.3 (#216).

Add regression tests for percent-sequence values, '+' vs '%2B' distinctness, '=' preservation in downloadRange, and query-separator encoding.

append_query_params called unquote() on every value before re-encoding,
which assumed all values were already percent-encoded. Callers pass raw
values, so this silently decoded them:

- A file literally named 'a%2Bb.csv' was decoded to 'a+b.csv', colliding
  with a genuinely different file named 'a+b.csv' and producing the same
  download URL. Any value containing a percent sequence was corrupted.
- Values were encoded with the default safe set, so '=' inside values was
  turned into '%3D', changing 'downloadRange=bytes=0-9' into
  'bytes%3D0-9' on the range-download path.

Values are now treated as raw strings and percent-encoded exactly once,
and '=' is kept in the safe set so range requests are transmitted as the
API expects. The URL-encoding behaviour was introduced in v4.0.3 (jpmorganchase#216).

Add regression tests for percent-sequence values, '+' vs '%2B'
distinctness, '=' preservation in downloadRange, and query-separator
encoding.
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