rinogo / yourls-keep-query-string

A simple YOURLS API plugin that adds the short URL's query string (if any) to the long URL
MIT License
5 stars 3 forks source link

Encoding forward slashes breaks some redirects #4

Closed jeffshead closed 1 year ago

jeffshead commented 1 year ago

I'm using Yourls v 1.9.2-dev with PHP 8.1.

With the the Keep Query String plugin enabled, forward slashes are getting encoded (%2F) so the redirects fail.

Below are the literal URLs (domain names are fake) that are saved in the database (no encoded characters):

- Short URL: https://sh.rt/terms
- Long URL: https://www.example.com/index.php?/Knowledgebase/List/Index/1/policies--terms

With the Keep Query String plugin activated, I am being redirected to: https://www.example.com/index.php?%2FKnowledgebase%2FList%2FIndex%2F1%2Fpolicies--terms

That particular system that I'm being redirected to throws a 404 for the encoded URL. So it seems some systems cannot handle encoded forward slashes in the query and they require them to be left unencoded.

If I disable the plugin, this does not happen and the redirects work because the forward slashes are not being encoded.

I'm assuming it's more of an issue with the system I'm being redirected to, than it is with this plugin but it would be nice to be able to specify to not url-encode the query string of the long url or to be able to specify which domains are exempted from having their long url query string url-encoded.

rinogo commented 1 year ago

Thanks for using the plugin! Yes, everything after the ? in the URL is considered the query string.

Are you comfortable with modifying the code? If so, it should be pretty trivial to add an exemption for the domain you’re referring to. If you’re not super comfortable, I can point you in the right direction.

jeffshead commented 1 year ago

Thank you! Sure, I can edit the code with your direction.

rinogo commented 1 year ago

Cool! I’m on my tablet right now and haven’t tested this, but you should be able to modify this line:

if (yourls_is_GO()) {

…to be something like:

if (yourls_is_GO() && !str_contains($url, ’/index.php?/Knowledgebase/List/’)) {

This would disable any URL that contains that exact string.

Without knowing more about your requirements, it’s hard to know what to recommend in place of ’/index.php?/Knowledgebase/List/’. You could even try using something like ://www.example.com to disable the domain entirely.

Hope this helps!

jeffshead commented 1 year ago

Sweet! Thanks so much for taking the time.

This would disable any URL that contains that exact string.

I'm assuming you mean any URL that contains that exact string will just bypass the Keep Query String plugin and still work/redirect.

I tested it and it works. I also tested (without modifying your plugin) using mod_rewrite in .htaccess so that index.php? can be eliminated from the long URL's. That works too. Just not sure which is the best approach.