Skip to content

Commit 7b55e6b

Browse files
authored
Fixed the issue with downloader doesn't work with redirects #101 (#137)
* Fixed the issue with downloader doesn't work with redirects #101 * Coderabbit was mad * Configurable max reddirects! * Fix off-by-one error in redirect limit.
1 parent c7ee7a0 commit 7b55e6b

2 files changed

Lines changed: 81 additions & 8 deletions

File tree

config/config.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ type ApiConfiguration struct {
9393
// servers.
9494
DisableRemoteDownload bool `json:"-" yaml:"disable_remote_download"`
9595

96+
// RemoteDownload contains configuration for server remote download functionality.
97+
RemoteDownload struct {
98+
// MaxRedirects controls how many HTTP redirects are followed when performing a remote download.
99+
MaxRedirects int `default:"10" json:"max_redirects" yaml:"max_redirects"`
100+
} `json:"remote_download" yaml:"remote_download"`
101+
96102
// The maximum size for files uploaded through the Panel in MiB.
97103
UploadLimit int64 `default:"100" json:"upload_limit" yaml:"upload_limit"`
98104

router/downloader/downloader.go

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/goccy/go-json"
1818
"github.com/google/uuid"
1919

20+
"github.com/pelican-dev/wings/config"
2021
"github.com/pelican-dev/wings/server"
2122
)
2223

@@ -101,6 +102,8 @@ const (
101102
ErrDownloadFailed = errors.Sentinel("downloader: download request failed")
102103
)
103104

105+
const defaultMaxRedirects = 10
106+
104107
type Counter struct {
105108
total int
106109
onWrite func(total int)
@@ -184,17 +187,67 @@ func (dl *Download) Execute() error {
184187
// At this point we have verified the destination is not within the local network, so we can
185188
// now make a request to that URL and pull down the file, saving it to the server's data
186189
// directory.
187-
req, err := http.NewRequestWithContext(ctx, http.MethodGet, dl.req.URL.String(), nil)
188-
if err != nil {
189-
return errors.WrapIf(err, "downloader: failed to create request")
190+
currentURL := dl.req.URL
191+
if currentURL == nil {
192+
return errors.New("downloader: download request url is nil")
190193
}
191194

192-
req.Header.Set("User-Agent", "Pelican Panel (https://pelican.dev)")
193-
res, err := client.Do(req)
194-
if err != nil {
195-
return ErrDownloadFailed
195+
visited := make(map[string]struct{})
196+
var res *http.Response
197+
var finalURL *url.URL
198+
199+
maxRedirects := maxRedirectAttempts()
200+
for redirects := 0; redirects < maxRedirects; redirects++ {
201+
urlStr := currentURL.String()
202+
if _, seen := visited[urlStr]; seen {
203+
return errors.New("downloader: detected redirect loop")
204+
}
205+
visited[urlStr] = struct{}{}
206+
207+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, urlStr, nil)
208+
if err != nil {
209+
return errors.WrapIf(err, "downloader: failed to create request")
210+
}
211+
212+
req.Header.Set("User-Agent", "Pelican Panel (https://pelican.dev)")
213+
res, err = client.Do(req)
214+
if err != nil {
215+
return errors.WrapIf(err, "downloader: failed to perform request")
216+
}
217+
218+
if res.StatusCode >= http.StatusMultipleChoices && res.StatusCode < http.StatusBadRequest {
219+
location := res.Header.Get("Location")
220+
res.Body.Close()
221+
if location == "" {
222+
return errors.New("downloader: redirect response missing location header")
223+
}
224+
225+
nextURL, err := currentURL.Parse(location)
226+
if err != nil {
227+
return errors.WrapIf(err, "downloader: invalid redirect location")
228+
}
229+
if nextURL.Scheme != "http" && nextURL.Scheme != "https" {
230+
return errors.New("downloader: redirect to unsupported scheme")
231+
}
232+
233+
currentURL = nextURL
234+
finalURL = nextURL
235+
continue
236+
}
237+
238+
finalURL = currentURL
239+
break
240+
}
241+
242+
if res == nil {
243+
return errors.New("downloader: exceeded maximum redirect attempts")
196244
}
197245
defer res.Body.Close()
246+
247+
if res.StatusCode >= http.StatusMultipleChoices && res.StatusCode < http.StatusBadRequest {
248+
return errors.New("downloader: exceeded maximum redirect attempts")
249+
}
250+
198251
if res.StatusCode != http.StatusOK {
199252
return errors.New("downloader: got bad response status from endpoint: " + res.Status)
200253
}
@@ -219,7 +272,11 @@ func (dl *Download) Execute() error {
219272
if dl.req.FileName != "" {
220273
dl.path = dl.req.FileName
221274
} else {
222-
parts := strings.Split(dl.req.URL.Path, "/")
275+
pathSource := dl.req.URL
276+
if finalURL != nil {
277+
pathSource = finalURL
278+
}
279+
parts := strings.Split(pathSource.Path, "/")
223280
dl.path = parts[len(parts)-1]
224281
}
225282
}
@@ -335,3 +392,13 @@ func mustParseCIDR(ip string) *net.IPNet {
335392
}
336393
return block
337394
}
395+
396+
func maxRedirectAttempts() int {
397+
cfg := config.Get()
398+
if cfg != nil {
399+
if v := cfg.Api.RemoteDownload.MaxRedirects; v > 0 {
400+
return v
401+
}
402+
}
403+
return defaultMaxRedirects
404+
}

0 commit comments

Comments
 (0)