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

Exotic http lib leak not handled by goth #65

Closed
dmoklaf opened this issue Mar 8, 2016 · 4 comments
Closed

Exotic http lib leak not handled by goth #65

dmoklaf opened this issue Mar 8, 2016 · 4 comments

Comments

@dmoklaf
Copy link
Contributor

dmoklaf commented Mar 8, 2016

The http lib has an exotic case of connection leak which is not taken care of by goth :
if for some reasons (e.g., ongoing production incident) the target authentication URL is redirecting too many times, the golang http client might return a non-nil response object together with a non-nil error.

This is an under-documented weakness of the Golang version 1 libraries, left for compatibility reasons, causing connection leak in many projects.

Consequently, when checking for an http client error, the goth code should also check if the response is non-nil, and if non-nil close its body.

@markbates
Copy link
Owner

Please submit a PR, I'd happily merge it.

@rakesh-eltropy
Copy link
Collaborator

@rgeronimi It is not fixed for all the providers. We still have same issue in dropbox, box, digital ocean, lastfm, linkedin, salesforce, spotify, twitch, twitter and yammer providers.

@dmoklaf
Copy link
Contributor Author

dmoklaf commented Mar 9, 2016

Ah thanks, I hacked a simple shell script to search for such errors in my dependencies, and I missed the http Do pattern.
Fixed now, submitting a second PR.

@rakesh-eltropy
Copy link
Collaborator

@rgeronimi can you please close this.

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

3 participants