-
Notifications
You must be signed in to change notification settings - Fork 226
RequestAsync support basic auth #105
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
base: master
Are you sure you want to change the base?
RequestAsync support basic auth #105
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for HTTP Basic Authentication by extracting credentials from the URI's UserInfo component and converting them into a proper Authorization header, resolving an issue where the builder returns URLs not supported by the HTTP client.
Key Changes:
- Extracts username and password from URI's UserInfo property
- Converts credentials to Base64-encoded Authorization header
- Strips credentials from the URI before creating the HTTP request
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| headers ??= new Dictionary<string, string>(); | ||
| headers.TryAdd( "Authorization", $"Basic {Convert.ToBase64String( bytes )}" ); | ||
|
|
||
| uri = new Uri( requestUri.Replace( $"{userInfo}@", "" ), UriKind.Absolute ); |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using string replacement to remove credentials from the URI is fragile and could fail if the userInfo contains special regex characters or appears elsewhere in the URL. Use UriBuilder to properly reconstruct the URI without credentials by setting its UserName and Password properties to empty strings.
| uri = new Uri( requestUri.Replace( $"{userInfo}@", "" ), UriKind.Absolute ); | |
| var uriBuilder = new UriBuilder( uri ) | |
| { | |
| UserName = string.Empty, | |
| Password = string.Empty | |
| }; | |
| uri = uriBuilder.Uri; |
| var bytes = System.Text.Encoding.UTF8.GetBytes( userInfo ); | ||
|
|
||
| headers ??= new Dictionary<string, string>(); | ||
| headers.TryAdd( "Authorization", $"Basic {Convert.ToBase64String( bytes )}" ); |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TryAdd will silently fail if an Authorization header already exists, potentially causing authentication to fail unexpectedly. Consider either throwing an exception if the header already exists, or explicitly overwriting it with the credentials from the URI to ensure predictable behavior.
| headers.TryAdd( "Authorization", $"Basic {Convert.ToBase64String( bytes )}" ); | |
| headers["Authorization"] = $"Basic {Convert.ToBase64String( bytes )}"; |
Using the above code, the builder returns a URL that is not supported by the HTTP client. You must use the Authorisation header to be able to use authentication.
Resolves Facepunch/sbox-issues#5798