mirror of
git://develop.git.wordpress.org/
synced 2025-02-16 20:54:42 +01:00
Code Modernization: Check the return type of parse_url()
in download_url()
.
As per the PHP manual: > If the `component` parameter is omitted, an associative array is returned. > If the `component` parameter is specified, `parse_url()` returns a string (or an int, in the case of `PHP_URL_PORT`) instead of an array. If the requested component doesn't exist within the given URL, `null` will be returned. Reference: [https://www.php.net/manual/en/function.parse-url.php#refsect1-function.parse-url-returnvalues PHP Manual: parse_url(): Return Values] This commit adds three unit tests for `download_url()`: * The first test is "girl-scouting" to make sure that the code up to the point where the error is expected is tested. * The second test exposed a PHP 8.1 `basename(): Passing null to parameter #1 ($path) of type string is deprecated` error due to the call to `parse_url()` returning `null` when the component requested does not exist in the passed URL. * The output of the call to `parse_url()` stored in the `$url_path` variable is used in more places in the function logic. The third test exposes a second PHP 8.1 deprecation notice, this time for `substr(): Passing null to parameter #1 ($string) of type string is deprecated`. This commit also removes duplicate `parse_url()` calls. Neither `$url` nor `$url_filename` are changed between when they are first received/defined and when they are re-used, so there is no need to repeat the function calls. Follow-up to [51606], [51622]. Props jrf, hellofromTonya, SergeyBiryukov. See #53635. git-svn-id: https://develop.svn.wordpress.org/trunk@51626 602fd350-edb4-49c9-b593-d223f7449a82
This commit is contained in:
parent
d7c22d29cf
commit
5f5895bf61
@ -1126,7 +1126,11 @@ function download_url( $url, $timeout = 300, $signature_verification = false ) {
|
||||
return new WP_Error( 'http_no_url', __( 'Invalid URL Provided.' ) );
|
||||
}
|
||||
|
||||
$url_filename = basename( parse_url( $url, PHP_URL_PATH ) );
|
||||
$url_path = parse_url( $url, PHP_URL_PATH );
|
||||
$url_filename = '';
|
||||
if ( is_string( $url_path ) && '' !== $url_path ) {
|
||||
$url_filename = basename( $url_path );
|
||||
}
|
||||
|
||||
$tmpfname = wp_tempnam( $url_filename );
|
||||
if ( ! $tmpfname ) {
|
||||
@ -1212,9 +1216,8 @@ function download_url( $url, $timeout = 300, $signature_verification = false ) {
|
||||
// WordPress.org stores signatures at $package_url.sig.
|
||||
|
||||
$signature_url = false;
|
||||
$url_path = parse_url( $url, PHP_URL_PATH );
|
||||
|
||||
if ( '.zip' === substr( $url_path, -4 ) || '.tar.gz' === substr( $url_path, -7 ) ) {
|
||||
if ( is_string( $url_path ) && ( '.zip' === substr( $url_path, -4 ) || '.tar.gz' === substr( $url_path, -7 ) ) ) {
|
||||
$signature_url = str_replace( $url_path, $url_path . '.sig', $url );
|
||||
}
|
||||
|
||||
@ -1243,7 +1246,7 @@ function download_url( $url, $timeout = 300, $signature_verification = false ) {
|
||||
}
|
||||
|
||||
// Perform the checks.
|
||||
$signature_verification = verify_file_signature( $tmpfname, $signature, basename( parse_url( $url, PHP_URL_PATH ) ) );
|
||||
$signature_verification = verify_file_signature( $tmpfname, $signature, $url_filename );
|
||||
}
|
||||
|
||||
if ( is_wp_error( $signature_verification ) ) {
|
||||
|
@ -77,4 +77,78 @@ class Tests_Admin_includesFile extends WP_UnitTestCase {
|
||||
public function __return_5() {
|
||||
return 5;
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify that a WP_Error object is returned when invalid input is passed as the `$url` parameter.
|
||||
*
|
||||
* @covers ::download_url
|
||||
* @dataProvider data_download_url_empty_url
|
||||
*
|
||||
* @param mixed $url Input URL.
|
||||
*/
|
||||
public function test_download_url_empty_url( $url ) {
|
||||
$error = download_url( $url );
|
||||
$this->assertWPError( $error );
|
||||
$this->assertSame( 'http_no_url', $error->get_error_code() );
|
||||
$this->assertSame( 'Invalid URL Provided.', $error->get_error_message() );
|
||||
}
|
||||
|
||||
/**
|
||||
* Data provider.
|
||||
*
|
||||
* @return array
|
||||
*/
|
||||
public function data_download_url_empty_url() {
|
||||
return array(
|
||||
'null' => array( null ),
|
||||
'false' => array( false ),
|
||||
'integer 0' => array( 0 ),
|
||||
'empty string' => array( '' ),
|
||||
'string 0' => array( '0' ),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that PHP 8.1 "passing null to non-nullable" deprecation notice
|
||||
* is not thrown when the `$url` does not have a path component.
|
||||
*
|
||||
* @ticket 53635
|
||||
* @covers ::download_url
|
||||
*/
|
||||
public function test_download_url_no_warning_for_url_without_path() {
|
||||
$result = download_url( 'https://example.com' );
|
||||
|
||||
$this->assertIsString( $result );
|
||||
$this->assertNotEmpty( $result ); // File path will be generated, but will never be empty.
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that PHP 8.1 "passing null to non-nullable" deprecation notice
|
||||
* is not thrown when the `$url` does not have a path component,
|
||||
* and signature verification via a local file is requested.
|
||||
*
|
||||
* @ticket 53635
|
||||
* @covers ::download_url
|
||||
*/
|
||||
public function test_download_url_no_warning_for_url_without_path_with_signature_verification() {
|
||||
add_filter(
|
||||
'wp_signature_hosts',
|
||||
static function( $urls ) {
|
||||
$urls[] = 'example.com';
|
||||
return $urls;
|
||||
}
|
||||
);
|
||||
$error = download_url( 'https://example.com', 300, true );
|
||||
|
||||
/*
|
||||
* Note: This test is not testing the signature verification itself.
|
||||
* There is no signature available for the domain used in the test,
|
||||
* which is why an error is expected and that's fine.
|
||||
* The point of the test is to verify that the call to `verify_file_signature()`
|
||||
* is actually reached and that no PHP deprecation notice is thrown
|
||||
* before this point.
|
||||
*/
|
||||
$this->assertWPError( $error );
|
||||
$this->assertSame( 'signature_verification_no_signature', $error->get_error_code() );
|
||||
}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user