From 659bfbd346e94e2dfdb5397ba30ef34a9b761d51 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Thu, 15 Jan 2026 10:02:00 +0100 Subject: [PATCH 1/2] tests: Remove unused media:thumbnail element in data Currently, we only use these to test code paths that do not involve parsing but it would be nice to be able to reuse the tests for other simple tests. Unfortunately, `Parser::parse` will actually fail with `error_string == "Unknown"`. That is because `xml_get_error_code()` returns 201, which is not one of XML_ERROR_* codes: https://www.php.net/manual/en/xml.error-codes.php It looks like it is actually `XML_NS_ERR_UNDEFINED_NAMESPACE` leaking from libxml2. I can confirm it with `libxml_use_internal_errors(true);` and dumping `libxml_get_errors()`: Namespace prefix media on thumbnail is not defined Since we never use it anywhere, we can just drop the element. --- tests/data/feed-utf16be.xml | Bin 964 -> 816 bytes tests/data/feed-utf16le.xml | Bin 964 -> 816 bytes tests/data/feed_rss-2.0.xml | 1 - 3 files changed, 1 deletion(-) diff --git a/tests/data/feed-utf16be.xml b/tests/data/feed-utf16be.xml index 2375ba1207b564564cca0048a3a136fcf7430768..49454fd263743cb1c3bac6f2de41170b60daace6 100644 GIT binary patch delta 11 ScmX@YzJYDSPo~Me%;o?c{RD>q delta 143 zcmdnMc7%PyPo~NJOkDN345gfi47m(R40%9)CPNN`0+3$>q-`0L z81#X3CQv4u!5%1U%b>aqsvK<23d<#QPdfa1wubva-)DL^&pV3rclTzv*R M1}+AL$?uq)0J-lQ*Z=?k diff --git a/tests/data/feed-utf16le.xml b/tests/data/feed-utf16le.xml index 3f308fd855b73d577e72918a86b18ef6132abdd7..dbfdc73669284e0d12a00aacbe896620ba08342f 100644 GIT binary patch delta 11 ScmX@YzJYDS52ndI%oYG0^#q0h delta 143 zcmdnMc7%Py52nd|OkDN345gfi47m(R40%9)CPNN`0+3$>q-`0L z81#X3CQv4u!5%1U%b>aqsvK<23d<#QPdfa1wubva-)DL^&pV3rclTzv*R M1}+AL$#0pQ0l3^7)&Kwi diff --git a/tests/data/feed_rss-2.0.xml b/tests/data/feed_rss-2.0.xml index c358eb6dc..beb6b0372 100644 --- a/tests/data/feed_rss-2.0.xml +++ b/tests/data/feed_rss-2.0.xml @@ -8,7 +8,6 @@ Test feed description 1 http://example.net/tests/#1.1 http://example.net/tests/#1.1 - From d060f04e144e8def0b423002eeb76da9a451c081 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Sun, 18 Jan 2026 09:28:28 +0100 Subject: [PATCH 2/2] Fixes before phpstan2 - Improve typing and fix issues that will trip up PHPStan 2.0. - Fix insufficient regex escaping in `SimplePie::get_links()`. - Add editor config for .neon files. - Add PHPStan results cache to .gitignore. - Check our PHPStan extension. - Use `assertNull` in tests instead of `assertSame`. --- .editorconfig | 4 ++++ .gitignore | 1 + autoloader.php | 3 ++- demo/test.php | 2 +- library/SimplePie/Decode/HTML/Entities.php | 2 ++ phpstan.dist.neon | 1 + src/File.php | 7 ++++--- src/HTTP/Parser.php | 4 ++-- src/Registry.php | 1 - src/Sanitize.php | 6 ++++++ src/SimplePie.php | 7 ++++--- tests/IRITest.php | 5 +++-- tests/LocatorTest.php | 4 ++-- tests/Unit/IRITest.php | 4 ++-- tests/Unit/LocatorTest.php | 4 ++-- 15 files changed, 36 insertions(+), 19 deletions(-) diff --git a/.editorconfig b/.editorconfig index a760cdf84..13c02021c 100644 --- a/.editorconfig +++ b/.editorconfig @@ -19,6 +19,10 @@ indent_style = space max_line_length = off trim_trailing_whitespace = false +[*.neon] +indent_size = 4 +indent_style = space + [*.php] indent_size = 4 indent_style = space diff --git a/.gitignore b/.gitignore index 805c1f9b9..611895a6d 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ phpstan.neon phpunit.xml .php-cs-fixer.cache .phpunit.cache/ +.phpunit.result.cache diff --git a/autoloader.php b/autoloader.php index 4356b456f..1ae88be64 100644 --- a/autoloader.php +++ b/autoloader.php @@ -57,6 +57,7 @@ */ class SimplePie_Autoloader { + /** @var string */ protected $path; /** @@ -72,7 +73,7 @@ public function __construct() * * @param string $class The name of the class to attempt to load. */ - public function autoload($class) + public function autoload(string $class): void { // Only load the class if it starts with "SimplePie" if (strpos($class, 'SimplePie') !== 0) diff --git a/demo/test.php b/demo/test.php index 090ee94f5..2048bdc0c 100644 --- a/demo/test.php +++ b/demo/test.php @@ -37,7 +37,7 @@ } // Output buffer -function callable_htmlspecialchars($string) +function callable_htmlspecialchars(string $string): string { return htmlspecialchars($string); } diff --git a/library/SimplePie/Decode/HTML/Entities.php b/library/SimplePie/Decode/HTML/Entities.php index 3366526a5..639a8c257 100644 --- a/library/SimplePie/Decode/HTML/Entities.php +++ b/library/SimplePie/Decode/HTML/Entities.php @@ -72,6 +72,7 @@ public function parse() * * @access private * @return string|false The next byte, or false, if there is no more data + * @phpstan-impure */ public function consume() { @@ -547,6 +548,7 @@ public function entity() 'zwnj;' => "\xE2\x80\x8C" ]; + $consumed = ''; for ($i = 0, $match = null; $i < 9 && $this->consume() !== false; $i++) { // Cast for PHPStan on PHP < 8.0: We consumed as per the loop condition, // so `$this->consumed` is non-empty and the substr offset is valid. diff --git a/phpstan.dist.neon b/phpstan.dist.neon index da1d19fc6..848a033a1 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -5,6 +5,7 @@ parameters: - library/ - src/ - tests/ + - utils/ ignoreErrors: # Ignore that only one const exists atm diff --git a/src/File.php b/src/File.php index e0ce712be..0e00e81fd 100644 --- a/src/File.php +++ b/src/File.php @@ -126,10 +126,11 @@ public function __construct(string $url, int $timeout = 10, int $redirects = 5, } else { curl_setopt($fp, CURLOPT_ENCODING, ''); } + /** @var non-empty-string $url */ curl_setopt($fp, CURLOPT_URL, $url); - curl_setopt($fp, CURLOPT_HEADER, 1); - curl_setopt($fp, CURLOPT_RETURNTRANSFER, 1); - curl_setopt($fp, CURLOPT_FAILONERROR, 1); + curl_setopt($fp, CURLOPT_HEADER, true); + curl_setopt($fp, CURLOPT_RETURNTRANSFER, true); + curl_setopt($fp, CURLOPT_FAILONERROR, true); curl_setopt($fp, CURLOPT_TIMEOUT, $timeout); curl_setopt($fp, CURLOPT_CONNECTTIMEOUT, $timeout); curl_setopt($fp, CURLOPT_REFERER, \SimplePie\Misc::url_remove_credentials($url)); diff --git a/src/HTTP/Parser.php b/src/HTTP/Parser.php index d329be1c2..55483f6ff 100644 --- a/src/HTTP/Parser.php +++ b/src/HTTP/Parser.php @@ -243,7 +243,7 @@ private function add_header(string $name, string $value): void $headers[$name][] = $value; } else { // For PHPStan: should be enforced by template parameter but PHPStan is not smart enough. - /** @var array) */ + /** @var array */ $headers = &$this->headers; $headers[$name] .= ', ' . $value; } @@ -258,7 +258,7 @@ private function replace_header(string $name, string $value): void $headers[$name] = [$value]; } else { // For PHPStan: should be enforced by template parameter but PHPStan is not smart enough. - /** @var array) */ + /** @var array */ $headers = &$this->headers; $headers[$name] = $value; } diff --git a/src/Registry.php b/src/Registry.php index 228f8c33c..c02652bb8 100644 --- a/src/Registry.php +++ b/src/Registry.php @@ -125,7 +125,6 @@ public function register(string $type, $class, bool $legacy = false) return false; } - /** @var string */ $base_class = $this->default[$type]; if (!is_subclass_of($class, $base_class)) { diff --git a/src/Sanitize.php b/src/Sanitize.php index c8aa2dce6..45be4fd3c 100644 --- a/src/Sanitize.php +++ b/src/Sanitize.php @@ -656,6 +656,9 @@ protected function strip_tag(string $tag, DOMDocument $document, DOMXPath $xpath if ($this->encode_instead_of_strip) { foreach ($elements as $element) { + if (!($element instanceof \DOMNode)) { + continue; + } $fragment = $document->createDocumentFragment(); // For elements which aren't script or style, include the tag itself @@ -712,6 +715,9 @@ protected function strip_tag(string $tag, DOMDocument $document, DOMXPath $xpath return; } else { foreach ($elements as $element) { + if (!($element instanceof \DOMNode)) { + continue; + } $fragment = $document->createDocumentFragment(); $number = $element->childNodes->length; for ($i = $number; $i > 0; $i--) { diff --git a/src/SimplePie.php b/src/SimplePie.php index 6eaaeebfe..c243b274e 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1709,7 +1709,8 @@ public function init() $single_success = $this->multifeed_objects[$i]->init(); $success |= $single_success; if (!$single_success) { - $this->error[$i] = $this->multifeed_objects[$i]->error(); + $error = $this->multifeed_objects[$i]->error() ?? ''; + $this->error[$i] = is_string($error) ? $error : implode('; ', $error); } $i++; } @@ -1812,7 +1813,7 @@ public function init() // Cache the file if caching is enabled $this->data['cache_expiration_time'] = $this->cache_duration + time(); - if ($cache && !$cache->set_data($this->get_cache_filename($this->feed_url), $this->data, $this->cache_duration)) { + if ($cache instanceof DataCache && !$cache->set_data($this->get_cache_filename($this->feed_url), $this->data, $this->cache_duration)) { trigger_error("$this->cache_location is not writable. Make sure you've set the correct relative or absolute path, and that the location is server-writable.", E_USER_WARNING); } return true; @@ -2862,7 +2863,7 @@ public function get_links(string $rel = 'alternate') } // https://datatracker.ietf.org/doc/html/rfc8288 if (is_string($link_headers) && - preg_match_all('/<(?P[^>]+)>\s*;\s*rel\s*=\s*(?P"?)' . preg_quote($rel) . '(?P=quote)\s*(?=,|$)/i', $link_headers, $matches)) { + preg_match_all('/<(?P[^>]+)>\s*;\s*rel\s*=\s*(?P"?)' . preg_quote($rel, '/') . '(?P=quote)\s*(?=,|$)/i', $link_headers, $matches)) { return $matches['uri']; } } diff --git a/tests/IRITest.php b/tests/IRITest.php index 676c31070..0176d9966 100644 --- a/tests/IRITest.php +++ b/tests/IRITest.php @@ -473,6 +473,7 @@ function ($errno, $errstr): bool { E_USER_NOTICE ); + // @phpstan-ignore property.notFound (we want to test that it fails) $should_fail = $iri->nonexistent_prop; } @@ -481,7 +482,7 @@ public function testBlankHost(): void $iri = new SimplePie_IRI('http://example.com/a/?b=c#d'); $iri->host = null; - self::assertSame(null, $iri->host); + self::assertNull($iri->host); self::assertSame('http:/a/?b=c#d', (string) $iri); } @@ -490,6 +491,6 @@ public function testBadPort(): void $iri = new SimplePie_IRI(); $iri->port = 'example'; - self::assertSame(null, $iri->port); + self::assertNull($iri->port); } } diff --git a/tests/LocatorTest.php b/tests/LocatorTest.php index 181ecafb5..b8c7b25fe 100755 --- a/tests/LocatorTest.php +++ b/tests/LocatorTest.php @@ -59,7 +59,7 @@ public function testInvalidMIMEType(): void $locator->set_registry($registry); $feed = $locator->find(SIMPLEPIE_LOCATOR_ALL, $all); - self::assertSame(null, $feed); + self::assertNull($feed); } public function testDirectNoDOM(): void @@ -140,7 +140,7 @@ public function test_from_file(File $data): void self::assertInstanceOf(FileMock::class, $feed); $success = array_filter($expected, [get_class($this), 'filter_success']); - $found = array_map([get_class($this), 'map_url_file'], $all); + $found = is_array($all) ? array_map([get_class($this), 'map_url_file'], $all) : []; self::assertSame($success, $found); } diff --git a/tests/Unit/IRITest.php b/tests/Unit/IRITest.php index d744d70c2..a4064ae3d 100644 --- a/tests/Unit/IRITest.php +++ b/tests/Unit/IRITest.php @@ -491,7 +491,7 @@ public function testBlankHost(): void $iri = new IRI('http://example.com/a/?b=c#d'); $iri->host = null; - self::assertSame(null, $iri->host); + self::assertNull($iri->host); self::assertSame('http:/a/?b=c#d', (string) $iri); } @@ -500,6 +500,6 @@ public function testBadPort(): void $iri = new IRI(); $iri->port = 'example'; - self::assertSame(null, $iri->port); + self::assertNull($iri->port); } } diff --git a/tests/Unit/LocatorTest.php b/tests/Unit/LocatorTest.php index 799be1e4e..59c1dda51 100644 --- a/tests/Unit/LocatorTest.php +++ b/tests/Unit/LocatorTest.php @@ -72,7 +72,7 @@ public function testInvalidMIMEType(): void $locator->set_registry($registry); $feed = $locator->find(SimplePie::LOCATOR_ALL, $all); - self::assertSame(null, $feed); + self::assertNull($feed); } public function testDirectNoDOM(): void @@ -153,7 +153,7 @@ public function test_from_file(File $data): void self::assertInstanceOf(FileMock::class, $feed); $success = array_filter($expected, [get_class($this), 'filter_success']); - $found = array_map([get_class($this), 'map_url_file'], $all); + $found = is_array($all) ? array_map([get_class($this), 'map_url_file'], $all) : []; self::assertSame($success, $found); }