Redirects preserve query parameters. #228
Conversation
lib/middleware/redirects.js
Outdated
| @@ -54,15 +61,15 @@ Redirect.prototype.test = function(url) { | |||
|
|
|||
| return { | |||
| type: this.type, | |||
| destination: dest | |||
| destination: dest + qs | |||
There was a problem hiding this comment.
I think it's possible for destinations to contain a query string - do we want to check and possibly merge query strings (overwriting? adding? if we get a collision)
There was a problem hiding this comment.
Let's keep it simple -- I'm now appending them, including a dup if there's a dup between the supplied qs and the destination qs.
| @@ -34,6 +34,13 @@ var Redirect = function(source, destination, type) { | |||
| }; | |||
|
|
|||
| Redirect.prototype.test = function(url) { | |||
| var qs = ''; | |||
There was a problem hiding this comment.
Should we explicitly remove the query string in all places we match (rewrites, headers etc.)?
There was a problem hiding this comment.
We don't need to - it's pathToRegexp that falls apart with a query string, everywhere else we use minimatch which handles these things just fine:
minimatch('/foo/bar?baz=qux', '/foo/*'); // => true
lib/middleware/redirects.js
Outdated
| @@ -54,15 +61,15 @@ Redirect.prototype.test = function(url) { | |||
|
|
|||
| return { | |||
| type: this.type, | |||
| destination: dest | |||
| destination: dest + qs | |||
There was a problem hiding this comment.
Let's keep it simple -- I'm now appending them, including a dup if there's a dup between the supplied qs and the destination qs.
This updates redirects to properly match and redirect when a query string is present, preserving the query string. It explicitly does not: