getgrav / grav-plugin-feed

Grav Feed Plugin
https://getgrav.org
MIT License
16 stars 11 forks source link

1.8.2 breaks all feeds #61

Closed thespad closed 3 years ago

thespad commented 3 years ago

After upgrading to 1.8.2 all feeds fail to load and instead simply redirect back to the original page (i.e. my.site/blog.rss just loads the content from my.site/blog although the url is still my.site/blog.rss).

Rolling back to 1.8.1 fixes the issue. JSON feeds aren't enabled, but it doesn't seem to make a difference to the issue whether they are or not.

Can replicate on a completely clean grav+admin install, just to eliminate anything we might have customised or changed that could have broken things.

Grav v1.7.15 - Admin v1.10.15

rhukster commented 3 years ago

I will look at this ASAP.

rhukster commented 3 years ago

I can't actually reproduce this in any of my test sites. .rss and .xml are both rendering feed correctly.

thespad commented 3 years ago

Hmmm, is there anything useful I can provide or gather to help try and troubleshoot? If it were just broken generally I'd put it down to something weird I was doing but the fact that it specifically breaks with 1.8.2 and not earlier versions suggests that there's something odd going on.

Edit: here's a brand new instance with it happening

https://grav-test.spad.uk/home

https://grav-test.spad.uk/home.rss

Settings as vanilla as I can make them

image

petira commented 3 years ago

I have the same problem. I also have the latest version of Grav and Admin Panel. The problem is related to the update to Feed v1.8.2.

rhukster commented 3 years ago

I just downloaded a fresh skeleton-blog-site installation: https://getgrav.org/download/skeletons/blog-site/2.0.0

The one I had didn't have latest Feed or SimpleSearch updates, so i just updated those via bin/gpm update

After doing that the feeds still worked fine for .rss and .atom .. no .json enabled.

thespad commented 3 years ago

So if it helps my test site was setup with the barebones install from https://github.com/getgrav/grav/releases/download/1.7.15/grav-admin-v1.7.15.zip - if I get a chance in the morning I'll try and replicate with the skeleton install.

thespad commented 3 years ago

Recreated my test site with the deployment from https://getgrav.org/download/skeletons/blog-site/2.0.0 and I'm still seeing the same behaviour - works with the shipped 1.8.0, breaks when upgraded to 1.8.2. I've stood up a 2nd identical copy for comparison.

v1.8.2 https://grav-test.spad.uk https://grav-test.spad.uk/blog.rss

v1.8.0 https://grav-test2.spad.uk https://grav-test2.spad.uk/blog.rss

If it makes any difference this is using nginx but the config is pretty much stock + the grav sample conf.

Tugzrida commented 3 years ago

I'm seeing the same issue. Up to date grav running with nginx and php 7.4.3.

Narrowed it down to this line:

https://github.com/getgrav/grav-plugin-feed/blob/60ccc941b43206d9f463e416d7e1e1d7bbd36b94/feed.php#L101

Looks like the JSON fixes in 52ddd0d2474e4d381cf81b113e98a55e1f885fea introduced a bug.

For my site, when loading example.com/blog.rss, $url is set to /blog and $uri to //blog. As they aren't the same, the if statement doesn't trigger. As a temporary fix, I've just removed that clause from the if statement.

Not sure why it's difficult to reproduce. If I'm reading this line correctly, any path at the base of the site(where dirname is /) will end up with two slashes.

https://github.com/getgrav/grav-plugin-feed/blob/60ccc941b43206d9f463e416d7e1e1d7bbd36b94/feed.php#L97

rhukster commented 3 years ago

Thanks! This helps a lot. All my testing was on sites that were in subdirectories so I never saw that // issue, even at root. I’ll get a fix out today.

rhukster commented 3 years ago

I've pushed a fix (see above). can someone please test to make sure this works for you now?

thespad commented 3 years ago

Did a direct install and it's looking good, will do some more testing to see if I can break it.

rhukster commented 3 years ago

I'm pretty sure it's good, so going to release it.

Tugzrida commented 3 years ago

Works on my site after doing the update through the admin panel :)

thespad commented 3 years ago

All good, thanks for sorting it so quickly @rhukster