From 97ad002880c7bb2b9273c08434108b83b1d54e00 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 22 Dec 2021 18:26:37 +0100 Subject: [PATCH 1/3] Set up content types for all API endpoints If Tracy does not see Content-Type or sees text/html, it will inject its JavaScript, which breaks tests that compare response body. --- src/common.php | 9 +++++---- src/controllers/Index.php | 2 ++ src/controllers/Opml/Export.php | 9 +++++---- src/controllers/Opml/ImportPage.php | 1 + src/controllers/Sources/Update.php | 4 ++++ src/controllers/Sources/Write.php | 2 +- src/helpers/Authentication.php | 1 + src/helpers/View.php | 6 +++--- src/helpers/responses.php | 16 ++++++++++++++++ 9 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 src/helpers/responses.php diff --git a/src/common.php b/src/common.php index d9db78c785..ec1044ec45 100644 --- a/src/common.php +++ b/src/common.php @@ -3,6 +3,7 @@ use Dice\Dice; use helpers\Configuration; use helpers\DatabaseConnection; +use function helpers\sendError; use Monolog\Formatter\LineFormatter; use Monolog\Handler\ErrorLogHandler; use Monolog\Handler\NullHandler; @@ -10,9 +11,11 @@ use Monolog\Logger; require __DIR__ . '/constants.php'; +require_once __DIR__ . '/helpers/responses.php'; $autoloader = @include BASEDIR . '/vendor/autoload.php'; // we will show custom error if ($autoloader === false) { + header('Content-type: text/plain'); echo 'The PHP dependencies are missing. Did you run `composer install` in the selfoss directory?'; exit; } @@ -83,8 +86,7 @@ $dice->addRule(daos\TagsInterface::class, $shared); if ($configuration->isChanged('dbSocket') && $configuration->isChanged('dbHost')) { - echo 'You cannot set both `db_socket` and `db_host` options.' . PHP_EOL; - exit; + sendError('You cannot set both `db_socket` and `db_host` options.' . PHP_EOL); } // Database connection @@ -231,8 +233,7 @@ function($pattern, $text) { } elseif ($logger_destination === 'error_log') { $handler = new ErrorLogHandler(ErrorLogHandler::OPERATING_SYSTEM, $configuration->loggerLevel); } else { - echo 'The `logger_destination` option needs to be either `error_log` or a file path prefixed by `file:`.'; - exit; + sendError('The `logger_destination` option needs to be either `error_log` or a file path prefixed by `file:`.'); } $formatter = new LineFormatter(null, null, true, true); diff --git a/src/controllers/Index.php b/src/controllers/Index.php index dc91868721..f2188f6d36 100644 --- a/src/controllers/Index.php +++ b/src/controllers/Index.php @@ -63,11 +63,13 @@ public function home() { $home = BASEDIR . '/public/index.html'; if (!file_exists($home)) { http_response_code(500); + header('Content-type: text/plain'); echo 'Please build the assets using `npm run build` or obtain a pre-built packages from https://selfoss.aditu.de.'; exit; } // show as full html page + header('Content-type: text/html'); readfile($home); return; diff --git a/src/controllers/Opml/Export.php b/src/controllers/Opml/Export.php index 27bd6b28f8..d359c209d5 100644 --- a/src/controllers/Opml/Export.php +++ b/src/controllers/Opml/Export.php @@ -94,6 +94,11 @@ private function writeSource(array $source) { public function export() { $this->authentication->needsLoggedIn(); + // save content as file and suggest file name + $exportName = 'selfoss-subscriptions-' . date('YmdHis') . '.xml'; + header('Content-Disposition: attachment; filename="' . $exportName . '"'); + header('Content-Type: text/xml; charset=UTF-8'); + $this->logger->debug('start OPML export'); $this->writer->openMemory(); $this->writer->setIndent(true); @@ -166,10 +171,6 @@ public function export() { $this->writer->endDocument(); $this->logger->debug('finished OPML export'); - // save content as file and suggest file name - $exportName = 'selfoss-subscriptions-' . date('YmdHis') . '.xml'; - header('Content-Disposition: attachment; filename="' . $exportName . '"'); - header('Content-Type: text/xml; charset=UTF-8'); echo $this->writer->outputMemory(); } } diff --git a/src/controllers/Opml/ImportPage.php b/src/controllers/Opml/ImportPage.php index 11ec42152e..2c2d7b6adb 100644 --- a/src/controllers/Opml/ImportPage.php +++ b/src/controllers/Opml/ImportPage.php @@ -25,6 +25,7 @@ public function __construct(Authentication $authentication) { */ public function show() { $this->authentication->needsLoggedIn(); + header('Content-type: text/html'); readfile(BASEDIR . '/public/opml.html'); } } diff --git a/src/controllers/Sources/Update.php b/src/controllers/Sources/Update.php index 6ff65c7ee8..5526072dd4 100644 --- a/src/controllers/Sources/Update.php +++ b/src/controllers/Sources/Update.php @@ -27,6 +27,8 @@ public function __construct(Authentication $authentication, ContentLoader $conte * @return void */ public function updateAll() { + header('Content-type: text/plain'); + // only allow access for localhost and loggedin users if (!$this->authentication->allowedToUpdate()) { exit('unallowed access'); @@ -47,6 +49,8 @@ public function updateAll() { * @return void */ public function update($id) { + header('Content-type: text/plain'); + // only allow access for localhost and authenticated users if (!$this->authentication->allowedToUpdate()) { exit('unallowed access'); diff --git a/src/controllers/Sources/Write.php b/src/controllers/Sources/Write.php index 20714a7a57..dee16a47db 100644 --- a/src/controllers/Sources/Write.php +++ b/src/controllers/Sources/Write.php @@ -109,7 +109,7 @@ public function write($id = null) { $validation = $this->sourcesDao->validate($title, $spout, $data); if ($validation !== true) { - $this->view->error(json_encode($validation)); + $this->view->jsonError($validation); } // add/edit source diff --git a/src/helpers/Authentication.php b/src/helpers/Authentication.php index 87dbc7ec2a..368794525a 100644 --- a/src/helpers/Authentication.php +++ b/src/helpers/Authentication.php @@ -172,6 +172,7 @@ public function needsLoggedIn() { */ public function forbidden() { header('HTTP/1.0 403 Forbidden'); + header('Content-type: text/plain'); echo 'Access forbidden!'; exit; } diff --git a/src/helpers/View.php b/src/helpers/View.php index a9c37085b6..f63fc6b9e0 100644 --- a/src/helpers/View.php +++ b/src/helpers/View.php @@ -99,8 +99,7 @@ public function isAjax() { * @return void */ public function error($message) { - header('HTTP/1.0 400 Bad Request'); - exit($message); + sendError($message); } /** @@ -111,8 +110,9 @@ public function error($message) { * @return void */ public function jsonError($data) { + header('HTTP/1.0 400 Bad Request'); header('Content-type: application/json'); - $this->error(json_encode($data)); + exit(json_encode($data)); } /** diff --git a/src/helpers/responses.php b/src/helpers/responses.php new file mode 100644 index 0000000000..b5bbc7bcb9 --- /dev/null +++ b/src/helpers/responses.php @@ -0,0 +1,16 @@ + Date: Wed, 22 Dec 2021 17:55:07 +0100 Subject: [PATCH 2/3] Fix tests on PHP 5.6 Since the switch to Tracy, timezone warnings started to occur. These are no an issue on PHP 7+: https://github.com/php/doc-en/pull/1238 Also fix path to php.ini. --- tests/integration/php.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/php.ini b/tests/integration/php.ini index 6cc845ab60..18263195a9 100644 --- a/tests/integration/php.ini +++ b/tests/integration/php.ini @@ -2,3 +2,5 @@ display_errors = on display_startup_errors = on error_reporting = E_ALL log_errors = on +; Fix warning on PHP 5 +date.timezone = UTC From 8a39c109cf7f56f0b1254cdca962b80b701df88f Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 22 Dec 2021 11:37:33 +0100 Subject: [PATCH 3/3] Use Tracy for error handling It offers much nicer error traces and the code is significantly cleaner. This was the last thing we used F3 framework for. Now we are officially fat-free-free. So long and thanks for all the fish. --- NEWS.md | 5 +- README.md | 2 +- composer.json | 2 +- composer.lock | 107 +++++++++++++------- docs/content/docs/administration/options.md | 10 ++ src/common.php | 74 +++----------- src/error.500.phtml | 50 +++++++++ src/helpers/Configuration.php | 2 +- src/helpers/TracyLogger.php | 52 ++++++++++ tests/integration/helpers/selfoss_server.py | 1 + 10 files changed, 206 insertions(+), 99 deletions(-) create mode 100644 src/error.500.phtml create mode 100644 src/helpers/TracyLogger.php diff --git a/NEWS.md b/NEWS.md index 8393da3f66..3c39d5e565 100644 --- a/NEWS.md +++ b/NEWS.md @@ -98,14 +98,15 @@ - RSS spout now prefers the feed logo to website favicon. ([#1152](https://github.com/fossar/selfoss/pull/1152)) - RSS spout now tries to use favicon from the feed domain when there is no logo or home page favicon. ([#1152](https://github.com/fossar/selfoss/pull/1152)) - Setting `DEBUG` to `1` in `src/common.php` no longer logs HTTP bodies, only headers. Set it to `2` if you need the bodies as well. ([#1152](https://github.com/fossar/selfoss/pull/1152)) -- PHP startup errors are now logged, instead of having F3 crash with Error 500 ([#1195](https://github.com/fossar/selfoss/pull/1195)) +- The debugging level (previously set by modifying `src/common.php`) can be changed in the `config.ini` using `debug` key. ([#1261](https://github.com/fossar/selfoss/pull/1261)) - In order to support offline mode, we moved much of the UI to the browser. ([#1150](https://github.com/fossar/selfoss/pull/1150), [#1184](https://github.com/fossar/selfoss/pull/1184), [#1215](https://github.com/fossar/selfoss/pull/1215), [#1216](https://github.com/fossar/selfoss/pull/1216)) - We carried out a significant internal refactoring ([#1164](https://github.com/fossar/selfoss/pull/1164), [#1190](https://github.com/fossar/selfoss/pull/1190)) - Removed Instapaper spout since it has been broken since its acquisition. Sources using it were migrated to “RSS Feed (with content extraction)”. ([#1245](https://github.com/fossar/selfoss/pull/1245)) - Placeholders are now used for images before they are loaded to avoid content jumping around ([#1204](https://github.com/fossar/selfoss/pull/1204)) - Search button is now always on the screen, avoiding the need to scroll to top to be able to use it. ([#1231](https://github.com/fossar/selfoss/issues/1231)) - Button for opening articles, tags, sources and filters in the sidebar, as well as the source and tag links in articles are now real links, allowing to open them in a new tab by middle-clicking them. ([#1216](https://github.com/fossar/selfoss/issues/1216), [#695](https://github.com/fossar/selfoss/issues/695)) -- Configuration is no longer managed by F3 framework. ([#1261](https://github.com/fossar/selfoss/pull/1261)) +- [F3 framework](https://fatfreeframework.com) is no longer used. So long… ([#1261](https://github.com/fossar/selfoss/pull/1261), [#1295](https://github.com/fossar/selfoss/pull/1295), [#1296](https://github.com/fossar/selfoss/pull/1296)) +- [Tracy](https://tracy.nette.org/) is now used for error handling, resulting in much nicer error messages. ([#1296](https://github.com/fossar/selfoss/pull/1296)) ## 2.18 – 2018-03-05 diff --git a/README.md b/README.md index cc466a9fac..19c58edcfa 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,6 @@ Very special thanks to all contributors of pull requests here on [GitHub](https: Special thanks to the great programmers of these libraries used by selfoss: -* [FatFree PHP Framework](https://fatfreeframework.com/) * [SimplePie](http://simplepie.org/) * [jQuery](https://jquery.com/) * [WideImage](http://wideimage.sourceforge.net/) @@ -100,6 +99,7 @@ Special thanks to the great programmers of these libraries used by selfoss: * [Spectrum Colorpicker](https://github.com/bgrins/spectrum) * [Graby](https://github.com/j0k3r/graby) * [FullTextRSS filters](http://help.fivefilters.org/customer/portal/articles/223153-site-patterns) +* [Tracy](https://tracy.nette.org/) Icon made by http://blackbooze.com/ diff --git a/composer.json b/composer.json index 53ddac4044..827ca90f0a 100644 --- a/composer.json +++ b/composer.json @@ -5,7 +5,6 @@ "require": { "php": ">= 5.6", "ext-gd": "*", - "bcosca/fatfree-core": "^3.7.0", "bramus/router": "^1.6", "danielstjules/stringy": "^3.1", "fossar/guzzle-transcoder": "^0.2.0", @@ -23,6 +22,7 @@ "php-http/guzzle6-adapter": "^1.0", "simplepie/simplepie": "^1.3", "smottt/wideimage": "^1.1", + "tracy/tracy": "^2.5", "violet/streaming-json-encoder": "^1.1", "willwashburn/phpamo": "^1.0" }, diff --git a/composer.lock b/composer.lock index 9c8285d5c2..8f04a0962f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,43 +4,8 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c6028fdec0e065363013d1461e6009bd", + "content-hash": "5ae67f7bd862e293545823d8a36fb58f", "packages": [ - { - "name": "bcosca/fatfree-core", - "version": "3.7.3", - "source": { - "type": "git", - "url": "https://github.com/bcosca/fatfree-core.git", - "reference": "3e23ae05384b2f830e99c5888b94118819ed948b" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/bcosca/fatfree-core/zipball/3e23ae05384b2f830e99c5888b94118819ed948b", - "reference": "3e23ae05384b2f830e99c5888b94118819ed948b", - "shasum": "" - }, - "require": { - "php": ">=5.4" - }, - "type": "library", - "autoload": { - "classmap": [ - "." - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "GPL-3.0" - ], - "description": "A powerful yet easy-to-use PHP micro-framework designed to help you build dynamic and robust Web applications - fast!", - "homepage": "http://fatfreeframework.com/", - "support": { - "issues": "https://github.com/bcosca/fatfree-core/issues", - "source": "https://github.com/bcosca/fatfree-core/tree/3.7.3" - }, - "time": "2020-12-13T12:49:39+00:00" - }, { "name": "bramus/router", "version": "1.6.1", @@ -2795,6 +2760,76 @@ ], "time": "2020-10-23T09:01:57+00:00" }, + { + "name": "tracy/tracy", + "version": "v2.5.9", + "source": { + "type": "git", + "url": "https://github.com/nette/tracy.git", + "reference": "cd9b889371cfbffe17e5b1a19e0a11de1ad31c8f" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/nette/tracy/zipball/cd9b889371cfbffe17e5b1a19e0a11de1ad31c8f", + "reference": "cd9b889371cfbffe17e5b1a19e0a11de1ad31c8f", + "shasum": "" + }, + "require": { + "ext-json": "*", + "ext-session": "*", + "php": ">=5.4.4" + }, + "require-dev": { + "nette/di": "~2.3 || ~3.0.0", + "nette/tester": "~1.7 || ~2.0", + "nette/utils": "~2.3" + }, + "suggest": { + "https://nette.org/donate": "Please support Tracy via a donation" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.5-dev" + } + }, + "autoload": { + "classmap": [ + "src" + ], + "files": [ + "src/shortcuts.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "David Grudl", + "homepage": "https://davidgrudl.com" + }, + { + "name": "Nette Community", + "homepage": "https://nette.org/contributors" + } + ], + "description": "😎 Tracy: the addictive tool to ease debugging PHP code for cool developers. Friendly design, logging, profiler, advanced features like debugging AJAX calls or CLI support. You will love it.", + "homepage": "https://tracy.nette.org", + "keywords": [ + "Xdebug", + "debug", + "debugger", + "nette", + "profiler" + ], + "support": { + "issues": "https://github.com/nette/tracy/issues", + "source": "https://github.com/nette/tracy/tree/v2.5.9" + }, + "time": "2020-05-17T09:25:46+00:00" + }, { "name": "true/punycode", "version": "v2.1.1", diff --git a/docs/content/docs/administration/options.md b/docs/content/docs/administration/options.md index 10ba9b6169..5e378ad6d5 100644 --- a/docs/content/docs/administration/options.md +++ b/docs/content/docs/administration/options.md @@ -59,6 +59,16 @@ Port for database connections. By default `3306` will be used for MySQL and `543 A UNIX domain socket used for connecting to the MySQL database server. Usually, you want to use `db_host=localhost`, which should use the default socket path (typically `/run/mysqld/mysqld.sock` for MySQL or `/run/postgresql` for PostgreSQL) but if you need to specify a different location, you can. This is orthogonal to `db_host` option. +### `debug` +
+ +Level of detail for diagnostics. Enable this for debug traces if you encounter selfoss internal errors or are developing selfoss. + +* `0` (default) – Debugging is disabled, warnings will only be [logged](#logger-level). +* `1` – Debugging is enabled, any error or warning will terminate the application and cause debugging information to be printed into the web browser. +* `2` – Same as `1` but additional information (HTTP requests) will be logged when fetching feed content. +
+ ### `logger_destination`
diff --git a/src/common.php b/src/common.php index ec1044ec45..61bc148e9d 100644 --- a/src/common.php +++ b/src/common.php @@ -9,6 +9,7 @@ use Monolog\Handler\NullHandler; use Monolog\Handler\StreamHandler; use Monolog\Logger; +use Tracy\Debugger; require __DIR__ . '/constants.php'; require_once __DIR__ . '/helpers/responses.php'; @@ -20,28 +21,22 @@ exit; } -$startup_error = error_get_last(); - -// F3 crashes when there were PHP startups error even though -// they might not affect the program (e.g. unable to load an extension). -// It also sets its own error_reporting value and uses the previous one -// as a signal to disable the initialization failure check. -error_reporting(0); - -$f3 = Base::instance(); - -// Disable deprecation warnings. -// Dice uses ReflectionParameter::getClass(), which is deprecated in PHP 8 -// but we have not set an error handler yet because it needs a Logger instantiated by Dice. -error_reporting(E_ALL & ~E_DEPRECATED); - -$f3->set('AUTOLOAD', false); -$f3->set('BASEDIR', BASEDIR); +// Catch any errors and hopefully log them. +Debugger::$errorTemplate = __DIR__ . '/error.500.phtml'; +Debugger::enable(Debugger::PRODUCTION); $configuration = new Configuration(__DIR__ . '/../config.ini', $_ENV); -$f3->set('DEBUG', $configuration->debug); -$f3->set('cache', $configuration->cache); +if ($configuration->debug !== 0) { + // Enable strict mode to loudly fail on any error or warning. + // We ignore deprecation warnings because Dice uses deprecated + // ReflectionParameter::getClass(), which we cannot do anything about. + Debugger::$strictMode = E_ALL & ~E_DEPRECATED; + // Switch to development mode so that traces are displayed. + Debugger::enable(Debugger::DEVELOPMENT); + // Dispatch will not run in production mode preventing bar from loading. + Debugger::dispatch(); +} $dice = new Dice(); @@ -242,42 +237,5 @@ function($pattern, $text) { } $log->pushHandler($handler); -if (isset($startup_error)) { - $log->warn('PHP likely encountered a startup error: ', [$startup_error]); -} - -// init error handling -$f3->set('ONERROR', - function(Base $f3) use ($configuration, $log, $handler) { - $exception = $f3->get('EXCEPTION'); - - try { - if ($exception) { - $log->error($exception->getMessage(), ['exception' => $exception]); - } else { - $log->error($f3->get('ERROR.text')); - } - - if ($configuration->debug !== 0) { - echo 'An error occurred' . ': '; - echo $f3->get('ERROR.text') . "\n"; - echo $f3->get('ERROR.trace'); - } else { - if ($handler instanceof StreamHandler) { - echo 'An error occured, please check the log file “' . $handler->getUrl() . '”.' . PHP_EOL; - } elseif ($handler instanceof ErrorLogHandler) { - echo 'An error occured, please check your system logs.' . PHP_EOL; - } else { - echo 'An error occurred' . PHP_EOL; - } - } - } catch (Exception $e) { - echo 'Unable to write logs.' . PHP_EOL; - echo $e->getMessage() . PHP_EOL; - } - } -); - -if ($configuration->debug !== 0) { - ini_set('display_errors', '0'); -} +// Try to log errors encountered by error handler. +Debugger::setLogger($dice->create(helpers\TracyLogger::class)); diff --git a/src/error.500.phtml b/src/error.500.phtml new file mode 100644 index 0000000000..31d967633c --- /dev/null +++ b/src/error.500.phtml @@ -0,0 +1,50 @@ + + + + + +selfoss failed + + + +
+
+

