fix: use tagged switch and compose cleanup functions in registry push
Use tagged switch on r.Method in test handlers to resolve staticcheck QF1002. Compose user and image-source cleanup functions so WithImageCleanup is not silently overwritten by the default image source cleanup.
- commit
- edd6de6f323ba749ff46726c4b18798773cb21b0
- author
- brice · · 2026-03-15 06:04 UTC
- parent
- d2e89e6
- signature
- unsigned
2 files · +20 -14
M pkg/registrypush/push.go +12 -6
| @@ -125,7 +125,7 @@ func (p *Pusher) Push(ctx context.Context, ref string) error { | |||
| 125 | 125 | return fmt.Errorf("image ref %s must include a tag", ref) | |
| 126 | 126 | } | |
| 127 | 127 | ||
| 128 | - | cleanup := p.cleanupFn | |
| 128 | + | userCleanup := p.cleanupFn | |
| 129 | 129 | p.cleanupFn = nil | |
| 130 | 130 | ||
| 131 | 131 | authHeader, err := resolveAuthHeader(parsedRef) | |
| @@ -137,11 +137,17 @@ func (p *Pusher) Push(ctx context.Context, ref string) error { | |||
| 137 | 137 | if err != nil { | |
| 138 | 138 | return err | |
| 139 | 139 | } | |
| 140 | - | if p.cleanupFn != nil { | |
| 141 | - | cleanup = p.cleanupFn | |
| 142 | - | } | |
| 143 | - | if cleanup != nil { | |
| 144 | - | defer cleanup() | |
| 140 | + | sourceCleanup := p.cleanupFn | |
| 141 | + | switch { | |
| 142 | + | case sourceCleanup != nil && userCleanup != nil: | |
| 143 | + | defer func() { | |
| 144 | + | sourceCleanup() | |
| 145 | + | userCleanup() | |
| 146 | + | }() | |
| 147 | + | case sourceCleanup != nil: | |
| 148 | + | defer sourceCleanup() | |
| 149 | + | case userCleanup != nil: | |
| 150 | + | defer userCleanup() | |
| 145 | 151 | } | |
| 146 | 152 | ||
| 147 | 153 | baseURL := registryBaseURL(parsedRef.Context().RegistryStr()) | |
| @@ -125,7 +125,7 @@ func (p *Pusher) Push(ctx context.Context, ref string) error { | |||||
| 125 | return fmt.Errorf("image ref %s must include a tag", ref) | 125 | return fmt.Errorf("image ref %s must include a tag", ref) | ||
| 126 | } | 126 | } | ||
| 127 | 127 | ||||
| 128 | - | cleanup := p.cleanupFn | 128 | + | userCleanup := p.cleanupFn |
| 129 | p.cleanupFn = nil | 129 | p.cleanupFn = nil | ||
| 130 | 130 | ||||
| 131 | authHeader, err := resolveAuthHeader(parsedRef) | 131 | authHeader, err := resolveAuthHeader(parsedRef) | ||
| @@ -137,11 +137,17 @@ func (p *Pusher) Push(ctx context.Context, ref string) error { | |||||
| 137 | if err != nil { | 137 | if err != nil { | ||
| 138 | return err | 138 | return err | ||
| 139 | } | 139 | } | ||
| 140 | - | if p.cleanupFn != nil { | 140 | + | sourceCleanup := p.cleanupFn |
| 141 | - | cleanup = p.cleanupFn | 141 | + | switch { |
| 142 | - | } | 142 | + | case sourceCleanup != nil && userCleanup != nil: |
| 143 | - | if cleanup != nil { | 143 | + | defer func() { |
| 144 | - | defer cleanup() | 144 | + | sourceCleanup() |
| 145 | + | userCleanup() | |||
| 146 | + | }() | |||
| 147 | + | case sourceCleanup != nil: | |||
| 148 | + | defer sourceCleanup() | |||
| 149 | + | case userCleanup != nil: | |||
| 150 | + | defer userCleanup() | |||
| 145 | } | 151 | } | ||
| 146 | 152 | ||||
| 147 | baseURL := registryBaseURL(parsedRef.Context().RegistryStr()) | 153 | baseURL := registryBaseURL(parsedRef.Context().RegistryStr()) | ||
M pkg/registrypush/push_test.go +8 -8
| @@ -307,10 +307,10 @@ func TestUploadBlob_RejectsCrossOriginRedirect(t *testing.T) { | |||
| 307 | 307 | defer attacker.Close() | |
| 308 | 308 | ||
| 309 | 309 | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 310 | - | switch { | |
| 311 | - | case r.Method == http.MethodHead: | |
| 310 | + | switch r.Method { | |
| 311 | + | case http.MethodHead: | |
| 312 | 312 | w.WriteHeader(http.StatusNotFound) | |
| 313 | - | case r.Method == http.MethodPost: | |
| 313 | + | case http.MethodPost: | |
| 314 | 314 | w.Header().Set("Location", attacker.URL+"/v2/repo/blobs/uploads/evil") | |
| 315 | 315 | w.WriteHeader(http.StatusAccepted) | |
| 316 | 316 | default: | |
| @@ -348,16 +348,16 @@ func (t *authRecordingTransport) RoundTrip(req *http.Request) (*http.Response, e | |||
| 348 | 348 | ||
| 349 | 349 | func TestUploadBlob_AuthHeaderOnlySentToSameOrigin(t *testing.T) { | |
| 350 | 350 | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 351 | - | switch { | |
| 352 | - | case r.Method == http.MethodHead: | |
| 351 | + | switch r.Method { | |
| 352 | + | case http.MethodHead: | |
| 353 | 353 | w.WriteHeader(http.StatusNotFound) | |
| 354 | - | case r.Method == http.MethodPost: | |
| 354 | + | case http.MethodPost: | |
| 355 | 355 | w.Header().Set("Location", "/v2/repo/blobs/uploads/abc?chunk=0") | |
| 356 | 356 | w.WriteHeader(http.StatusAccepted) | |
| 357 | - | case r.Method == http.MethodPatch: | |
| 357 | + | case http.MethodPatch: | |
| 358 | 358 | w.Header().Set("Location", "/v2/repo/blobs/uploads/abc?chunk=1") | |
| 359 | 359 | w.WriteHeader(http.StatusAccepted) | |
| 360 | - | case r.Method == http.MethodPut: | |
| 360 | + | case http.MethodPut: | |
| 361 | 361 | w.WriteHeader(http.StatusCreated) | |
| 362 | 362 | default: | |
| 363 | 363 | w.WriteHeader(http.StatusInternalServerError) | |
| @@ -307,10 +307,10 @@ func TestUploadBlob_RejectsCrossOriginRedirect(t *testing.T) { | |||||
| 307 | defer attacker.Close() | 307 | defer attacker.Close() | ||
| 308 | 308 | ||||
| 309 | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 309 | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 310 | - | switch { | 310 | + | switch r.Method { |
| 311 | - | case r.Method == http.MethodHead: | 311 | + | case http.MethodHead: |
| 312 | w.WriteHeader(http.StatusNotFound) | 312 | w.WriteHeader(http.StatusNotFound) | ||
| 313 | - | case r.Method == http.MethodPost: | 313 | + | case http.MethodPost: |
| 314 | w.Header().Set("Location", attacker.URL+"/v2/repo/blobs/uploads/evil") | 314 | w.Header().Set("Location", attacker.URL+"/v2/repo/blobs/uploads/evil") | ||
| 315 | w.WriteHeader(http.StatusAccepted) | 315 | w.WriteHeader(http.StatusAccepted) | ||
| 316 | default: | 316 | default: | ||
| @@ -348,16 +348,16 @@ func (t *authRecordingTransport) RoundTrip(req *http.Request) (*http.Response, e | |||||
| 348 | 348 | ||||
| 349 | func TestUploadBlob_AuthHeaderOnlySentToSameOrigin(t *testing.T) { | 349 | func TestUploadBlob_AuthHeaderOnlySentToSameOrigin(t *testing.T) { | ||
| 350 | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 350 | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 351 | - | switch { | 351 | + | switch r.Method { |
| 352 | - | case r.Method == http.MethodHead: | 352 | + | case http.MethodHead: |
| 353 | w.WriteHeader(http.StatusNotFound) | 353 | w.WriteHeader(http.StatusNotFound) | ||
| 354 | - | case r.Method == http.MethodPost: | 354 | + | case http.MethodPost: |
| 355 | w.Header().Set("Location", "/v2/repo/blobs/uploads/abc?chunk=0") | 355 | w.Header().Set("Location", "/v2/repo/blobs/uploads/abc?chunk=0") | ||
| 356 | w.WriteHeader(http.StatusAccepted) | 356 | w.WriteHeader(http.StatusAccepted) | ||
| 357 | - | case r.Method == http.MethodPatch: | 357 | + | case http.MethodPatch: |
| 358 | w.Header().Set("Location", "/v2/repo/blobs/uploads/abc?chunk=1") | 358 | w.Header().Set("Location", "/v2/repo/blobs/uploads/abc?chunk=1") | ||
| 359 | w.WriteHeader(http.StatusAccepted) | 359 | w.WriteHeader(http.StatusAccepted) | ||
| 360 | - | case r.Method == http.MethodPut: | 360 | + | case http.MethodPut: |
| 361 | w.WriteHeader(http.StatusCreated) | 361 | w.WriteHeader(http.StatusCreated) | ||
| 362 | default: | 362 | default: | ||
| 363 | w.WriteHeader(http.StatusInternalServerError) | 363 | w.WriteHeader(http.StatusInternalServerError) | ||