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

nhttp: Support unquoted Authorization header parameters #3665

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

the-ress
Copy link
Contributor

@the-ress the-ress commented Jan 6, 2024

Problem

When authenticating using the Digest scheme, some HTTP clients (e.g. httpx) send the algorithm parameter even when it was not explicitly specified by the server in the WWW-Authenticate header. The value is being sent without quotes as required by RFC 7616. This trips up the parsing which only expects quoted parameters and the server return error 400.

Example header:

Authorization: Digest username="maker", realm="Printer API", nonce="[redacted]", uri="/api/version", response="[redacted]", algorithm=MD5

Additionally, I noticed the scheme part (Digest) wasn't consumed separately and got attached to the first param instead. In a header like this, with nonce as the first parameter, its name would be understood as Digest nonce instead of just nonce and the value would get ignored.

Authorization: Digest nonce="860bd2ef00017e3a", ...

Changes

  1. Unquoted parameters are now supported (both known and unknown).
  2. The initial Digest scheme at the start is consumed before parsing the parameters. The actual value isn't checked, following the existing pattern. (We only really read nonce and response, the rest is assumed for performance reasons.)
  3. The separator -> read_par transition is now fallthrough so it doesn't eat the first char of the value (", first char of an unquoted value, etc.)

Copy link
Contributor

@vorner vorner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just this little nitpick, but thank you for discovering this.

value = auto.add_state()
quote = auto.add_state()
value_quoted = auto.add_state()
value_quoted = auto.add_state()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need two value_quoted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's a typo I missed. I removed it.

@the-ress the-ress force-pushed the unquoted-authorization-parameters branch from e8fba38 to 5c5b3c3 Compare January 11, 2024 09:29
@vorner vorner merged commit fda58ec into prusa3d:master Jan 15, 2024
1 check passed
@vorner
Copy link
Contributor

vorner commented Jan 15, 2024

Thank you. I expect this'll get released in the next non-patch release (5.2, not 5.1.3), though I can't guarantee it.

@the-ress
Copy link
Contributor Author

thanks

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.

None yet

2 participants