Update to v1.5.6 #3
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and the tests that use them. This will be necessary for higher levels of PHPStan.
Needed for PHPStan level 1 to pass. Keeping the duplicates to prevent people from re-adding them. Choosing Gulf Standard Time over South Georgia and the South Sandwich Islands Time since the former is used by UAE and Oman, rather than few small islands.
Needed for PHPStan level 2 to pass.
Introduced in a2a2268, `Misc::SIMPLEPIE_BUILD` is private so accessing it through `static` will not work unless the child class redefines the property. Needed for PHPStan level 2 to pass.
Multifeed support makes error handling confusing (sometimes `SimplePie::get_error()` returns string and other times an array). And that will only get worse once we start throwing exceptions. Additionally, it is ugly on conceptual level (single responsibility principle).
SPDX is a standard for describing licensing and copyright status of project files that has been gaining traction lately. In addition to not filling more than half a screen, it is machine readable and can be used to produce Software Bill of Materials. We are keeping the long header in the compiled single-file bundle since that might be copied into third-party projects in a way that makes it easy to accidentally forget also copying the license text. https://reuse.software/tutorial/ https://www.linuxfoundation.org/blog/blog/solving-license-compliance-at-the-source-adding-spdx-license-ids
- Properties are usually nullable - Add constructor typehints - Elaborate array types for getters and properties
The `File` class handles redirects by calling the constructor recursively. To avoid setting the `permanent_url` incorrectly at the top of the constructor, the value of the property was reset after each constructor call. And since local variables were used, only the reset following the first internal constructor call would actually do anything. This was very confusing and would stop considering permanent redirects after the first one. Let’s avoid doing stuff after popping the stack, tail recursion FTW. Additionally, the `permanent_url` would only be updated when `HTTP 301 Moved Permanently` status code was encountered. Let’s also support `HTTP 308 Permanent Redirect`. Tracking the mutability in a property is not the cleanest but a bigger refactoring is probably not worth it with the upcoming deprecation of File. Unfortunately, we cannot easily test this since we cannot mock curl.
Having multiple classes in a single file goes against PSR-1 and we already have SuccessException defined separately.
It is pointless.
We will use SimplePieException as an alias for SimplePie\Exception since using it unqualified is just confusing.
This makes it easier for tools to verify that a mentioned class exists.
The tests would complain because of the extra apostrophe outside of the root element:
PHP Notice: is invalid XML, likely due to invalid characters. XML error: Invalid document end at line 5, column 11 in src/SimplePie.php on line 1708
The matches are strings making PHPStan level 2 complain we were doing arithmetic operations on them. While at it, let’s use auxiliary variables up front to make the code a bit cleaner.
Needed for PHPStan level 2 to pass. `DOMXpath::query` returns `DOMNodeList<DOMNode>` but here the XPath query looks for a or link elements so we can be more specific.
The `idna_convert` library is outdated and there is now a native PHP function to convert IDNs to Punycode. It requires `intl` extension or a [polyfill](https://github.com/symfony/polyfill-intl-idn). `compress_parse_url` will append `?` and `#` to the URI even if there is no query string or hash fragment, and insert `//` even when there is no authority part. To avoid this and the breakage of tests this would cause, let’s call the function only for files having a non-empty authority part of the URI (i.e. non-local files) that contains characters that are not considered printable ASCII (such as bytes of UTF-8 encoded Unicode characters). In the future, we might leave the IDN support to HTTP library to deal with.
Type hint prevents anything else being passed.
As pointed out by PHPStan, it did not expect them:
Method SimplePie\Sanitize::pass_file_data() invoked with 5 parameters, 0-4 required
This has been broken since the support for passing curl options was introduced in
51f9f08
Needed for PHPStan level 2 to pass. This is not complete (there are more magic properties) and the types are conservative (e.g. path accepts setting `null` which will be converted to an empty string) but this class is going away in the future and this is sufficient for our internal use.
…#821) PHPStan would complain: Method Mock_CacheLegacy::get_handler() should return SimplePie\Cache\Base but return statement is missing. Let’s throw an exception to properly terminate the method body.
Because we support `psr/simple-cache` 1.0, which gets installed on PHP 7,
and `Psr\SimpleCache\InvalidArgumentException` interface does not extend
`Throwable` in that version, PHPStan would complain:
PHPDoc tag @throws with type Psr\SimpleCache\InvalidArgumentException is not subtype of Throwable
Let’s declare we actually throw a `Throwable` using an intersection type.
…o release-1.9.0
Several PHP functions that have not been doing anything since PHP 8.0/8.1 will be deprecated in PHP 8.5 and will thus be throwing warnings: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_no-op_functions_from_the_resource_to_object_conversion To solve this while supporting multiple versions of PHP, they can be called conditionally based on a PHP version check. This PR adds this for affected functions throughout the SimplePie codebase.
Release 1.9.0
SimplePie uses uppercase for encoding everywhere, except in the parser, which makes `array_unique()` to fail to do its job when the document declares `utf-8` and we already have `UTF-8` in the list: https://github.com/simplepie/simplepie/blob/1838b28db27ee206e0c2cf183f7b2af75abed143/src/SimplePie.php#L1780-L1785
Flagged by PHPStan (strict mode). Found during #939 [PHPUnit documentation](https://docs.phpunit.de/en/12.3/assertions.html) proposes to use either `$this->assertTrue()` or `self::assertTrue()` as discussed in sebastianbergmann/phpunit#1914. A static method should be called statically (I could not even find in the official PHP documentation anything that suggests that it should be allowed otherwise, but it does still work so far): * https://phpstan.org/error-identifiers/staticMethod.dynamicCall * phpstan/phpstan#514
This type hint is a syntax only supported by PHPStan, so it generates errors in other systems such as Intellephense
After updating selfoss to SimplePie 1.9.0, PHPStan started to complain:
Parameter #1 $level of method SimplePie\SimplePie::set_autodiscovery_level() expects 0|1|2|4|8|16|31, 7 given.
Yet, the PHPDoc claimed:
level can be a combination of the above constants, see bitwise OR operator
Let’s swtich to `int-mask-of`.
`CURLINFO_REDIRECT_COUNT` is only non-zero when `CURLOPT_FOLLOWLOCATION` is enabled but we track redirects ourselves since 692e8bc. https://curl.se/libcurl/c/CURLOPT_FOLLOWLOCATION.html https://curl.se/libcurl/c/CURLINFO_REDIRECT_COUNT.html
Since starting to handle redirects ourselves in 692e8bc, `url` info item (aka `CURLINFO_EFFECTIVE_URL`) will be the same as the initial URI.
In fact, when the server sends interim responses, this was counterproductive – it would overwrite the final response code returned by `CURLINFO_HTTP_CODE` with the interim one (e.g. 100). Though, the parser will still include the headers of all but the first response in the body.
Explicit comparison is clearer than relying on `CURLE_OK` being falsey (`0`).
This is only needed for PHPStan. Making the type assertion explicit and moving it earlier will allow us to make the `prepareHeaders` call conditional without having to add more casts.
The method call was introduced in 4b1fc33 to support tunneling through HTTP proxy using CONNECT method, whose response curl includes in the `curl_exec` result. Ideally, we would prevent that with `CURLOPT_SUPPRESS_CONNECT_HEADERS` but that is only available in PHP 7.3: https://www.php.net/manual/en/curl.constants.php#constant.curlopt-suppress-connect-headers
The implementation of magic_quotes_gpc was removed in PHP 5.4 and the function for checking was dropped in PHP 8.0: https://www.php.net/manual/en/function.get-magic-quotes-gpc.php
This will slightly simplify the code.
It is an ancient technology for using custom fonts based on Flash. It will not work in any modern browser.
The test runner was removed in 0fcde72.
Multifeed has been deprecated since ff1b513.
This is a new deprecation in PHP 8.5. See https://wiki.php.net/rfc/deprecations_php_8_5 / php/php-src@49e3956
This is no-op outside multi-feed mode, which has already been deprecated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mostly bug and issue fixes, some new features.
Redmine issue: https://redmine.pfsense.org/issues/11508