Remove context from MakeDirector

So that it is impossible to use an inappropriate context. We should use the context from the request.
authgear · Dec 9, 2024 · e06cc06 · e06cc06
1 parent f8650e2
commit e06cc06
Show file tree

Hide file tree

Showing 10 changed files with 23 additions and 23 deletions.
diff --git a/.vettedpositions b/.vettedpositions
@@ -269,13 +269,15 @@
 /pkg/images/deps/middleware_request.go:39:39: requestcontext
 /pkg/images/deps/middleware_request.go:40:34: requestcontext
 /pkg/images/deps/middleware_request.go:45:40: requestcontext
-/pkg/images/handler/get.go:95:43: requestcontext
 /pkg/images/handler/post.go:162:83: requestcontext
 /pkg/images/handler/post.go:193:10: requestcontext
 /pkg/lib/accountmanagement/rate_limit_middleware.go:41:10: requestcontext
 /pkg/lib/admin/authz/middleware.go:85:18: requestcontext
 /pkg/lib/authenticationflow/intl_middleware.go:16:41: requestcontext
 /pkg/lib/authenticationflow/rate_limit_middleware.go:31:49: requestcontext
+/pkg/lib/cloudstorage/azure.go:128:32: requestcontext
+/pkg/lib/cloudstorage/gcs.go:139:38: requestcontext
+/pkg/lib/cloudstorage/minio.go:100:41: requestcontext
 /pkg/lib/config/contextbackground.go:11:52: contextbackground
 /pkg/lib/deps/context.go:23:10: requestcontext
 /pkg/lib/deps/contextbackground.go:10:28: contextbackground

