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

Still working? #3

Open enoversum opened 3 years ago

enoversum commented 3 years ago

I’m trying to use it on the latest YOURLS version (1.8.1), and for some reason parameters are simply not attached. I tried it directly, so no third-party website where a link is placed is to blame here.

Expected behaviour: Shortlink is: my.site/shorturl With query string: my.site/shorturl?src=github Longlink is: mylongurl.com/longstory Longlink with query string: mylongurl.com/longstory?src=github

Actual behaviour: Longlink stays: mylongurl.com/longstory

rinogo commented 3 years ago

Hmm. Weird! It's still working for me as of 1.7.9. If I upgrade, I'll take a look. There's not quite enough traction on this project to put a lot of energy into updating it, but if you check it out and find the problem, let me know here! The plugin is pretty simple; it should be fairly easy to experiment a little!

enoversum commented 3 years ago

Hey @rinogo , I had just installed your plugin with the current version, so I don't know if it worked with previous iterations of YOURLS. I tried it on a URL I shortened before I installed your plugin - but that should work nonetheless, right :) ?

rinogo commented 3 years ago

Hi! Yes, I think it should work even in that scenario. If you end up diving in and finding the problem, please let me know! Likewise, if I update my local installation, I'll do some testing on my end.

rinogo commented 3 years ago

P.S. I took a little deeper dive -

The plugin code doesn't really use much of the YOURLS API, so I'm a little surprised it's not working.

I reviewed the changelogs for YOURLS between 1.7.9 and 1.8.1 and I don't see anything relevant. I also analyzed the diff between 1.7.9 and 1.8.1 and don't see anything there, either.

I noticed that the definition for yourls_is_GO() did change slightly. I don't think that's the issue, but you could probably verify this by temporarily commenting out line 12 and line 15.

Beyond that, putting a bunch of dump statements (e.g. print_r()) to see what's going on. Good luck!

enoversum commented 3 years ago

Thanks again @rinogo for getting back and taking the time to investigate. I didn't do any modding from my side to YOURLS, just going with a few plugins. Commenting out lines 12–15 unfortunately doesn’t help, it still ignores the query string. Also, even newly created URLs won't work. I also tried disabling each and every plugin, with the same result: Query strings aren't being added to the long link. So, unfortunately, it seems to have to do with this plugin. Just one last straw: May it have to do with my server running on NGINX? I know systems like WordPress work flawlessly most of the times, but in some cases need a few server rules to run smoothly.

rinogo commented 3 years ago

You're welcome! I can't promise any level of support, but I'm happy to give a little help here and there!

By the way, I think I may have not been clear enough. You should comment out line 12 and line 15; not line 12 through 15.

And nope, the NGINX element shouldn't be an issue at all!

I actually just tried to update a development instance of YOURLS that I have to 1.8.1, but I'm getting an error after installation saying, Composer detected issues in your platform: Your Composer dependencies require a PHP version ">= 7.2.0". I don't have time at the moment to update PHP just to look into this error. However, that does potentially point out what might be the issue - it might be due to a difference in PHP versions instead of YOURLS versions.

enoversum commented 3 years ago

By the way, I think I may have not been clear enough. You should comment out line 12 and line 15; not line 12 through 15.

Ah, right, so it's just the condition to be commented out. That, unfortunately, didn't help

I actually just tried to update a development instance of YOURLS that I have to 1.8.1, but I'm getting an error after installation saying, Composer detected issues in your platform: Your Composer dependencies require a PHP version ">= 7.2.0". I don't have time at the moment to update PHP just to look into this error. However, that does potentially point out what might be the issue - it might be due to a difference in PHP versions instead of YOURLS versions.

I’m using the very latest version of PHP 7.4, and for most applications 7.4 works flawlessly, maybe it's not too much of a hassle to upgrade to 7.4? Yet I of course don't know your setup and requirements. And I get you don't want to invest tons of time into your free plugin 😃 .

rinogo commented 3 years ago

Hmm. Hit me up at rich at social5 dot com; maybe we can do something creative!

bmercernccer commented 3 years ago

For what it's worth, I've been having the same problem with this plugin, here's a bit more info that might be helpful. I noticed the issue using YOURLS version: 1.7.10 and PHP version: 7.3.15. Upgrading to the latest YOURLS version had no effect on the issue, still the same results with 1.8.1.

I've been trying some of the other query string plugins, but had similar issues with all of them, which makes me think it may be a PHP change.

I'm not able to test on an older PHP at this time unfortunately.

skarjoss commented 2 years ago

For anyone looking for a solution, check this

rinogo commented 2 years ago

@oscarquinde - if you find a way to get this working, please submit a Pull Request - I'll happily accept it!