Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate header-setting options don't set multiple headers with different casing #21704

Open
huonw opened this issue Dec 2, 2024 · 0 comments
Labels
diagnostics Poor diagnostics, error messages that could be improved remote

Comments

@huonw
Copy link
Contributor

huonw commented Dec 2, 2024

Currently the https://www.pantsbuild.org/prerelease/reference/global-options#remote_execution_headers and https://www.pantsbuild.org/prerelease/reference/global-options#remote_store_headers options allow setting a single header multiple times, just differing by casing, and getting potentially non-deterministic behaviour.

For instance, both of these are setting the example header, but whether a or b is sent is likely to vary from run to run:

[GLOBAL.remote_store_headers]
EXAMPLE = "a"
example = "b"

We should validate the option only specifies a header with a single casing (i.e. emit an error like Found header `example` with multiple names: EXAMPLE, example. Choose one only one casing.), and normalise to lower case (this would allow simplifying the change in #21684)


         Should this code also deal with multiple key/value pairs of varying case for a single header?

I ask because on the Rust side in headers_to_http_header_map the conversion should non-deterministically merge them or the "last" one in iteration order will overwrite. However, on the Python side, it would probably be good to validate headers and ensure that only key/value pair exists per header, so that the user gets a good error message.

Doing that would also allow normalizing headers before then checking for user-agent.

Thoughts?

Originally posted by @tdyas in #21684 (comment)

@huonw huonw added remote diagnostics Poor diagnostics, error messages that could be improved labels Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostics Poor diagnostics, error messages that could be improved remote
Projects
None yet
Development

No branches or pull requests

1 participant