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

Incorrect warning about using sensitive credentials with HTTP when HTTPS is used #970

Closed
anna-oake opened this issue Feb 5, 2025 · 2 comments · Fixed by #972
Closed
Assignees
Labels
bug v3 For resty v3

Comments

@anna-oake
Copy link

A code like this would produce a warning saying Using sensitive credentials in HTTP mode is not secure. Use HTTPS:

resty.New().
    SetBaseURL("https://example.com").
    SetAuthToken("example")

This behaviour is incorrect, because the base URL clearly starts with the secure https protocol scheme.

The behaviour was introduced in the commit 3b97332 with this piece of code:

resty/middleware.go

Lines 318 to 322 in 3b97332

if !c.IsDisableWarn() && credentialsAdded {
if strings.HasPrefix(r.URL, "http") {
r.log.Warnf("Using sensitive credentials in HTTP mode is not secure. Use HTTPS")
}
}

Any URL string starting with https:// would indeed technically have an http prefix, so the warning is logged regardless.

This could be fixed by either changing the statement to something inversed like:

if !strings.HasPrefix(r.URL, "https")

Or, if done properly, by checking the parsed protocol scheme. Something along the lines of:

if r.RawRequest.URL.Scheme == "http"

Thank you!

@anna-oake
Copy link
Author

Just realised the warning is actually produced when a request is made, not when Resty is instantiated.

This doesn't change the issue though.

@jeevatkm jeevatkm self-assigned this Feb 6, 2025
@jeevatkm
Copy link
Member

jeevatkm commented Feb 6, 2025

@anna-oake Thanks for trying out v3 beta. I will analyze and fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug v3 For resty v3
Development

Successfully merging a pull request may close this issue.

2 participants