From 7a25cf62a38fc26977f7c035a71addf9c634aa52 Mon Sep 17 00:00:00 2001 From: Andrew Harper Date: Fri, 2 Oct 2026 09:57:35 -0400 Subject: [PATCH] Add option to encode raw semicolons in query strings Go's URL parser rejects query pairs containing an unencoded semicolon, and ReverseProxy removes them from the outbound request. Clients that send values like ?a=1;2 lose the whole parameter without any error. When ENCODE_QUERY_SEMICOLONS is enabled, raw semicolons are encoded as %3B before caching and proxying, so they are passed upstream as part of the value and requests with different values get distinct cache keys. --- README.md | 1 + internal/config.go | 4 ++ internal/config_test.go | 3 + internal/handler.go | 2 + internal/query_semicolon_handler.go | 28 ++++++++ internal/query_semicolon_handler_test.go | 86 ++++++++++++++++++++++++ internal/service.go | 1 + 7 files changed, 125 insertions(+) create mode 100644 internal/query_semicolon_handler.go create mode 100644 internal/query_semicolon_handler_test.go diff --git a/README.md b/README.md index b13178e..6545478 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,7 @@ environment variables that you can set. | `EAB_HMAC_KEY` | The Base64-encoded EAB HMAC key to use when provisioning TLS certificates, if required. | None | | `FORWARD_HEADERS` | Whether to forward X-Forwarded-* and X-Request-ID headers from the client. | Disabled when running with TLS; enabled otherwise | | `LOG_REQUESTS` | Log all requests. Set to `0` or `false` to disable request logging | Enabled | +| `ENCODE_QUERY_SEMICOLONS` | Set to `1` or `true` to encode raw `;` in query strings as `%3B`, so they're passed upstream as part of the value. Otherwise Go drops any query parameter containing a raw `;`. | Disabled | | `DEBUG` | Set to `1` or `true` to enable debug logging. | Disabled | To prevent naming clashes with your application's own environment variables, diff --git a/internal/config.go b/internal/config.go index fd2887d..f890f0b 100644 --- a/internal/config.go +++ b/internal/config.go @@ -72,6 +72,8 @@ type Config struct { ForwardHeaders bool + EncodeQuerySemicolons bool + LogLevel slog.Level LogRequests bool } @@ -114,6 +116,8 @@ func NewConfig() (*Config, error) { H2CEnabled: getEnvBool("H2C_ENABLED", defaultH2CEnabled), + EncodeQuerySemicolons: getEnvBool("ENCODE_QUERY_SEMICOLONS", false), + LogLevel: logLevel, LogRequests: getEnvBool("LOG_REQUESTS", defaultLogRequests), } diff --git a/internal/config_test.go b/internal/config_test.go index 5dc1d9b..48ab7a2 100644 --- a/internal/config_test.go +++ b/internal/config_test.go @@ -106,6 +106,7 @@ func TestConfig_defaults(t *testing.T) { assert.Equal(t, defaultCacheSize, c.CacheSizeBytes) assert.Equal(t, slog.LevelInfo, c.LogLevel) assert.Equal(t, false, c.H2CEnabled) + assert.Equal(t, false, c.EncodeQuerySemicolons) } func TestConfig_override_defaults_with_env_vars(t *testing.T) { @@ -121,6 +122,7 @@ func TestConfig_override_defaults_with_env_vars(t *testing.T) { usingEnvVar(t, "H2C_ENABLED", "true") usingEnvVar(t, "GZIP_COMPRESSION_DISABLE_ON_AUTH", "true") usingEnvVar(t, "GZIP_COMPRESSION_JITTER", "64") + usingEnvVar(t, "ENCODE_QUERY_SEMICOLONS", "true") c, err := NewConfig() require.NoError(t, err) @@ -136,6 +138,7 @@ func TestConfig_override_defaults_with_env_vars(t *testing.T) { assert.Equal(t, true, c.H2CEnabled) assert.Equal(t, true, c.GzipCompressionDisableOnAuth) assert.Equal(t, 64, c.GzipCompressionJitter) + assert.Equal(t, true, c.EncodeQuerySemicolons) } func TestConfig_override_defaults_with_env_vars_using_prefix(t *testing.T) { diff --git a/internal/handler.go b/internal/handler.go index 61086dc..ae16e37 100644 --- a/internal/handler.go +++ b/internal/handler.go @@ -18,11 +18,13 @@ type HandlerOptions struct { gzipCompressionJitter int forwardHeaders bool logRequests bool + encodeQuerySemicolons bool } func NewHandler(options HandlerOptions) http.Handler { handler := NewProxyHandler(options.targetUrl, options.badGatewayPage, options.forwardHeaders) handler = NewCacheHandler(options.cache, options.maxCacheableResponseBody, handler) + handler = NewQuerySemicolonHandler(options.encodeQuerySemicolons, handler) handler = NewSendfileHandler(options.xSendfileEnabled, handler) handler = NewRequestStartHandler(handler) handler = NewRequestIDHandler(options.forwardHeaders, handler) diff --git a/internal/query_semicolon_handler.go b/internal/query_semicolon_handler.go new file mode 100644 index 0000000..b51da09 --- /dev/null +++ b/internal/query_semicolon_handler.go @@ -0,0 +1,28 @@ +package internal + +import ( + "net/http" + "strings" +) + +// Go drops query pairs containing a raw ";", so encode it to keep it as part of the value. +func NewQuerySemicolonHandler(enabled bool, next http.Handler) http.Handler { + if !enabled { + return next + } + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.RawQuery, ";") { + next.ServeHTTP(w, r) + return + } + + r2 := new(http.Request) + *r2 = *r + u := *r.URL + u.RawQuery = strings.ReplaceAll(r.URL.RawQuery, ";", "%3B") + r2.URL = &u + + next.ServeHTTP(w, r2) + }) +} diff --git a/internal/query_semicolon_handler_test.go b/internal/query_semicolon_handler_test.go new file mode 100644 index 0000000..b08a954 --- /dev/null +++ b/internal/query_semicolon_handler_test.go @@ -0,0 +1,86 @@ +package internal + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestQuerySemicolonHandler_encodes_semicolons_when_enabled(t *testing.T) { + tests := map[string]string{ + "/?a=1;2&b=3": "a=1%3B2&b=3", + "/?a=1%3B2&b=3": "a=1%3B2&b=3", + "/?a=1%3b2&b=3": "a=1%3b2&b=3", + "/?a=1;;2": "a=1%3B%3B2", + } + + for target, expected := range tests { + t.Run(target, func(t *testing.T) { + var rawQuery, value string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rawQuery = r.URL.RawQuery + value = r.URL.Query().Get("a") + })) + defer upstream.Close() + + options := handlerOptions(upstream.URL) + options.encodeQuerySemicolons = true + h := NewHandler(options) + + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest("GET", target, nil)) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, expected, rawQuery) + assert.Contains(t, value, ";") + }) + } +} + +func TestQuerySemicolonHandler_drops_semicolon_pairs_when_disabled(t *testing.T) { + var rawQuery string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rawQuery = r.URL.RawQuery + })) + defer upstream.Close() + + h := NewHandler(handlerOptions(upstream.URL)) + + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest("GET", "/?a=1;2&b=3", nil)) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "b=3", rawQuery) +} + +func TestQuerySemicolonHandler_keeps_cache_keys_distinct(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "public, max-age=60") + _, _ = w.Write([]byte(r.URL.Query().Get("a"))) + })) + defer upstream.Close() + + options := handlerOptions(upstream.URL) + options.encodeQuerySemicolons = true + h := NewHandler(options) + + serveRequest := func(target string) (string, string) { + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest("GET", target, nil)) + return w.Header().Get("X-Cache"), w.Body.String() + } + + cache, body := serveRequest("/?a=1;2") + assert.Equal(t, "miss", cache) + assert.Equal(t, "1;2", body) + + cache, body = serveRequest("/") + assert.Equal(t, "miss", cache) + assert.Equal(t, "", body) + + cache, body = serveRequest("/?a=1%3B2") + assert.Equal(t, "hit", cache) + assert.Equal(t, "1;2", body) +} diff --git a/internal/service.go b/internal/service.go index c34ba73..58e3e3d 100644 --- a/internal/service.go +++ b/internal/service.go @@ -30,6 +30,7 @@ func (s *Service) Run() int { logRequests: s.config.LogRequests, gzipCompressionDisableOnAuth: s.config.GzipCompressionDisableOnAuth, gzipCompressionJitter: s.config.GzipCompressionJitter, + encodeQuerySemicolons: s.config.EncodeQuerySemicolons, } handler := NewHandler(handlerOptions)