selfoss failed

+ +

We’re sorry! selfoss encountered an unexpected error and was unable to complete your request.

+ +

Please contact your administrator or try again later.

+ +

If you are the administrator, + + please check the logs or add + + selfoss is unable to log the error, make sure the data/logs directory is writeable (if applicable), or try adding + + debug=1 to your selfoss instance configuration.

+ +

error 500 |

+
+
+ + diff --git a/src/helpers/Configuration.php b/src/helpers/Configuration.php index 31ab8cfb24..11f7ad4953 100644 --- a/src/helpers/Configuration.php +++ b/src/helpers/Configuration.php @@ -24,7 +24,7 @@ class Configuration { // Internal but overridable values. - /** @var int debugging level @internal */ + /** @var int debugging level */ public $debug = 0; /** @var string @internal */ diff --git a/src/helpers/TracyLogger.php b/src/helpers/TracyLogger.php new file mode 100644 index 0000000000..6aa6c734fc --- /dev/null +++ b/src/helpers/TracyLogger.php @@ -0,0 +1,52 @@ + Monolog\Logger::DEBUG, + self::INFO => Monolog\Logger::INFO, + self::WARNING => Monolog\Logger::WARNING, + self::ERROR => Monolog\Logger::ERROR, + self::EXCEPTION => Monolog\Logger::CRITICAL, + self::CRITICAL => Monolog\Logger::CRITICAL, + ]; + + /** @var Monolog\Logger */ + protected $monolog; + + public function __construct(Monolog\Logger $monolog) { + $this->monolog = $monolog; + } + + public function log($message, $priority = self::INFO) { + $context = [ + 'at' => Helpers::getSource(), + ]; + + if ($message instanceof Throwable || $message instanceof Exception) { + $context['exception'] = $message; + $message = ''; + } + + $this->monolog->addRecord( + self::PRIORITY_MAP[$priority] ?: Monolog\Logger::ERROR, + $message, + $context + ); + } +} diff --git a/tests/integration/helpers/selfoss_server.py b/tests/integration/helpers/selfoss_server.py index 024cd41676..6a71a82618 100644 --- a/tests/integration/helpers/selfoss_server.py +++ b/tests/integration/helpers/selfoss_server.py @@ -45,6 +45,7 @@ def run(self): 'SELFOSS_PASSWORD': bcrypt.hashpw(self.password.encode('utf-8'), bcrypt.gensalt()), 'SELFOSS_DB_TYPE': 'sqlite', 'SELFOSS_PUBLIC': '1', + 'SELFOSS_DEBUG': '1', 'SELFOSS_LOGGER_LEVEL': 'DEBUG', }