Addition of trailing slash loses URL parameter

[rgls]

Short description of the issue

When the option “Should page URLs end with a slash” is enabled, URLs get rewritten to the original URL with a trailing slash added. Then the browser is redirected to this new URL. The new URL does no longer contain any URL parameters the original URL contained.

Expected behavior

When accessing http://example.com/somepage?a=b you should be redirected to http://example.com/somepage/?a=b.

Actual behavior

Instead you are redirected to http://example.com/somepage/ (without the parameters).

• ProcessWire version: 3.0.98

[ryancramerdesign]

Query strings (GET variables) are a type of user input. ProcessWire does not pass non-validated user input directly into output, which would be an HTTP 301 redirect header in this case. This is a matter of security and is by design.

If you want this redirect behavior, maintaining the query string, you can safely accomplish it with an Apache rewrite rule. You would add this after your "RewriteEngine On" in your .htaccess file:

# If your site runs from a subdirectory, replace "/" below with "/subdir/"
RewriteBase /
RewriteCond %{REQUEST_FILENAME} !-f
# If your admin is not at /processwire/ replace "processwire" below with your admin
RewriteCond %{REQUEST_URI} !processwire
RewriteCond %{REQUEST_URI} !(.*)/$
RewriteRule ^(.*)$ $1/ [L,QSA,R=301]

Your admin URL needs to be excluded from this rule because the PW admin depends on some URLs not having trailing slashes. If any part of your site also depends on not having trailing slashes (for instance, URLs with pagination numbers, URL segments, etc.) then those will need to be excluded as well, by adding more lines like the one that excludes the processwire admin above. For instance, I think you could exclude pagination URLs with RewriteCond %{REQUEST_URI} !/page[0-9]+$

[netcarver]

@rgls Does Ryan's .htaccess solution solve this issue for you?

[rgls]

Possibly. I haven't tried it, since my patch solves the problem for me and I am not really convinced by the cited security concerns.
Are POST data and URL segments removed as well? Both are unvalidated user input and shouldn't survive the redirect by Ryan's argumentation.
Furthermore it seems not a good idea to handle some part of the rewriting/redirecting in the rewrite module and another part in the code of processwire itself.

[LostKobrakai]

POST data is droped as well as POST requests cannot be 301/302 redirected at all. But the security concern of not keeping get query data when just adding the trailing slash is in my opinion a bad justification, as that request could hit the server without the redirect and it would need to handle bad input there as well.

[rgls]

Exactly. Security isn't handled in the rewrite/redirect portion of the code but must be handled elsewhere. This only fixes a (slightly) malformed request.

I can't see how moving the rewriting from processwire code to the apache rewrite module makes it any safer. The same request (with trailing slash and url parameters) will hit the server and has to be handled.

And yes, you are right about the POST data. I hadn't thought that part through. Nevertheless URL segments are unvalidated user input as well (as is, if you think about it, the whole URL...).

[netcarver]

@ryancramerdesign Any further thoughts on this issue?

[ryancramerdesign]

@netcarver GET variables are input and can be anything, and of any quantity and length. HTTP headers (as used in redirects) are output. As a matter of security the core will not take anonymous/unknown input and throw it right back into the output. That's just a line I don't cross with the core–security is the first priority. If someone evaluates there are more benefits than risks for their own case, then that's fine, but their code should do it. Using .htaccess for this purpose is also a safe way to handle it.

[netcarver]

@ryancramerdesign Understood. In that case I'm going to close this issue.