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

httpc: Fix percent-encoding of userinfo in URLs #8575

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

sliiser
Copy link
Contributor

@sliiser sliiser commented Jun 13, 2024

According to RFC3986 section-3.2.1, the valid characters for the userinfo component are as follows:

userinfo = *( unreserved / pct-encoded / sub-delims / ":" )

This does not include the "@" character, which must be percent-encoded when it appears in the userinfo component of a URL.

The Basic authentication scheme, as defined in RFC7617, does not restrict the use of any characters except for the colon (":") character in the user id. The colon should not be percent-encoded, it is just not a valid part of the user id.

When the userinfo component from the URL is converted into a Basic Authorization header, then the string is correctly validated, but is not decoded. This means that the percent-encoded characters end up in the Authorization header, which the servers are expected to interpet literally and not as percent-encoded. This results in user ids and passwords containing reserved characters to be misinterpreted by servers and rejected.

This commit ensures that the userinfo component is properly decoded before being used in the Basic Authorization header.

According to RFC3986 section-3.2.1, the valid characters for the
userinfo component are as follows:

  userinfo    = *( unreserved / pct-encoded / sub-delims / ":" )

This does not include the "@" character, which must be percent-encoded
when it appears in the userinfo component of a URL.

The Basic authentication scheme, as defined in RFC7617, does not
restrict the use of any characters except for the colon (":") character
in the user id. The colon should not be percent-encoded, it is just not
a valid part of the user id.

When the userinfo component from the URL is converted into a Basic
Authorization header, then the string is correctly validated, but is not
decoded. This means that the percent-encoded characters end up in the
Authorization header, which the servers are expected to interpet
literally and not as percent-encoded. This results in user ids and
passwords containing reserved characters to be misinterpreted by servers
and rejected.

This commit ensures that the userinfo component is properly decoded
before being used in the Basic Authorization header.
@CLAassistant
Copy link

CLAassistant commented Jun 13, 2024

CLA assistant check
All committers have signed the CLA.

@sliiser sliiser changed the base branch from master to maint June 13, 2024 12:39
Copy link
Contributor

github-actions bot commented Jun 13, 2024

CT Test Results

  2 files   22 suites   10m 22s ⏱️
349 tests 344 ✅  5 💤 0 ❌
619 runs  568 ✅ 51 💤 0 ❌

Results for commit a0a2b37.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@rickard-green rickard-green added the team:PS Assigned to OTP team PS label Jun 17, 2024
@IngelaAndin IngelaAndin added the Planned Focus issue added in sprint planning label Jun 19, 2024
@Whaileee Whaileee added the testing currently being tested, tag is used by OTP internal CI label Jul 10, 2024
@Whaileee Whaileee merged commit e535c5f into erlang:maint Jul 12, 2024
17 checks passed
@sliiser sliiser deleted the httpc/userinfo-percent-encoding-fix branch August 27, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Planned Focus issue added in sprint planning team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants