diff --git a/internal/handlers/image_signature_internal_test.go b/internal/handlers/image_signature_internal_test.go index 0ddecf2..babdb1e 100644 --- a/internal/handlers/image_signature_internal_test.go +++ b/internal/handlers/image_signature_internal_test.go @@ -262,3 +262,73 @@ func TestHandleImage_SourceQueryEncodedAnyWay(t *testing.T) { }) } } + +// TestHandleImage_SourceURLHoldingPercent25Verifies signs a source path and a +// source query that each hold %25, the percent-escape of a literal "%", and +// sends each to the route written as the README says, with that "%" written as +// %25, and as GenerateSignedURL writes it. A 502 means the signature passed. +func TestHandleImage_SourceURLHoldingPercent25Verifies(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + sourcePath string + sourceQuery string + // writtenPathAndQuery is the source path and query as the README says + // to write them into the image URL + writtenPathAndQuery string + }{ + {"path", "/sale/50%25off.jpg", "", "/sale/50%2525off.jpg"}, + {"query", photoPath, "discount=50%25", photoPath + "%3Fdiscount=50%2525"}, + } + + for _, tt := range tests { + t.Run(tt.name+" written by the README rule", func(t *testing.T) { + t.Parallel() + + expires := time.Now().Add(time.Hour) + signer := signature.New("test-signing-key-must-be-32-chars") + sig := signer.Sign(&signature.Request{ + SourceHost: signedHost, + SourcePath: tt.sourcePath, + SourceQuery: tt.sourceQuery, + Width: 50, + Height: 50, + Format: string(imgcache.FormatJPEG), + Quality: 85, + FitMode: string(imgcache.FitCover), + Expires: expires, + }) + + target := fmt.Sprintf("/v1/image/%s%s/50x50.jpeg?sig=%s&exp=%d", + signedHost, tt.writtenPathAndQuery, sig, expires.Unix()) + + status := getImage(t, setupTestHandler(t), target) + if status != http.StatusBadGateway { + t.Errorf("status = %d, want %d", status, http.StatusBadGateway) + } + }) + + t.Run(tt.name+" written by GenerateSignedURL", func(t *testing.T) { + t.Parallel() + + fix := setupTestHandler(t) + + signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{ + SourceHost: signedHost, + SourcePath: tt.sourcePath, + SourceQuery: tt.sourceQuery, + Size: imgcache.Size{Width: 50, Height: 50}, + Format: imgcache.FormatJPEG, + }, time.Hour) + if err != nil { + t.Fatalf("GenerateSignedURL() error = %v", err) + } + + status := getImage(t, fix, signedURL) + if status != http.StatusBadGateway { + t.Errorf("status = %d, want %d", status, http.StatusBadGateway) + } + }) + } +} diff --git a/internal/imgcache/urlparser_internal_test.go b/internal/imgcache/urlparser_internal_test.go index 518a04e..603d516 100644 --- a/internal/imgcache/urlparser_internal_test.go +++ b/internal/imgcache/urlparser_internal_test.go @@ -407,6 +407,71 @@ func TestParseImagePath_PathTraversal(t *testing.T) { } } +// TestParseImageURL_SourceURLHoldingPercent25 verifies that a source path and +// a source query holding %25, the percent-escape of a literal "%", are +// accepted as they are, although decoding them once more leaves a "%" that +// does not start a percent-escape. +func TestParseImageURL_SourceURLHoldingPercent25(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + want *ParsedURL + }{ + { + name: "path", + input: "/v1/image/cdn.example.com/sale/50%25off.jpg/800x600.webp", + want: &ParsedURL{ + Host: testHostCDN, Path: "/sale/50%25off.jpg", + Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + }, + }, + { + name: "query", + input: "/v1/image/cdn.example.com/photos/cat.jpg?discount=50%25/800x600.webp", + want: &ParsedURL{ + Host: testHostCDN, Path: testPathCat, Query: "discount=50%25", + Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := ParseImageURL(tt.input) + if err != nil { + t.Fatalf("ParseImageURL() unexpected error = %v", err) + } + + assertParsedURL(t, got, tt.want) + }) + } +} + +// TestParseImageURL_DoubleEncodedTraversalNextToPercent verifies that a +// double-encoded ".." is still refused when the path also holds a "%" that +// does not start a percent-escape, either as written or after one more +// decoding. +func TestParseImageURL_DoubleEncodedTraversalNextToPercent(t *testing.T) { + t.Parallel() + + for _, input := range []string{ + "/v1/image/cdn.example.com/photos/%252e%252e/secret/50%25off.jpg/800x600.jpeg", + "/v1/image/cdn.example.com/photos/%252e%252e/secret/100%/800x600.jpeg", + } { + _, err := ParseImageURL(input) + t.Logf("ParseImageURL(%q) error = %v", input, err) + + if !errors.Is(err, ErrPathTraversal) { + t.Errorf("ParseImageURL(%q) error = %v, want ErrPathTraversal", + input, err) + } + } +} + // errorIs checks if err matches target (handles wrapped errors). func errorIs(err, target error) bool { if errors.Is(err, target) {