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

Follow redirects on pull endpoint #149

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mrxbox98
Copy link
Contributor

This adds following pull requests on the pull endpoint. I don't have that much experience in Go so someone should probably check to make sure I didn't break anything.

@@ -163,6 +163,17 @@ func (dl *Download) Execute() error {
return ErrDownloadFailed
}
defer res.Body.Close()

//If the http code is a redirect get the location and set the url to that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note the security implications of this (noted at the start):

// Disallow any redirect on an HTTP call. This is a security requirement: do not modify
// this logic without first ensuring that the new target location IS NOT within the current
// instance's local network.
//
// This specific error response just causes the client to not follow the redirect and
// returns the actual redirect response to the caller. Not perfect, but simple and most
// people won't be using URLs that redirect anyways hopefully?
//
// We'll re-evaluate this down the road if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it uses recursion and the execute function first checks if the URL is in the local network it should throw an error if its in the local network.

if res.StatusCode == http.StatusMovedPermanently || res.StatusCode == http.StatusFound {
redirect, redirectError := url.Parse(res.Header.Get("Location"))
if redirectError != nil {
return errors.New("downloader: redirect specified without loication")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You misspelled "location" btw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants