Skip to content

Quote query parameter keys#3441

Open
dfgvaetyj3456356-hash wants to merge 1 commit into
elastic:mainfrom
dfgvaetyj3456356-hash:fix/quote-query-param-keys
Open

Quote query parameter keys#3441
dfgvaetyj3456356-hash wants to merge 1 commit into
elastic:mainfrom
dfgvaetyj3456356-hash:fix/quote-query-param-keys

Conversation

@dfgvaetyj3456356-hash

Copy link
Copy Markdown
Contributor

Summary

  • URL-encode query parameter keys when building request query strings
  • add a regression test covering special characters in both keys and values

Why

_quote_query() already encoded parameter values, but left keys unchanged. When callers use the low-level params mapping, a key containing query delimiters such as & or = can change the shape of the final query string instead of being treated as one parameter name.

Testing

  • python -m pytest test_elasticsearch/test_client/test_utils.py -q

@cla-checker-service

cla-checker-service Bot commented May 31, 2026

Copy link
Copy Markdown

💚 CLA has been signed

@dfgvaetyj3456356-hash

Copy link
Copy Markdown
Contributor Author

I signed it using the GitHub account: dfgvaetyj3456356-hash.

@miguelgrinberg

Copy link
Copy Markdown
Contributor

What problem does this address? Why would you ever want to use query string parameter names that use forbidden characters, considering that Elasticsearch does not have any in its entire API surface?

@dfgvaetyj3456356-hash

Copy link
Copy Markdown
Contributor Author

Fair question. This is not about any known Elasticsearch API parameter name needing & or =.

The issue I was addressing is at the low-level client/helper boundary: callers can pass arbitrary params dicts, and before this change _quote_query() encoded values but left keys raw. That means a caller-supplied key containing query delimiters can change the serialized query-string shape instead of being treated as a literal key, e.g. a key like ilter_path&pretty is parsed as two parameters after serialization.

So the rationale is defensive consistency: if _quote_query() accepts a mapping and URL-encodes values, it should also URL-encode keys so the mapping round-trips to exactly the key/value pairs supplied by the caller. I agree this is not about Elastic's normal API surface using such names; if you consider unsupported parameter names outside the behavior you want to harden, I am fine with closing it.

@miguelgrinberg

Copy link
Copy Markdown
Contributor

Okay, I'll look at this when I have time, thanks.

@github-actions

Copy link
Copy Markdown

This pull request is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 2 days.

@github-actions github-actions Bot added the Stale label Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants