diff --git a/CHANGELOG.md b/CHANGELOG.md index 958dc1ab..16a7d2ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## [Unreleased] +### Added +- Add [IMGPROXY_SOURCE_URL_QUERY_SEPARATOR](https://docs.imgproxy.net/latest/configuration/options#IMGPROXY_SOURCE_URL_QUERY_SEPARATOR) config. + ### Fixed - (pro) Fix object detecttion accuracy. diff --git a/config/config.go b/config/config.go index 3a6bb968..1e89f77a 100644 --- a/config/config.go +++ b/config/config.go @@ -105,6 +105,8 @@ var ( CookiePassthrough bool CookieBaseURL string + SourceURLQuerySeparator string + LocalFileSystemRoot string S3Enabled bool @@ -315,6 +317,7 @@ func Reset() { CookiePassthrough = false CookieBaseURL = "" + SourceURLQuerySeparator = "?" LocalFileSystemRoot = "" S3Enabled = false S3Region = "" @@ -558,6 +561,11 @@ func Configure() error { configurators.Bool(&CookiePassthrough, "IMGPROXY_COOKIE_PASSTHROUGH") configurators.String(&CookieBaseURL, "IMGPROXY_COOKIE_BASE_URL") + // Can't rely on configurators.String here because it ignores empty values + if s, ok := os.LookupEnv("IMGPROXY_SOURCE_URL_QUERY_SEPARATOR"); ok { + SourceURLQuerySeparator = s + } + configurators.String(&LocalFileSystemRoot, "IMGPROXY_LOCAL_FILESYSTEM_ROOT") configurators.Bool(&S3Enabled, "IMGPROXY_USE_S3") diff --git a/imagedata/download.go b/imagedata/download.go index 13217a04..43ffa780 100644 --- a/imagedata/download.go +++ b/imagedata/download.go @@ -16,6 +16,7 @@ import ( defaultTransport "github.com/imgproxy/imgproxy/v3/transport" azureTransport "github.com/imgproxy/imgproxy/v3/transport/azure" + transportCommon "github.com/imgproxy/imgproxy/v3/transport/common" fsTransport "github.com/imgproxy/imgproxy/v3/transport/fs" gcsTransport "github.com/imgproxy/imgproxy/v3/transport/gcs" s3Transport "github.com/imgproxy/imgproxy/v3/transport/s3" @@ -133,20 +134,7 @@ func headersToStore(res *http.Response) map[string]string { func BuildImageRequest(ctx context.Context, imageURL string, header http.Header, jar *cookiejar.Jar) (*http.Request, context.CancelFunc, error) { reqCtx, reqCancel := context.WithTimeout(ctx, time.Duration(config.DownloadTimeout)*time.Second) - // Non-http(s) URLs may contain percent symbol outside of the percent-encoded sequences. - // Parsing such URLs will fail with an error. - // To prevent this, we replace all percent symbols with %25. - // - // Also, such URLs may contain a hash symbol, which is a fragment identifier. - // We replace them with %23 to prevent cutting off the fragment part. - // Since we already replaced all percent symbols, we won't mix up %23 that were in the original URL - // and %23 that appeared after the replacement. - // - // We will revert these replacements in `transport/common.GetBucketAndKey`. - if !strings.HasPrefix(imageURL, "http://") && !strings.HasPrefix(imageURL, "https://") { - imageURL = strings.ReplaceAll(imageURL, "%", "%25") - imageURL = strings.ReplaceAll(imageURL, "#", "%23") - } + imageURL = transportCommon.EscapeURL(imageURL) req, err := http.NewRequestWithContext(reqCtx, "GET", imageURL, nil) if err != nil { diff --git a/transport/azure/azure.go b/transport/azure/azure.go index 250126ba..97b9c60b 100644 --- a/transport/azure/azure.go +++ b/transport/azure/azure.go @@ -84,7 +84,7 @@ func New() (http.RoundTripper, error) { } func (t transport) RoundTrip(req *http.Request) (*http.Response, error) { - container, key := common.GetBucketAndKey(req.URL) + container, key, _ := common.GetBucketAndKey(req.URL) if len(container) == 0 || len(key) == 0 { body := strings.NewReader("Invalid ABS URL: container name or object key is empty") diff --git a/transport/common/common.go b/transport/common/common.go index e0a41697..6ddf0ada 100644 --- a/transport/common/common.go +++ b/transport/common/common.go @@ -3,9 +3,32 @@ package common import ( "net/url" "strings" + + "github.com/imgproxy/imgproxy/v3/config" ) -func GetBucketAndKey(u *url.URL) (bucket, key string) { +func EscapeURL(u string) string { + // Non-http(s) URLs may contain percent symbol outside of the percent-encoded sequences. + // Parsing such URLs will fail with an error. + // To prevent this, we replace all percent symbols with %25. + // + // Also, such URLs may contain a hash symbol (a fragment identifier) or a question mark + // (a query string). + // We replace them with %23 and %3F to make `url.Parse` treat them as a part of the path. + // Since we already replaced all percent symbols, we won't mix up %23/%3F that were in the + // original URL and %23/%3F that appeared after the replacement. + // + // We will revert these replacements in `GetBucketAndKey`. + if !strings.HasPrefix(u, "http://") && !strings.HasPrefix(u, "https://") { + u = strings.ReplaceAll(u, "%", "%25") + u = strings.ReplaceAll(u, "?", "%3F") + u = strings.ReplaceAll(u, "#", "%23") + } + + return u +} + +func GetBucketAndKey(u *url.URL) (bucket, key, query string) { bucket = u.Host // We can't use u.Path here because `url.Parse` unescapes the original URL's path. @@ -21,16 +44,25 @@ func GetBucketAndKey(u *url.URL) (bucket, key string) { key = strings.TrimLeft(key, "/") - // We replaced all `%` with `%25` in `imagedata.BuildImageRequest` to prevent parsing errors. - // Also, we replaced all `#` with `%23` to prevent cutting off the fragment part. - // We need to revert these replacements. + // We percent-encoded `%`, `#`, and `?` in `EscapeURL` to prevent parsing errors. + // Now we need to revert these replacements. // - // It's important to revert %23 first because the original URL may also contain %23, - // and we don't want to mix them up. + // It's important to revert %25 last because %23/%3F may appear in the original URL and + // we don't want to mix them up. bucket = strings.ReplaceAll(bucket, "%23", "#") + bucket = strings.ReplaceAll(bucket, "%3F", "?") bucket = strings.ReplaceAll(bucket, "%25", "%") key = strings.ReplaceAll(key, "%23", "#") + key = strings.ReplaceAll(key, "%3F", "?") key = strings.ReplaceAll(key, "%25", "%") + // Cut the query string if it's present. + // Since we replaced `?` with `%3F` in `EscapeURL`, `url.Parse` will treat query + // string as a part of the path. + // Also, query string separator may be different from `?`, so we can't rely on `url.URL.RawQuery`. + if len(config.SourceURLQuerySeparator) > 0 { + key, query, _ = strings.Cut(key, config.SourceURLQuerySeparator) + } + return } diff --git a/transport/fs/fs.go b/transport/fs/fs.go index 34370771..99434a25 100644 --- a/transport/fs/fs.go +++ b/transport/fs/fs.go @@ -30,7 +30,7 @@ func New() transport { func (t transport) RoundTrip(req *http.Request) (resp *http.Response, err error) { header := make(http.Header) - _, path := common.GetBucketAndKey(req.URL) + _, path, _ := common.GetBucketAndKey(req.URL) path = "/" + path f, err := t.fs.Open(path) diff --git a/transport/gcs/gcs.go b/transport/gcs/gcs.go index b8c4e598..2988ee13 100644 --- a/transport/gcs/gcs.go +++ b/transport/gcs/gcs.go @@ -78,7 +78,7 @@ func New() (http.RoundTripper, error) { } func (t transport) RoundTrip(req *http.Request) (*http.Response, error) { - bucket, key := common.GetBucketAndKey(req.URL) + bucket, key, query := common.GetBucketAndKey(req.URL) if len(bucket) == 0 || len(key) == 0 { body := strings.NewReader("Invalid GCS URL: bucket name or object key is empty") @@ -98,7 +98,7 @@ func (t transport) RoundTrip(req *http.Request) (*http.Response, error) { bkt := t.client.Bucket(bucket) obj := bkt.Object(key) - if g, err := strconv.ParseInt(req.URL.RawQuery, 10, 64); err == nil && g > 0 { + if g, err := strconv.ParseInt(query, 10, 64); err == nil && g > 0 { obj = obj.Generation(g) } diff --git a/transport/s3/s3.go b/transport/s3/s3.go index 757a162b..90b05e36 100644 --- a/transport/s3/s3.go +++ b/transport/s3/s3.go @@ -103,7 +103,7 @@ func New() (http.RoundTripper, error) { } func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) { - bucket, key := common.GetBucketAndKey(req.URL) + bucket, key, query := common.GetBucketAndKey(req.URL) if len(bucket) == 0 || len(key) == 0 { body := strings.NewReader("Invalid S3 URL: bucket name or object key is empty") @@ -125,8 +125,8 @@ func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) { Key: aws.String(key), } - if len(req.URL.RawQuery) > 0 { - input.VersionId = aws.String(req.URL.RawQuery) + if len(query) > 0 { + input.VersionId = aws.String(query) } statusCode := http.StatusOK diff --git a/transport/swift/swift.go b/transport/swift/swift.go index a5f0317a..e2c26fe1 100644 --- a/transport/swift/swift.go +++ b/transport/swift/swift.go @@ -51,7 +51,7 @@ func New() (http.RoundTripper, error) { } func (t transport) RoundTrip(req *http.Request) (resp *http.Response, err error) { - container, objectName := common.GetBucketAndKey(req.URL) + container, objectName, _ := common.GetBucketAndKey(req.URL) if len(container) == 0 || len(objectName) == 0 { body := strings.NewReader("Invalid Swift URL: container name or object name is empty")