diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index 318c7b09..4f5cff0b 100644 --- a/actions/DisplayAction.php +++ b/actions/DisplayAction.php @@ -35,6 +35,9 @@ class DisplayAction implements ActionInterface throw new \Exception('This bridge is not whitelisted'); } + $formatFactory = new FormatFactory(); + $format = $formatFactory->create($format); + $bridge = $bridgeFactory->create($bridgeClassName); $bridge->loadConfiguration(); @@ -150,8 +153,7 @@ class DisplayAction implements ActionInterface 'icon' => $bridge->getIcon() ]; } catch (\Throwable $e) { - error_log($e); - + Logger::error(sprintf('Exception in %s', $bridgeClassName), ['e' => $e]); $errorCount = logBridgeError($bridge::NAME, $e->getCode()); if ($errorCount >= Configuration::getConfig('error', 'report_limit')) { @@ -163,23 +165,10 @@ class DisplayAction implements ActionInterface $message = sprintf('Bridge returned error %s! (%s)', $e->getCode(), $request['_error_time']); $item->setTitle($message); - - $item->setURI( - (isset($_SERVER['REQUEST_URI']) ? parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH) : '') - . '?' - . http_build_query($request) - ); - + $item->setURI(get_current_url()); $item->setTimestamp(time()); - $message = sprintf( - 'Uncaught Exception %s: %s at %s line %s', - get_class($e), - $e->getMessage(), - trim_path_prefix($e->getFile()), - $e->getLine() - ); - + $message = create_sane_exception_message($e); $content = render_template('bridge-error.html.php', [ 'message' => $message, 'stacktrace' => create_sane_stacktrace($e), @@ -204,8 +193,6 @@ class DisplayAction implements ActionInterface ]); } - $formatFactory = new FormatFactory(); - $format = $formatFactory->create($format); $format->setItems($items); $format->setExtraInfos($infos); $lastModified = $cache->getTime(); diff --git a/formats/AtomFormat.php b/formats/AtomFormat.php index c611226f..e67e8ade 100644 --- a/formats/AtomFormat.php +++ b/formats/AtomFormat.php @@ -18,12 +18,7 @@ class AtomFormat extends FormatAbstract public function stringify() { - $https = $_SERVER['HTTPS'] ?? null; - $urlPrefix = $https === 'on' ? 'https://' : 'http://'; - $urlHost = $_SERVER['HTTP_HOST'] ?? ''; - $urlRequest = $_SERVER['REQUEST_URI'] ?? ''; - - $feedUrl = $urlPrefix . $urlHost . $urlRequest; + $feedUrl = get_current_url(); $extraInfos = $this->getExtraInfos(); if (empty($extraInfos['uri'])) { diff --git a/formats/JsonFormat.php b/formats/JsonFormat.php index bb9e81a2..cb47f12d 100644 --- a/formats/JsonFormat.php +++ b/formats/JsonFormat.php @@ -25,18 +25,13 @@ class JsonFormat extends FormatAbstract public function stringify() { - $https = $_SERVER['HTTPS'] ?? null; - $urlPrefix = $https === 'on' ? 'https://' : 'http://'; - $urlHost = $_SERVER['HTTP_HOST'] ?? ''; - $urlRequest = $_SERVER['REQUEST_URI'] ?? ''; - + $host = $_SERVER['HTTP_HOST'] ?? ''; $extraInfos = $this->getExtraInfos(); - $data = [ 'version' => 'https://jsonfeed.org/version/1', - 'title' => (!empty($extraInfos['name'])) ? $extraInfos['name'] : $urlHost, - 'home_page_url' => (!empty($extraInfos['uri'])) ? $extraInfos['uri'] : REPOSITORY, - 'feed_url' => $urlPrefix . $urlHost . $urlRequest + 'title' => empty($extraInfos['name']) ? $host : $extraInfos['name'], + 'home_page_url' => empty($extraInfos['uri']) ? REPOSITORY : $extraInfos['uri'], + 'feed_url' => get_current_url(), ]; if (!empty($extraInfos['icon'])) { diff --git a/formats/MrssFormat.php b/formats/MrssFormat.php index f4067b73..3f0ed312 100644 --- a/formats/MrssFormat.php +++ b/formats/MrssFormat.php @@ -40,13 +40,7 @@ class MrssFormat extends FormatAbstract public function stringify() { - $https = $_SERVER['HTTPS'] ?? null; - $urlPrefix = $https == 'on' ? 'https://' : 'http://'; - $urlHost = $_SERVER['HTTP_HOST'] ?? ''; - $urlRequest = $_SERVER['REQUEST_URI'] ?? ''; - - $feedUrl = $urlPrefix . $urlHost . $urlRequest; - + $feedUrl = get_current_url(); $extraInfos = $this->getExtraInfos(); if (empty($extraInfos['uri'])) { $uri = REPOSITORY; diff --git a/lib/Debug.php b/lib/Debug.php index 316d5595..650b2a50 100644 --- a/lib/Debug.php +++ b/lib/Debug.php @@ -73,9 +73,6 @@ class Debug ); if (self::$enabled) { - ini_set('display_errors', '1'); - error_reporting(E_ALL); - self::$secure = !empty($debug_whitelist); } @@ -99,25 +96,18 @@ class Debug return self::$secure; } - /** - * Adds a debug message to error_log if debug mode is enabled - * - * @param string $text The message to add to error_log - */ - public static function log($text) + public static function log($message) { if (!self::isEnabled()) { return; } - $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3); - $calling = end($backtrace); - $message = $calling['file'] . ':' - . $calling['line'] . ' class ' - . ($calling['class'] ?? '') . '->' - . $calling['function'] . ' - ' - . $text; - - error_log($message); + $lastFrame = end($backtrace); + $file = trim_path_prefix($lastFrame['file']); + $line = $lastFrame['line']; + $class = $lastFrame['class'] ?? ''; + $function = $lastFrame['function']; + $text = sprintf('%s:%s %s->%s() %s', $file, $line, $class, $function, $message); + Logger::info($text); } } diff --git a/lib/FeedExpander.php b/lib/FeedExpander.php index 36043ffe..1b04c35c 100644 --- a/lib/FeedExpander.php +++ b/lib/FeedExpander.php @@ -101,8 +101,8 @@ abstract class FeedExpander extends BridgeAbstract ]; $httpHeaders = ['Accept: ' . implode(', ', $mimeTypes)]; $content = getContents($url, $httpHeaders); - $rssContent = simplexml_load_string(trim($content)); - + // Suppress php errors. We will check return value for success. + $rssContent = @simplexml_load_string(trim($content)); if ($rssContent === false) { throw new \Exception(sprintf('Unable to parse xml from "%s"', $url)); } diff --git a/lib/FeedItem.php b/lib/FeedItem.php index a5aa7035..7a32ef9a 100644 --- a/lib/FeedItem.php +++ b/lib/FeedItem.php @@ -164,7 +164,7 @@ class FeedItem FILTER_FLAG_PATH_REQUIRED ) ) { - Debug::log('URI must include a scheme, host and path!'); + Debug::log(sprintf('Not a valid url: "%s"', $uri)); } else { $scheme = parse_url($uri, PHP_URL_SCHEME); diff --git a/lib/Logger.php b/lib/Logger.php new file mode 100644 index 00000000..dcd945e3 --- /dev/null +++ b/lib/Logger.php @@ -0,0 +1,54 @@ +getFile()); + $context['line'] = $context['e']->getLine(); + $context['code'] = $context['e']->getCode(); + $context['url'] = get_current_url(); + $context['trace'] = create_sane_stacktrace($context['e']); + unset($context['e']); + $ignoredExceptions = [ + 'Exception Exception: You must specify a format!', + 'Exception InvalidArgumentException: Format name invalid!', + 'Exception InvalidArgumentException: Unknown format given!', + 'Exception InvalidArgumentException: Bridge name invalid!', + ]; + foreach ($ignoredExceptions as $ignoredException) { + if (str_starts_with($context['message'], $ignoredException)) { + // Don't log this record because it's usually a bot + return; + } + } + } + $text = sprintf( + "[%s] rssbridge.%s %s %s\n", + now()->format('Y-m-d H:i:s'), + $level, + $message, + $context ? Json::encode($context) : '' + ); + error_log($text); + } +} diff --git a/lib/RssBridge.php b/lib/RssBridge.php index a4a434e3..c4441b98 100644 --- a/lib/RssBridge.php +++ b/lib/RssBridge.php @@ -14,17 +14,10 @@ final class RssBridge try { $this->run($request); } catch (\Throwable $e) { - error_log($e); - $message = sprintf( - 'Uncaught Exception %s: %s at %s line %s', - get_class($e), - $e->getMessage(), - trim_path_prefix($e->getFile()), - $e->getLine() - ); + Logger::error('Exception in main', ['e' => $e]); http_response_code(500); print render('error.html.php', [ - 'message' => $message, + 'message' => create_sane_exception_message($e), 'stacktrace' => create_sane_stacktrace($e), ]); } @@ -40,6 +33,17 @@ final class RssBridge } Configuration::loadConfiguration($customConfig, getenv()); + set_error_handler(function ($code, $message, $file, $line) { + if ((error_reporting() & $code) === 0) { + return false; + } + $text = sprintf('%s at %s line %s', $message, trim_path_prefix($file), $line); + Logger::warning($text); + if (Debug::isEnabled()) { + print sprintf('
%s
', $text); + } + }); + date_default_timezone_set(Configuration::getConfig('system', 'timezone')); $authenticationMiddleware = new AuthenticationMiddleware(); diff --git a/lib/contents.php b/lib/contents.php index fdca42c6..d90b7546 100644 --- a/lib/contents.php +++ b/lib/contents.php @@ -12,6 +12,7 @@ const RSSBRIDGE_HTTP_STATUS_CODES = [ '205' => 'Reset Content', '206' => 'Partial Content', '300' => 'Multiple Choices', + '301' => 'Moved Permanently', '302' => 'Found', '303' => 'See Other', '304' => 'Not Modified', @@ -136,7 +137,8 @@ function getContents( default: throw new HttpException( sprintf( - '%s %s', + '%s resulted in `%s %s`', + $url, $result['code'], RSSBRIDGE_HTTP_STATUS_CODES[$result['code']] ?? '' ), @@ -231,7 +233,13 @@ function _http_request(string $url, array $config = []): array } if ($attempts > $config['retries']) { // Finally give up - throw new HttpException(sprintf('%s (%s)', curl_error($ch), curl_errno($ch))); + throw new HttpException(sprintf( + 'cURL error %s: %s (%s) for %s', + curl_error($ch), + curl_errno($ch), + 'https://curl.haxx.se/libcurl/c/libcurl-errors.html', + $url + )); } } diff --git a/lib/utils.php b/lib/utils.php index de8ae8a8..f2f613e9 100644 --- a/lib/utils.php +++ b/lib/utils.php @@ -19,13 +19,13 @@ final class Json } /** - * Returns e.g. 'https://example.com/' or 'https://example.com/bridge/' + * Get the home page url of rss-bridge e.g. 'https://example.com/' or 'https://example.com/bridge/' */ function get_home_page_url(): string { - $https = $_SERVER['HTTPS'] ?? null; - $host = $_SERVER['HTTP_HOST'] ?? null; - $uri = $_SERVER['REQUEST_URI'] ?? null; + $https = $_SERVER['HTTPS'] ?? ''; + $host = $_SERVER['HTTP_HOST'] ?? ''; + $uri = $_SERVER['REQUEST_URI'] ?? ''; if (($pos = strpos($uri, '?')) !== false) { $uri = substr($uri, 0, $pos); } @@ -33,6 +33,29 @@ function get_home_page_url(): string return "$scheme://$host$uri"; } +/** + * Get the full current url e.g. 'http://example.com/?action=display&bridge=FooBridge' + */ +function get_current_url(): string +{ + $https = $_SERVER['HTTPS'] ?? ''; + $host = $_SERVER['HTTP_HOST'] ?? ''; + $uri = $_SERVER['REQUEST_URI'] ?? ''; + $scheme = $https === 'on' ? 'https' : 'http'; + return "$scheme://$host$uri"; +} + +function create_sane_exception_message(\Throwable $e): string +{ + return sprintf( + 'Exception %s: %s at %s line %s', + get_class($e), + $e->getMessage(), + trim_path_prefix($e->getFile()), + $e->getLine() + ); +} + function create_sane_stacktrace(\Throwable $e): array { $frames = array_reverse($e->getTrace()); @@ -40,18 +63,18 @@ function create_sane_stacktrace(\Throwable $e): array 'file' => $e->getFile(), 'line' => $e->getLine(), ]; - $stackTrace = []; + $trace = []; foreach ($frames as $i => $frame) { $file = $frame['file'] ?? '(no file)'; $line = $frame['line'] ?? '(no line)'; - $stackTrace[] = sprintf( + $trace[] = sprintf( '#%s %s:%s', $i, trim_path_prefix($file), $line, ); } - return $stackTrace; + return $trace; } /** @@ -151,3 +174,8 @@ function format_bytes(int $bytes, $precision = 2) return round($bytes, $precision) . ' ' . $units[$pow]; } + +function now(): \DateTimeImmutable +{ + return new \DateTimeImmutable(); +}