@marcel91200 / caddy Mirror · read-only / commits / d876de61
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 14, 05:15 UTC

rewrite: Fix query string logic

Matthew Holt committed Jan 11, 2020 at 11:40 UTC d876de61e512db7a31a7ae59723d5134048f283e
2 files changed +34 -22
modules/caddyhttp/rewrite/rewrite.go
+26 -22
@@ -171,23 +171,31 @@ func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.L
171
172 // buildQueryString takes an input query string and
173 // performs replacements on each component, returning
174 -// the resulting query string.
174 +// the resulting query string. This function appends
175 +// duplicate keys rather than replaces.
176 func buildQueryString(qs string, repl *caddy.Replacer) string {
177 var sb strings.Builder
177 - var wroteKey bool
178 +
179 + // first component must be key, which is the same
180 + // as if we just wrote a value in previous iteration
181 + wroteVal := true
182
183 for len(qs) > 0 {
180 - // determine the end of this component
184 + // determine the end of this component, which will be at
185 + // the next equal sign or ampersand, whichever comes first
186 nextEq, nextAmp := strings.Index(qs, "="), strings.Index(qs, "&")
182 - end := min(nextEq, nextAmp)
183 - if end == -1 {
184 - end = len(qs) // if there is nothing left, go to end of string
187 + ampIsNext := nextAmp >= 0 && (nextAmp < nextEq || nextEq < 0)
188 + end := len(qs) // assume no delimiter remains...
189 + if ampIsNext {
190 + end = nextAmp // ...unless ampersand is first...
191 + } else if nextEq >= 0 && (nextEq < nextAmp || nextAmp < 0) {
192 + end = nextEq // ...or unless equal is first.
193 }
194
195 // consume the component and write the result
196 comp := qs[:end]
197 comp, _ = repl.ReplaceFunc(comp, func(name, val string) (string, error) {
190 - if name == "http.request.uri.query" {
198 + if name == "http.request.uri.query" && wroteVal {
199 return val, nil // already escaped
200 }
201 return url.QueryEscape(val), nil
@@ -197,29 +205,25 @@ func buildQueryString(qs string, repl *caddy.Replacer) string {
205 }
206 qs = qs[end:]
207
200 - if wroteKey {
208 + // if previous iteration wrote a value,
209 + // that means we are writing a key
210 + if wroteVal {
211 + if sb.Len() > 0 {
212 + sb.WriteRune('&')
213 + }
214 + } else {
215 sb.WriteRune('=')
202 - } else if sb.Len() > 0 {
203 - sb.WriteRune('&')
216 }
205 -
206 - // remember that we just wrote a key, which is if the next
207 - // delimiter is an equals sign or if there is no ampersand
208 - wroteKey = nextEq < nextAmp || nextAmp < 0
209 -
217 sb.WriteString(comp)
218 +
219 + // remember for the next iteration that we just wrote a value,
220 + // which means the next iteration MUST write a key
221 + wroteVal = ampIsNext
222 }
223
224 return sb.String()
225 }
226
216 -func min(a, b int) int {
217 - if b < a {
218 - return b
219 - }
220 - return a
221 -}
222 -
227 // replacer describes a simple and fast substring replacement.
228 type replacer struct {
229 // The substring to find. Supports placeholders.
modules/caddyhttp/rewrite/rewrite_test.go
+8
@@ -138,6 +138,11 @@ func TestRewrite(t *testing.T) {
138 input: newRequest(t, "GET", "/foo/bar?a=b&c=d"),
139 expect: newRequest(t, "GET", "/foo/bar"),
140 },
141 + {
142 + rule: Rewrite{URI: "?qs={http.request.uri.query}"},
143 + input: newRequest(t, "GET", "/foo?a=b&c=d"),
144 + expect: newRequest(t, "GET", "/foo?qs=a%3Db%26c%3Dd"),
145 + },
146 {
147 rule: Rewrite{URI: "/foo?{http.request.uri.query}#frag"},
148 input: newRequest(t, "GET", "/foo/bar?a=b"),
@@ -216,6 +221,9 @@ func TestRewrite(t *testing.T) {
221 if expected, actual := tc.expect.URL.RequestURI(), tc.input.URL.RequestURI(); expected != actual {
222 t.Errorf("Test %d: Expected URL.RequestURI()='%s' but got '%s'", i, expected, actual)
223 }
224 + if expected, actual := tc.expect.URL.Fragment, tc.input.URL.Fragment; expected != actual {
225 + t.Errorf("Test %d: Expected URL.Fragment='%s' but got '%s'", i, expected, actual)
226 + }
227 }
228 }
229