Add IMGPROXY_SOURCE_URL_QUERY_SEPARATOR config

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.
 

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")

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 {

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")

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
 }

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)

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)
 	}
 

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

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")