From e62fe64ec931e06942f85f384b0d475cf44fe18c Mon Sep 17 00:00:00 2001 From: LandryBe Date: Thu, 15 Jun 2023 18:20:06 +0200 Subject: [PATCH] Encode query semicolons Co-authored-by: Romain --- .../reference/static-configuration/cli-ref.md | 3 + .../reference/static-configuration/env-ref.md | 3 + .../reference/static-configuration/file.toml | 1 + .../reference/static-configuration/file.yaml | 1 + docs/content/routing/entrypoints.md | 38 +++++++++++ .../fixtures/simple_encode_semicolons.toml | 32 +++++++++ integration/simple_test.go | 68 +++++++++++++++++++ pkg/config/static/entrypoints.go | 7 +- pkg/server/server_entrypoint_tcp.go | 26 ++++++- pkg/server/service/proxy.go | 1 + 10 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 integration/fixtures/simple_encode_semicolons.toml diff --git a/docs/content/reference/static-configuration/cli-ref.md b/docs/content/reference/static-configuration/cli-ref.md index 6b8537a7d..e9385b08b 100644 --- a/docs/content/reference/static-configuration/cli-ref.md +++ b/docs/content/reference/static-configuration/cli-ref.md @@ -114,6 +114,9 @@ Trust only forwarded headers from selected IPs. `--entrypoints..http`: HTTP configuration. +`--entrypoints..http.encodequerysemicolons`: +Defines whether request query semicolons should be URLEncoded. (Default: ```false```) + `--entrypoints..http.middlewares`: Default middlewares for the routers linked to the entry point. diff --git a/docs/content/reference/static-configuration/env-ref.md b/docs/content/reference/static-configuration/env-ref.md index f4b2b6def..184c4202b 100644 --- a/docs/content/reference/static-configuration/env-ref.md +++ b/docs/content/reference/static-configuration/env-ref.md @@ -123,6 +123,9 @@ HTTP/3 configuration. (Default: ```false```) `TRAEFIK_ENTRYPOINTS__HTTP3_ADVERTISEDPORT`: UDP port to advertise, on which HTTP/3 is available. (Default: ```0```) +`TRAEFIK_ENTRYPOINTS__HTTP_ENCODEQUERYSEMICOLONS`: +Defines whether request query semicolons should be URLEncoded. (Default: ```false```) + `TRAEFIK_ENTRYPOINTS__HTTP_MIDDLEWARES`: Default middlewares for the routers linked to the entry point. diff --git a/docs/content/reference/static-configuration/file.toml b/docs/content/reference/static-configuration/file.toml index 7d96d87f4..ca95739c9 100644 --- a/docs/content/reference/static-configuration/file.toml +++ b/docs/content/reference/static-configuration/file.toml @@ -30,6 +30,7 @@ trustedIPs = ["foobar", "foobar"] [entryPoints.EntryPoint0.http] middlewares = ["foobar", "foobar"] + encodeQuerySemicolons = true [entryPoints.EntryPoint0.http.redirections] [entryPoints.EntryPoint0.http.redirections.entryPoint] to = "foobar" diff --git a/docs/content/reference/static-configuration/file.yaml b/docs/content/reference/static-configuration/file.yaml index 435efb1ce..c3b6e830c 100644 --- a/docs/content/reference/static-configuration/file.yaml +++ b/docs/content/reference/static-configuration/file.yaml @@ -33,6 +33,7 @@ entryPoints: - foobar - foobar http: + encodeQuerySemicolons: true redirections: entryPoint: to: foobar diff --git a/docs/content/routing/entrypoints.md b/docs/content/routing/entrypoints.md index f20c0b456..305e69cd1 100644 --- a/docs/content/routing/entrypoints.md +++ b/docs/content/routing/entrypoints.md @@ -833,6 +833,44 @@ This section is a convenience to enable (permanent) redirecting of all incoming --entrypoints.foo.http.redirections.entrypoint.priority=10 ``` +### EncodeQuerySemicolons + +_Optional, Default=false_ + +The `encodeQuerySemicolons` option allows to enable query semicolons encoding. +One could use this option to avoid non-encoded semicolons to be interpreted as query parameter separators by Traefik. +When using this option, the non-encoded semicolons characters in query will be transmitted encoded to the backend. + +```yaml tab="File (YAML)" +entryPoints: + websecure: + address: ':443' + http: + encodeQuerySemicolons: true +``` + +```toml tab="File (TOML)" +[entryPoints.websecure] + address = ":443" + + [entryPoints.websecure.http] + encodeQuerySemicolons = true +``` + +```bash tab="CLI" +--entrypoints.websecure.address=:443 +--entrypoints.websecure.http.encodequerysemicolons=true +``` + +#### Examples + +| EncodeQuerySemicolons | Request Query | Resulting Request Query | +|-----------------------|---------------------|-------------------------| +| false | foo=bar;baz=bar | foo=bar&baz=bar | +| true | foo=bar;baz=bar | foo=bar%3Bbaz=bar | +| false | foo=bar&baz=bar;foo | foo=bar&baz=bar&foo | +| true | foo=bar&baz=bar;foo | foo=bar&baz=bar%3Bfoo | + ### Middlewares The list of middlewares that are prepended by default to the list of middlewares of each router associated to the named entry point. diff --git a/integration/fixtures/simple_encode_semicolons.toml b/integration/fixtures/simple_encode_semicolons.toml new file mode 100644 index 000000000..c460f361f --- /dev/null +++ b/integration/fixtures/simple_encode_semicolons.toml @@ -0,0 +1,32 @@ +[global] + checkNewVersion = false + sendAnonymousUsage = false + +[log] + level = "DEBUG" + +[entryPoints] + [entryPoints.web] + address = ":8000" + [entryPoints.encodeSemicolons] + address = ":8001" + [entryPoints.encodeSemicolons.http] + encodeQuerySemicolons = true + +[api] + insecure = true + +[providers.file] + filename = "{{ .SelfFilename }}" + +## dynamic configuration ## + +[http.routers] + [http.routers.router] + service = "service1" + rule = "Host(`other.localhost`)" + +[http.services] + [http.services.service1.loadBalancer] + [[http.services.service1.loadBalancer.servers]] + url = "{{ .Server1 }}" diff --git a/integration/simple_test.go b/integration/simple_test.go index e25f25cfb..baf7aa230 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -1412,3 +1412,71 @@ func (s *SimpleSuite) TestDebugLog(c *check.C) { c.Fail() } } + +func (s *SimpleSuite) TestEncodeSemicolons(c *check.C) { + s.createComposeProject(c, "base") + + s.composeUp(c) + defer s.composeDown(c) + + whoami1URL := "http://" + net.JoinHostPort(s.getComposeServiceIP(c, "whoami1"), "80") + + file := s.adaptFile(c, "fixtures/simple_encode_semicolons.toml", struct { + Server1 string + }{whoami1URL}) + defer os.Remove(file) + + cmd, output := s.traefikCmd(withConfigFile(file)) + defer output(c) + + err := cmd.Start() + c.Assert(err, checker.IsNil) + defer s.killCmd(cmd) + + err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("Host(`other.localhost`)")) + c.Assert(err, checker.IsNil) + + testCases := []struct { + desc string + request string + target string + body string + expected int + }{ + { + desc: "Transforming semicolons", + request: "GET /?bar=toto;boo=titi HTTP/1.1\r\nHost: other.localhost\r\n\r\n", + target: "127.0.0.1:8000", + expected: http.StatusOK, + body: "bar=toto&boo=titi", + }, + { + desc: "Encoding semicolons", + request: "GET /?bar=toto&boo=titi;aaaa HTTP/1.1\r\nHost: other.localhost\r\n\r\n", + target: "127.0.0.1:8001", + expected: http.StatusOK, + body: "bar=toto&boo=titi%3Baaaa", + }, + } + + for _, test := range testCases { + conn, err := net.Dial("tcp", test.target) + c.Assert(err, checker.IsNil) + + _, err = conn.Write([]byte(test.request)) + c.Assert(err, checker.IsNil) + + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + c.Assert(err, checker.IsNil) + + if resp.StatusCode != test.expected { + c.Errorf("%s failed with %d instead of %d", test.desc, resp.StatusCode, test.expected) + } + + if test.body != "" { + body, err := io.ReadAll(resp.Body) + c.Assert(err, checker.IsNil) + c.Assert(string(body), checker.Contains, test.body) + } + } +} diff --git a/pkg/config/static/entrypoints.go b/pkg/config/static/entrypoints.go index f97d78e22..68ac9285e 100644 --- a/pkg/config/static/entrypoints.go +++ b/pkg/config/static/entrypoints.go @@ -57,9 +57,10 @@ func (ep *EntryPoint) SetDefaults() { // HTTPConfig is the HTTP configuration of an entry point. type HTTPConfig struct { - Redirections *Redirections `description:"Set of redirection" json:"redirections,omitempty" toml:"redirections,omitempty" yaml:"redirections,omitempty" export:"true"` - Middlewares []string `description:"Default middlewares for the routers linked to the entry point." json:"middlewares,omitempty" toml:"middlewares,omitempty" yaml:"middlewares,omitempty" export:"true"` - TLS *TLSConfig `description:"Default TLS configuration for the routers linked to the entry point." json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` + Redirections *Redirections `description:"Set of redirection" json:"redirections,omitempty" toml:"redirections,omitempty" yaml:"redirections,omitempty" export:"true"` + Middlewares []string `description:"Default middlewares for the routers linked to the entry point." json:"middlewares,omitempty" toml:"middlewares,omitempty" yaml:"middlewares,omitempty" export:"true"` + TLS *TLSConfig `description:"Default TLS configuration for the routers linked to the entry point." json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` + EncodeQuerySemicolons bool `description:"Defines whether request query semicolons should be URLEncoded." json:"encodeQuerySemicolons,omitempty" toml:"encodeQuerySemicolons,omitempty" yaml:"encodeQuerySemicolons,omitempty"` } // HTTP2Config is the HTTP2 configuration of an entry point. diff --git a/pkg/server/server_entrypoint_tcp.go b/pkg/server/server_entrypoint_tcp.go index 5d7095768..96cc29dc4 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -535,7 +535,11 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati return nil, err } - handler = http.AllowQuerySemicolons(handler) + if configuration.HTTP.EncodeQuerySemicolons { + handler = encodeQuerySemicolons(handler) + } else { + handler = http.AllowQuerySemicolons(handler) + } if withH2c { handler = h2c.NewHandler(handler, &http2.Server{ @@ -596,3 +600,23 @@ func (t *trackedConnection) Close() error { t.tracker.RemoveConnection(t.WriteCloser) return t.WriteCloser.Close() } + +// This function is inspired by http.AllowQuerySemicolons. +func encodeQuerySemicolons(h http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + if strings.Contains(req.URL.RawQuery, ";") { + r2 := new(http.Request) + *r2 = *req + r2.URL = new(url.URL) + *r2.URL = *req.URL + + r2.URL.RawQuery = strings.ReplaceAll(req.URL.RawQuery, ";", "%3B") + // Because the reverse proxy director is building query params from requestURI it needs to be updated as well. + r2.RequestURI = r2.URL.RequestURI() + + h.ServeHTTP(rw, r2) + } else { + h.ServeHTTP(rw, req) + } + }) +} diff --git a/pkg/server/service/proxy.go b/pkg/server/service/proxy.go index 2669499bf..257c4ce0d 100644 --- a/pkg/server/service/proxy.go +++ b/pkg/server/service/proxy.go @@ -48,6 +48,7 @@ func buildProxy(passHostHeader *bool, responseForwarding *dynamic.ResponseForwar outReq.URL.Path = u.Path outReq.URL.RawPath = u.RawPath + // If a plugin/middleware adds semicolons in query params, they should be urlEncoded. outReq.URL.RawQuery = strings.ReplaceAll(u.RawQuery, ";", "&") outReq.RequestURI = "" // Outgoing request should not have RequestURI