Blank admin dashboard if included Snoopy HTTP client is missing openssl extension for getting security feed

[flowdy]

Summary

Snoopy just exits the PHP request if it cannot find OpenSSL extension.

Step to reproduce

Uninstall openssl extension if needed, enable security feed on dashboard.

Observed behavior

The manager is all blank (not a bit of content) after admin log in.

Expected behavior

An error message "HTTPS feed url cannot be fetched: openssl extension not detected" or alike should be shown at the place where the feed content would be rendered.

Environment

Tested on MODx 2.5.{1,2}-pl on nginx with PHP-FPM.

[Mark-H]

Snoopy?

[Jako]

RSS Feed Fetcher: https://github.com/modxcms/revolution/search?utf8=%E2%9C%93&q=snoopy

Used by the two widgets MODX News Feed/MODX Security Feed

[Jako]

@flowdy As workaround you could remove those widgets from the Dashboard.

[Mark-H]

... learned something new today :D

[flowdy]

Yes, I have done that, thank you.

What a fun that was poking around in the sources with die("Things work hitherto");, thus approaching the position where they stop working. On exit()ing, PHP certainly has no reason whatsoever to propagate that as an error. One could argue that exit() from a function in module loaded should be always be interpreted as die("exit() called from a module is nearly always wrong. Shame on you.").

[OptimusCrime]

@flowdy Could we get the actual PHP error/stacktrace?

[flowdy]

I do not have an error stack trace, because exit() is not die().
See https://github.com/modxcms/revolution/blob/2.x/core/model/modx/xmlrss/extlib/snoopy.class.php#L139.

[OptimusCrime]

Ah, I am sorry. I did not understand the context of your previous comment. I am not sure how to use the snoopy library or call it from MODX. Do you know if replacing the exit with a die would solve the problem correctly? In that case I can open a PR with this fix pretty soon.

[Jako]

@flowdy Thats the problem.

Are the widgets called directly? Maybe the content could be fetched remote. Or the exit has to be replaced by something not stopping the whole system.

[flowdy]

Snoopy library is included with require_once in the linked source file. I am not quite sure a die would help in a user-friendly way. Simply return. I tested it. The dashboard renders again, but I cannot see the message "Unable to retrieve RSS file for unknown reasons." which is intended to show up according to the sources

[Jako]

No error handler in the Snoopy code that could send the result to the MODX log?

[flowdy]

Why don't you just use php_curl to fetch a url in the first place? Snoopy just does not look good in quality to me and after all prone to no more support some day.

[flowdy]

The actual innermost error is that trigger_error(...) in Snoopy is not really effective, is it?

[Mark-H]

Looks like there's a couple of things we should do...

Step 1) Make sure that the manager doesn't break because of the trigger_error/exit in https://github.com/modxcms/revolution/blob/2.x/core/model/modx/xmlrss/extlib/snoopy.class.php#L139 - targeting next patch release

Step 2) Load feeds on the dashboard via AJAX instead of in the manager request. That will also speed up the dashboard loading as I've often see that take multiple seconds, which I'm 90% sure is due to the feeds. - targeting next feature release

Step 3) Nuke the rssfetch "class", snoopy, and all the other relevant code sprinkled throughout the core related to it. Replace with a nicely tested, simpler, maintained library from packagist in MODX 3. Guzzle is rather popular around the PHP world for sending requests, so that could be an option (which could also replace the modRest classes marked for deprecation in #13199). Or perhaps there's a library specific to reading RSS feeds. - targeting MODX 3

[Jako]

An iFrame would be possible too.

[Mark-H]

I'm working on some stuff to help with this. Trying to keep it BC so it can be included in 2.6.

[OptimusCrime]

@Mark-H Nice work. Can this be closed?

[sonicpunk]

#modxbughunt #1point to @OptimusCrime