diff --git a/pkg/images/handler/get.go b/pkg/images/handler/get.go
@@ -2,7 +2,6 @@ package handler
 
 import (
 	"bytes"
-	"context"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -63,7 +62,7 @@ func ParseImageVariant(s string) (ImageVariant, bool) {
 }
 
 type DirectorMaker interface {
-	MakeDirector(ctx context.Context, extractKey func(*http.Request) string) func(*http.Request)
+	MakeDirector(extractKey func(*http.Request) string) func(*http.Request)
 }
 
 type GetHandler struct {
@@ -92,7 +91,7 @@ func (h *GetHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	director := h.DirectorMaker.MakeDirector(r.Context(), ExtractKey)
+	director := h.DirectorMaker.MakeDirector(ExtractKey)
 
 	reverseProxy := httputil.ReverseProxy{
 		Director: director,

diff --git a/pkg/images/handler/get_mock_test.go b/pkg/images/handler/get_mock_test.go
diff --git a/pkg/images/handler/get_test.go b/pkg/images/handler/get_test.go
@@ -37,7 +37,7 @@ func TestGetHandler(t *testing.T) {
 			r, _ := http.NewRequest("GET", "http://localhost:3004/_images/app/image.jpg/profile", nil)
 			w := httptest.NewRecorder()
 
-			directorMaker.EXPECT().MakeDirector(gomock.Any(), gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
+			directorMaker.EXPECT().MakeDirector(gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
 			gock.New("http://localhost:3004").Reply(404)
 
 			router.HTTPHandler().ServeHTTP(w, r)
@@ -50,7 +50,7 @@ func TestGetHandler(t *testing.T) {
 			r, _ := http.NewRequest("GET", "http://localhost:3004/_images/app/image.jpg/invalid", nil)
 			w := httptest.NewRecorder()
 
-			directorMaker.EXPECT().MakeDirector(gomock.Any(), gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
+			directorMaker.EXPECT().MakeDirector(gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
 
 			router.HTTPHandler().ServeHTTP(w, r)
 			So(w.Result().StatusCode, ShouldEqual, 404)
@@ -62,7 +62,7 @@ func TestGetHandler(t *testing.T) {
 			r, _ := http.NewRequest("GET", "http://localhost:3004/_images/app/image.jpg/profile", nil)
 			w := httptest.NewRecorder()
 
-			directorMaker.EXPECT().MakeDirector(gomock.Any(), gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
+			directorMaker.EXPECT().MakeDirector(gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
 			vipsDaemon.EXPECT().Process(gomock.Any()).Times(1).Return(&vipsutil.Output{
 				Data:          nil,
 				FileExtension: "",
@@ -81,7 +81,7 @@ func TestGetHandler(t *testing.T) {
 			r, _ := http.NewRequest("GET", "http://localhost:3004/_images/app/image.jpg/profile", nil)
 			w := httptest.NewRecorder()
 
-			directorMaker.EXPECT().MakeDirector(gomock.Any(), gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
+			directorMaker.EXPECT().MakeDirector(gomock.Any()).AnyTimes().Return(func(r *http.Request) {})
 			vipsDaemon.EXPECT().Process(gomock.Any()).Times(1).Return(&vipsutil.Output{
 				Data:          nil,
 				FileExtension: ".jpeg",

diff --git a/pkg/images/service/images_cloud_storage_service.go b/pkg/images/service/images_cloud_storage_service.go
@@ -20,7 +20,7 @@ const MaxContentLength = 10 * 1024 * 1024
 type ImagesCloudStorageServiceStorage interface {
 	PresignPutObject(ctx context.Context, name string, header http.Header) (*http.Request, error)
 	PresignHeadObject(ctx context.Context, name string, expire time.Duration) (*url.URL, error)
-	MakeDirector(ctx context.Context, extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request)
+	MakeDirector(extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request)
 }
 
 type ImagesCloudStorageServiceHTTPClient struct {
@@ -79,6 +79,6 @@ func (p *ImagesCloudStorageService) checkDuplicate(ctx context.Context, key stri
 	return apierrors.AlreadyExists.WithReason("DuplicatedImage").Errorf("duplicated image")
 }
 
-func (p *ImagesCloudStorageService) MakeDirector(ctx context.Context, extractKey func(r *http.Request) string) func(r *http.Request) {
-	return p.Storage.MakeDirector(ctx, extractKey, PresignGetExpires)
+func (p *ImagesCloudStorageService) MakeDirector(extractKey func(r *http.Request) string) func(r *http.Request) {
+	return p.Storage.MakeDirector(extractKey, PresignGetExpires)
 }
diff --git a/pkg/lib/cloudstorage/azure.go b/pkg/lib/cloudstorage/azure.go
@@ -122,10 +122,10 @@ func (s *AzureStorage) SignedURL(name string, now time.Time, duration time.Durat
 	return &u, nil
 }
 
-func (s *AzureStorage) MakeDirector(ctx context.Context, extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
+func (s *AzureStorage) MakeDirector(extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
 	return func(r *http.Request) {
 		key := extractKey(r)
-		u, err := s.PresignGetObject(ctx, key, expire)
+		u, err := s.PresignGetObject(r.Context(), key, expire)
 		if err != nil {
 			panic(err)
 		}

diff --git a/pkg/lib/cloudstorage/cloudstorage.go b/pkg/lib/cloudstorage/cloudstorage.go
@@ -20,5 +20,5 @@ type storage interface {
 	// PresignGetObject returns an URL that is ready for use.
 	PresignGetObject(ctx context.Context, name string, expire time.Duration) (*url.URL, error)
 	// MakeDirector takes extractKey and returns a Director of httputil.ReverseProxy.
-	MakeDirector(ctx context.Context, extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request)
+	MakeDirector(extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request)
 }
diff --git a/pkg/lib/cloudstorage/gcs.go b/pkg/lib/cloudstorage/gcs.go
@@ -133,10 +133,10 @@ func (s *GCSStorage) PresignGetObject(ctx context.Context, name string, expire t
 	return s.PresignGetOrHeadObject(ctx, name, "GET", expire)
 }
 
-func (s *GCSStorage) MakeDirector(ctx context.Context, extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
+func (s *GCSStorage) MakeDirector(extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
 	return func(r *http.Request) {
 		key := extractKey(r)
-		u, err := s.PresignGetOrHeadObject(ctx, key, "GET", expire)
+		u, err := s.PresignGetOrHeadObject(r.Context(), key, "GET", expire)
 		if err != nil {
 			panic(err)
 		}

diff --git a/pkg/lib/cloudstorage/minio.go b/pkg/lib/cloudstorage/minio.go
@@ -93,7 +93,7 @@ func (s *MinIOStorage) PresignGetObject(ctx context.Context, name string, expire
 	return u, nil
 }
 
-func (s *MinIOStorage) MakeDirector(ctx context.Context, extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
+func (s *MinIOStorage) MakeDirector(extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
 	return func(r *http.Request) {
 		key := extractKey(r)
 		params := url.Values{}

diff --git a/pkg/lib/cloudstorage/s3.go b/pkg/lib/cloudstorage/s3.go
@@ -126,7 +126,7 @@ func (s *S3Storage) PresignGetObject(ctx context.Context, name string, expire ti
 	return u, nil
 }
 
-func (s *S3Storage) MakeDirector(ctx context.Context, extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
+func (s *S3Storage) MakeDirector(extractKey func(r *http.Request) string, expire time.Duration) func(r *http.Request) {
 	return func(r *http.Request) {
 		key := extractKey(r)
 		input := &s3.GetObjectInput{