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

support signing empty param values, e.g. 's.q=' #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

magibney
Copy link

As long as an empty value for a param (e.g., s.q=) is legal, we should support signing it. Current implementation simply drops params with empty values, which without reporting back to the caller, results in invalid auth headers. This should also be supported because in practice, sometimes having a parameter key present with an empty value can differ semantically from having no value explicitly specified for that parameter.

I believe this implementation is backward-compatible, and should support different ways of handling blank and nil params.

As long as an empty value for a param (e.g., s.q) is legal,
we should support signing it. Current implementation simply
drops params with empty values, which without reporting back
to the caller, results in invalid auth headers. This should
also be supported because in practice, sometimes having a parameter
key present with an empty value can differ semantically from
having no value explicitly specified for that parameter.
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