Document how a source query goes into an image URL, and accept it (closes #255)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
The README gave no way to put a source URL's query into a signed image URL, and GenerateSignedURL built one that failed: the route parsed chi's wildcard, which keeps the path as the client wrote it, so one %2F, %3B or %2C left the %3F undecoded and the signature failed. The route now parses the decoded path, GenerateSignedURL writes Go's own encoding of it, and the README says the ? before the query, any ? in it and every % must be percent-encoded, with a signed example. The path traversal check no longer reads the query and accepts a literal %, while a double-encoded .. stays refused. A percent-escape in a source path is now decoded once. Model: opus-5-5
This commit was merged in pull request #281.
This commit is contained in:
@@ -47,6 +47,9 @@ func ValidateDimension(name string, value int) error {
|
||||
var sizeFormatRegex = regexp.MustCompile(
|
||||
`^(\d+)x(\d+)(?:\.(\w+))?$|^(orig)(?:\.(\w+))?$`)
|
||||
|
||||
// percentEscapeRegex matches one percent-escape, such as "%2e"
|
||||
var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`)
|
||||
|
||||
// ParsedURL contains the parsed components of an image proxy URL.
|
||||
type ParsedURL struct {
|
||||
// Host is the source origin host (e.g., "cdn.example.com")
|
||||
@@ -61,9 +64,8 @@ type ParsedURL struct {
|
||||
Format ImageFormat
|
||||
}
|
||||
|
||||
// ParseImagePath parses the path captured by chi's wildcard:
|
||||
// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL
|
||||
// This is the primary entry point when using chi routing.
|
||||
// ParseImagePath parses an image URL's path after /v1/image/, decoded once:
|
||||
// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL.
|
||||
// Examples:
|
||||
// - cdn.example.com/photos/cat.jpg/800x600.webp
|
||||
// - cdn.example.com/photos/cat.jpg/0x0.jpeg
|
||||
@@ -79,9 +81,10 @@ func ParseImagePath(path string) (*ParsedURL, error) {
|
||||
return parseImageComponents(path)
|
||||
}
|
||||
|
||||
// ParseImageURL parses a full URL path like /v1/image/<host>/<path>/<size>.<format>,
|
||||
// or /v1/image/<host>/<path>/<size> for JPEG XL
|
||||
// Use ParseImagePath instead when working with chi's wildcard capture.
|
||||
// ParseImageURL parses an image URL's path, decoded once as r.URL.Path holds
|
||||
// it: /v1/image/<host>/<path>/<size>.<format>, or /v1/image/<host>/<path>/<size>
|
||||
// for JPEG XL. A source query follows the source path after a "?", which the
|
||||
// URL writes as %3F.
|
||||
func ParseImageURL(urlPath string) (*ParsedURL, error) {
|
||||
// Remove the /v1/image/ prefix
|
||||
const prefix = "/v1/image/"
|
||||
@@ -100,12 +103,6 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) {
|
||||
// parseImageComponents parses <host>/<path>/<size>.<format>, or
|
||||
// <host>/<path>/<size> for JPEG XL.
|
||||
func parseImageComponents(remainder string) (*ParsedURL, error) {
|
||||
// Check for path traversal before any other processing
|
||||
err := checkPathTraversal(remainder)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Find the last path segment, which holds "size" or "size.format"
|
||||
lastSlash := strings.LastIndex(remainder, "/")
|
||||
if lastSlash == -1 {
|
||||
@@ -161,6 +158,13 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
|
||||
path = path[:qIndex]
|
||||
}
|
||||
|
||||
// The source query names no file on the upstream, so only the host and
|
||||
// the source path are checked for path traversal
|
||||
err = checkPathTraversal(host + path)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Ensure path starts with /
|
||||
if !strings.HasPrefix(path, "/") {
|
||||
path = "/" + path
|
||||
@@ -186,12 +190,7 @@ func checkPathTraversal(path string) error {
|
||||
// Decode multiple times to catch double-encoding
|
||||
decoded := path
|
||||
for range 3 {
|
||||
newDecoded, err := url.PathUnescape(decoded)
|
||||
if err != nil {
|
||||
// Malformed encoding is suspicious
|
||||
return ErrPathTraversal
|
||||
}
|
||||
|
||||
newDecoded := decodePercentEscapes(decoded)
|
||||
if newDecoded == decoded {
|
||||
break
|
||||
}
|
||||
@@ -232,6 +231,18 @@ func checkPathTraversal(path string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// decodePercentEscapes decodes every percent-escape in s, such as %2e, and
|
||||
// keeps a "%" that does not start one as it is, where url.PathUnescape would
|
||||
// refuse the whole string. A source path holding %25, a literal "%", has such
|
||||
// a "%" after one more decoding.
|
||||
func decodePercentEscapes(s string) string {
|
||||
return percentEscapeRegex.ReplaceAllStringFunc(s, func(escape string) string {
|
||||
decoded, _ := url.PathUnescape(escape)
|
||||
|
||||
return decoded
|
||||
})
|
||||
}
|
||||
|
||||
// parseSizeFormat parses strings like "800x600.webp", "orig.png" or "800x600"
|
||||
func parseSizeFormat(s string) (Size, ImageFormat, error) {
|
||||
matches := sizeFormatRegex.FindStringSubmatch(s)
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user