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

Returning http.DefaultClient can be harmful. #717

Open
seiyab opened this issue Apr 5, 2024 · 0 comments
Open

Returning http.DefaultClient can be harmful. #717

seiyab opened this issue Apr 5, 2024 · 0 comments

Comments

@seiyab
Copy link

seiyab commented Apr 5, 2024

Problem
Minimal reproduction of problem: https://go.dev/play/p/z-sitms7DpF
Returning http.DefaultClient, programmers can sometimes pollute global variable unintentionally. It's because http.DefaultClient is a pointer. Recently, a similar mistake caused a vulnerability https://pkg.go.dev/vuln/GO-2024-2618.

Description
Currently, function ContextClient() in internal package returns http.DefaultClient under some situation.

func ContextClient(ctx context.Context) *http.Client {
if ctx != nil {
if hc, ok := ctx.Value(HTTPClient).(*http.Client); ok {
return hc
}
}
return http.DefaultClient
}

Some functions that utilize internal.ContextClient(), like oauth2.NewClient(), can also return http.DefaultClient. Since http.DefautClient is a pointer, modifying returned default client cause change in global variable and possibly break entire application randomly.
I suggest returning a new instance &http.Client{}, that is same as a definition of http.DefaultClient.

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

No branches or pull requests

1 participant