Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.8.2 breaks all feeds #61

Closed
thespad opened this issue May 27, 2021 · 14 comments
Closed

1.8.2 breaks all feeds #61

thespad opened this issue May 27, 2021 · 14 comments
Assignees
Labels

Comments

@thespad
Copy link

thespad commented May 27, 2021

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
Copy link
Member

I will look at this ASAP.

@rhukster rhukster self-assigned this May 27, 2021
@rhukster rhukster added the bug label May 27, 2021
@rhukster
Copy link
Member

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

@thespad
Copy link
Author

thespad commented May 27, 2021

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
Copy link

petira commented May 27, 2021

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
Copy link
Member

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
Copy link
Author

thespad commented May 27, 2021

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
Copy link
Author

thespad commented May 27, 2021

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
Copy link

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

Narrowed it down to this line:

if ($url === $uri && property_exists($page->header(), 'content')) {

Looks like the JSON fixes in 52ddd0d 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.

$uri = $uri_info['dirname'] . '/' . $uri_info['filename'];

@rhukster
Copy link
Member

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 added a commit that referenced this issue May 28, 2021
@rhukster
Copy link
Member

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

@thespad
Copy link
Author

thespad commented May 28, 2021

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

@rhukster
Copy link
Member

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

@Tugzrida
Copy link

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

@thespad
Copy link
Author

thespad commented May 30, 2021

All good, thanks for sorting it so quickly @rhukster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants