From 4542f86ad79abc04a00772a12d61d3e008f8ac25 Mon Sep 17 00:00:00 2001 From: LexLuthr Date: Thu, 26 Dec 2024 14:19:39 +0400 Subject: [PATCH 1/2] allow 2 redirects for http client --- transport/httptransport/http_transport.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/transport/httptransport/http_transport.go b/transport/httptransport/http_transport.go index 0716aca1e..9d8db46e6 100644 --- a/transport/httptransport/http_transport.go +++ b/transport/httptransport/http_transport.go @@ -210,8 +210,12 @@ func (h *httpTransport) Execute(ctx context.Context, transportInfo []byte, dealI } else { // do not follow http redirects for security reasons t.client = &http.Client{ + // Custom CheckRedirect function to limit redirects CheckRedirect: func(req *http.Request, via []*http.Request) error { - return http.ErrUseLastResponse + if len(via) >= 2 { // Limit to 2 redirects + return fmt.Errorf("too many redirects: %d", len(via)) + } + return nil }, } From 3442484b60462e4bc54d8db5747d52b5098bcdba Mon Sep 17 00:00:00 2001 From: LexLuthr Date: Thu, 26 Dec 2024 14:48:08 +0400 Subject: [PATCH 2/2] update test and reponse --- transport/httptransport/http_transport.go | 2 +- transport/httptransport/http_transport_test.go | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/transport/httptransport/http_transport.go b/transport/httptransport/http_transport.go index 9d8db46e6..32a91c3a6 100644 --- a/transport/httptransport/http_transport.go +++ b/transport/httptransport/http_transport.go @@ -213,7 +213,7 @@ func (h *httpTransport) Execute(ctx context.Context, transportInfo []byte, dealI // Custom CheckRedirect function to limit redirects CheckRedirect: func(req *http.Request, via []*http.Request) error { if len(via) >= 2 { // Limit to 2 redirects - return fmt.Errorf("too many redirects: %d", len(via)) + return http.ErrUseLastResponse } return nil }, diff --git a/transport/httptransport/http_transport_test.go b/transport/httptransport/http_transport_test.go index 240fad8a7..5799fe495 100644 --- a/transport/httptransport/http_transport_test.go +++ b/transport/httptransport/http_transport_test.go @@ -387,8 +387,8 @@ func TestDownloadFromPrivateIPs(t *testing.T) { } func TestDontFollowHttpRedirects(t *testing.T) { - // we should not follow http redirects for security reasons. If the target URL tries to redirect, the client should return 303 response instead. - // This test sets up two servers, with one redirecting to the other. Without the redirect check the download would have been completed successfully. + // we should not follow more than 2 http redirects for security reasons. If the target URL tries to redirect, the client should return 303 response instead. + // This test sets up 3 servers, with one redirecting to the other. Without the redirect check the download would have been completed successfully. rawSize := (100 * readBufferSize) + 30 ctx := context.Background() st := newServerTest(t, rawSize) @@ -422,8 +422,14 @@ func TestDontFollowHttpRedirects(t *testing.T) { redirectSvr := httptest.NewServer(redirectHandler) defer redirectSvr.Close() + var redirectHandler1 http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, redirectSvr.URL, http.StatusSeeOther) + } + redirectSvr1 := httptest.NewServer(redirectHandler1) + defer redirectSvr1.Close() + of := getTempFilePath(t) - th := executeTransfer(t, ctx, New(nil, newDealLogger(t, ctx), NChunksOpt(numChunks)), carSize, types.HttpRequest{URL: redirectSvr.URL}, of) + th := executeTransfer(t, ctx, New(nil, newDealLogger(t, ctx), NChunksOpt(numChunks)), carSize, types.HttpRequest{URL: redirectSvr1.URL}, of) require.NotNil(t, th) evts := waitForTransferComplete(th)