From 676fcf43792a1199619f95ecf836a8d84b58d26a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= Date: Sat, 20 May 2023 14:16:32 +0200 Subject: [PATCH] Replace substr with cleaner string methods Use str_starts_with, str_ends_with or string offset where appropriate. This fixes a bug in MimeAnalyzer where the "UTF-16LE" header could not be identified because of wrong constant. This is the exact type of bug that the new functions can avoid. Change-Id: I9f30881e7e895f011db29cf5dcbe43bc4f341062 --- img_auth.php | 2 +- includes/OutputPage.php | 9 +++++---- includes/Request/FauxResponse.php | 2 +- includes/Request/PathRouter.php | 2 +- includes/ResourceLoader/OOUIIconPackModule.php | 2 +- includes/ResourceLoader/OOUIImageModule.php | 2 +- includes/WebRequest.php | 5 ++--- includes/libs/ParamValidator/TypeDef/UploadDef.php | 2 +- includes/libs/filebackend/SwiftFileBackend.php | 2 +- includes/libs/mime/MimeAnalyzer.php | 8 ++++---- includes/libs/objectcache/MemcachedBagOStuff.php | 5 ++--- includes/libs/rdbms/database/DatabasePostgres.php | 2 +- includes/registration/MissingExtensionException.php | 2 +- includes/upload/UploadBase.php | 2 +- maintenance/convertExtensionToRegistration.php | 2 +- maintenance/dumpCategoriesAsRdf.php | 2 +- maintenance/generateSitemap.php | 4 ++-- maintenance/importSiteScripts.php | 2 +- maintenance/includes/Benchmarker.php | 2 +- maintenance/update.php | 2 +- maintenance/updateCredits.php | 2 +- tests/phpunit/includes/api/ApiOptionsTest.php | 2 +- tests/phpunit/includes/db/DatabaseTestHelper.php | 2 +- tests/phpunit/structure/AutoLoaderStructureTest.php | 2 +- .../unit/includes/registration/ExtensionRegistryTest.php | 4 ++-- 25 files changed, 36 insertions(+), 37 deletions(-) diff --git a/img_auth.php b/img_auth.php index 992d33ba8f4..bd2106b0ff5 100644 --- a/img_auth.php +++ b/img_auth.php @@ -135,7 +135,7 @@ function wfImageAuthMain() { $filename = $repo->getZonePath( 'public' ) . $path; // Check to see if the file exists and is not deleted $bits = explode( '!', $name, 2 ); - if ( substr( $path, 0, 9 ) === '/archive/' && count( $bits ) == 2 ) { + if ( str_starts_with( $path, '/archive/' ) && count( $bits ) == 2 ) { $file = $repo->newFromArchiveName( $bits[1], $name ); } else { $file = $repo->newFile( $name ); diff --git a/includes/OutputPage.php b/includes/OutputPage.php index a6be95fac59..e1b5f48a4a6 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -4410,9 +4410,10 @@ class OutputPage extends ContextSource { $media = 'all'; } - if ( substr( $style, 0, 1 ) == '/' || - substr( $style, 0, 5 ) == 'http:' || - substr( $style, 0, 6 ) == 'https:' ) { + if ( str_starts_with( $style, '/' ) || + str_starts_with( $style, 'http:' ) || + str_starts_with( $style, 'https:' ) + ) { $url = $style; } else { $config = $this->getConfig(); @@ -4463,7 +4464,7 @@ class OutputPage extends ContextSource { } else { $remotePath = $remotePathPrefix; } - if ( strpos( $path, $remotePath ) !== 0 || substr( $path, 0, 2 ) === '//' ) { + if ( !str_starts_with( $path, $remotePath ) || str_starts_with( $path, '//' ) ) { // - Path is outside wgResourceBasePath, ignore. // - Path is protocol-relative. Fixes T155310. Not supported by RelPath lib. return $path; diff --git a/includes/Request/FauxResponse.php b/includes/Request/FauxResponse.php index 6154efef73d..4aee59d89dd 100644 --- a/includes/Request/FauxResponse.php +++ b/includes/Request/FauxResponse.php @@ -45,7 +45,7 @@ class FauxResponse extends WebResponse { * @param null|int $http_response_code Forces the HTTP response code to the specified value. */ public function header( $string, $replace = true, $http_response_code = null ) { - if ( substr( $string, 0, 5 ) == 'HTTP/' ) { + if ( str_starts_with( $string, 'HTTP/' ) ) { $parts = explode( ' ', $string, 3 ); $this->code = intval( $parts[1] ); } else { diff --git a/includes/Request/PathRouter.php b/includes/Request/PathRouter.php index 1082fa9a701..444aea7be0f 100644 --- a/includes/Request/PathRouter.php +++ b/includes/Request/PathRouter.php @@ -101,7 +101,7 @@ class PathRouter { if ( !isset( $options['strict'] ) || !$options['strict'] ) { // Unless this is a strict path make sure that the path has a $1 if ( strpos( $path, '$1' ) === false ) { - if ( substr( $path, -1 ) !== '/' ) { + if ( $path[-1] !== '/' ) { $path .= '/'; } $path .= '$1'; diff --git a/includes/ResourceLoader/OOUIIconPackModule.php b/includes/ResourceLoader/OOUIIconPackModule.php index ac72ca88947..e59e99c5cfb 100644 --- a/includes/ResourceLoader/OOUIIconPackModule.php +++ b/includes/ResourceLoader/OOUIIconPackModule.php @@ -54,7 +54,7 @@ class OOUIIconPackModule extends OOUIImageModule { $data[$theme] = []; // Load and merge the JSON data for all "icons-foo" modules foreach ( self::$knownImagesModules as $module ) { - if ( substr( $module, 0, 5 ) === 'icons' ) { + if ( str_starts_with( $module, 'icons' ) ) { $moreData = $this->readJSONFile( $this->getThemeImagesPath( $theme, $module ) ); if ( $moreData ) { $data[$theme] = array_replace_recursive( $data[$theme], $moreData ); diff --git a/includes/ResourceLoader/OOUIImageModule.php b/includes/ResourceLoader/OOUIImageModule.php index b794d7c6fcb..c3de3f28c4d 100644 --- a/includes/ResourceLoader/OOUIImageModule.php +++ b/includes/ResourceLoader/OOUIImageModule.php @@ -77,7 +77,7 @@ class OOUIImageModule extends ImageModule { } // Extra selectors to allow using the same icons for old-style MediaWiki UI code - if ( substr( $module, 0, 5 ) === 'icons' ) { + if ( str_starts_with( $module, 'icons' ) ) { $definition['selectorWithoutVariant'] = '.oo-ui-icon-{name}, .mw-ui-icon-{name}:before'; $definition['selectorWithVariant'] = '.oo-ui-image-{variant}.oo-ui-icon-{name}, ' . '.mw-ui-icon-{name}-{variant}:before'; diff --git a/includes/WebRequest.php b/includes/WebRequest.php index b3ce0abcda8..ed6613ceef3 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -405,9 +405,8 @@ class WebRequest { foreach ( (array)$bases as $keyValue => $base ) { // Find the part after $wgArticlePath $base = str_replace( '$1', '', $base ); - $baseLen = strlen( $base ); - if ( substr( $path, 0, $baseLen ) == $base ) { - $raw = substr( $path, $baseLen ); + if ( str_starts_with( $path, $base ) ) { + $raw = substr( $path, strlen( $base ) ); if ( $raw !== '' ) { $matches = [ 'title' => rawurldecode( $raw ) ]; if ( $key ) { diff --git a/includes/libs/ParamValidator/TypeDef/UploadDef.php b/includes/libs/ParamValidator/TypeDef/UploadDef.php index 4449ded09a9..b651c9671e1 100644 --- a/includes/libs/ParamValidator/TypeDef/UploadDef.php +++ b/includes/libs/ParamValidator/TypeDef/UploadDef.php @@ -115,7 +115,7 @@ class UploadDef extends TypeDef { $constant = ''; foreach ( get_defined_constants() as $c => $v ) { // @phan-suppress-next-line PhanTypeComparisonFromArray - if ( $v === $err && substr( $c, 0, 11 ) === 'UPLOAD_ERR_' ) { + if ( $v === $err && str_starts_with( $c, 'UPLOAD_ERR_' ) ) { $constant = " ($c?)"; } } diff --git a/includes/libs/filebackend/SwiftFileBackend.php b/includes/libs/filebackend/SwiftFileBackend.php index e2e0718a110..61a4b55f871 100644 --- a/includes/libs/filebackend/SwiftFileBackend.php +++ b/includes/libs/filebackend/SwiftFileBackend.php @@ -1792,7 +1792,7 @@ class SwiftFileBackend extends FileBackendStore { } } // Ceph RGW does not use in URLs (OpenStack Swift uses "/v1/") - if ( substr( $this->authCreds['storage_url'], -3 ) === '/v1' ) { + if ( str_ends_with( $this->authCreds['storage_url'], '/v1' ) ) { $this->isRGW = true; // take advantage of strong consistency in Ceph } } diff --git a/includes/libs/mime/MimeAnalyzer.php b/includes/libs/mime/MimeAnalyzer.php index 16d82a3d021..1ae00fa990e 100644 --- a/includes/libs/mime/MimeAnalyzer.php +++ b/includes/libs/mime/MimeAnalyzer.php @@ -708,13 +708,13 @@ class MimeAnalyzer implements LoggerAwareInterface { $script_type = null; # detect by shebang - if ( substr( $head, 0, 2 ) == "#!" ) { + if ( str_starts_with( $head, "#!" ) ) { $script_type = "ASCII"; - } elseif ( substr( $head, 0, 5 ) == "\xef\xbb\xbf#!" ) { + } elseif ( str_starts_with( $head, "\xef\xbb\xbf#!" ) ) { $script_type = "UTF-8"; - } elseif ( substr( $head, 0, 7 ) == "\xfe\xff\x00#\x00!" ) { + } elseif ( str_starts_with( $head, "\xfe\xff\x00#\x00!" ) ) { $script_type = "UTF-16BE"; - } elseif ( substr( $head, 0, 7 ) == "\xff\xfe#\x00!" ) { + } elseif ( str_starts_with( $head, "\xff\xfe#\x00!" ) ) { $script_type = "UTF-16LE"; } diff --git a/includes/libs/objectcache/MemcachedBagOStuff.php b/includes/libs/objectcache/MemcachedBagOStuff.php index 55c1442e4e6..65b35165dad 100644 --- a/includes/libs/objectcache/MemcachedBagOStuff.php +++ b/includes/libs/objectcache/MemcachedBagOStuff.php @@ -134,9 +134,8 @@ abstract class MemcachedBagOStuff extends MediumSpecificBagOStuff { return $key; } - $prefixLength = strlen( $this->routingPrefix ); - if ( substr( $key, 0, $prefixLength ) === $this->routingPrefix ) { - return substr( $key, $prefixLength ); + if ( str_starts_with( $key, $this->routingPrefix ) ) { + return substr( $key, strlen( $this->routingPrefix ) ); } return $key; diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php index 24d2aefbfd3..e99c827aae9 100644 --- a/includes/libs/rdbms/database/DatabasePostgres.php +++ b/includes/libs/rdbms/database/DatabasePostgres.php @@ -1062,7 +1062,7 @@ SQL; public function streamStatementEnd( &$sql, &$newLine ) { # Allow dollar quoting for function declarations - if ( substr( $newLine, 0, 4 ) == '$mw$' ) { + if ( str_starts_with( $newLine, '$mw$' ) ) { if ( $this->delimiter ) { $this->delimiter = false; } else { diff --git a/includes/registration/MissingExtensionException.php b/includes/registration/MissingExtensionException.php index 0b6ece93c1b..74f9703a59c 100644 --- a/includes/registration/MissingExtensionException.php +++ b/includes/registration/MissingExtensionException.php @@ -28,7 +28,7 @@ class MissingExtensionException extends Exception { * @param string $error Text of error mtime gave */ public function __construct( $path, $error ) { - $this->isSkin = substr( $path, -10 ) === "/skin.json"; + $this->isSkin = str_ends_with( $path, "/skin.json" ); $m = []; preg_match( "!/([^/]*)/[^/]*.json$!", $path, $m ); if ( $m ) { diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 73289338f2c..8ac08772488 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1710,7 +1710,7 @@ abstract class UploadBase { # use set/animate to add event-handler attribute to parent if ( ( $strippedElement === 'set' || $strippedElement === 'animate' ) && $stripped === 'attributename' - && substr( $value, 0, 2 ) === 'on' + && str_starts_with( $value, 'on' ) ) { wfDebug( __METHOD__ . ": Found svg setting event-handler attribute with " . "\"<$strippedElement $stripped='$value'...\" in uploaded file." ); diff --git a/maintenance/convertExtensionToRegistration.php b/maintenance/convertExtensionToRegistration.php index ac18ef115a6..4e948f7bc77 100644 --- a/maintenance/convertExtensionToRegistration.php +++ b/maintenance/convertExtensionToRegistration.php @@ -247,7 +247,7 @@ class ConvertExtensionToRegistration extends Maintenance { $path = $this->stripPath( $val, $this->dir ); // When path starts with tests/parser/ the file would be autodiscovered with // extension registry, so no need to add it to extension.json - if ( substr( $path, 0, 13 ) !== 'tests/parser/' || substr( $path, -4 ) !== '.txt' ) { + if ( !str_starts_with( $path, 'tests/parser/' ) || !str_ends_with( $path, '.txt' ) ) { $out[$key] = $path; } } diff --git a/maintenance/dumpCategoriesAsRdf.php b/maintenance/dumpCategoriesAsRdf.php index 9d34e70bd26..b45b4167e7c 100644 --- a/maintenance/dumpCategoriesAsRdf.php +++ b/maintenance/dumpCategoriesAsRdf.php @@ -120,7 +120,7 @@ class DumpCategoriesAsRdf extends Maintenance { */ public function addDumpHeader( $timestamp ) { $licenseUrl = $this->getConfig()->get( MainConfigNames::RightsUrl ); - if ( substr( $licenseUrl, 0, 2 ) == '//' ) { + if ( str_starts_with( $licenseUrl, '//' ) ) { $licenseUrl = 'https:' . $licenseUrl; } $this->rdfWriter->about( $this->categoriesRdf->getDumpURI() ) diff --git a/maintenance/generateSitemap.php b/maintenance/generateSitemap.php index a805c1796de..69874ee9cdb 100644 --- a/maintenance/generateSitemap.php +++ b/maintenance/generateSitemap.php @@ -195,7 +195,7 @@ class GenerateSitemap extends Maintenance { $dbDomain = WikiMap::getCurrentWikiDbDomain()->getId(); $this->fspath = realpath( $fspath ) . DIRECTORY_SEPARATOR; $this->urlpath = $this->getOption( 'urlpath', "" ); - if ( $this->urlpath !== "" && substr( $this->urlpath, -1 ) !== '/' ) { + if ( $this->urlpath !== "" && $this->urlpath[-1] !== '/' ) { $this->urlpath .= '/'; } $this->identifier = $this->getOption( 'identifier', $dbDomain ); @@ -517,7 +517,7 @@ class GenerateSitemap extends Maintenance { private function indexEntry( $filename ) { return "\t\n" . "\t\t" . wfGetServerUrl( PROTO_CANONICAL ) . - ( substr( $this->urlpath, 0, 1 ) === "/" ? "" : "/" ) . + ( $this->urlpath[0] === "/" ? "" : "/" ) . "{$this->urlpath}$filename\n" . "\t\t{$this->timestamp}\n" . "\t\n"; diff --git a/maintenance/importSiteScripts.php b/maintenance/importSiteScripts.php index 38f5c072a45..46404cd4719 100644 --- a/maintenance/importSiteScripts.php +++ b/maintenance/importSiteScripts.php @@ -98,7 +98,7 @@ class ImportSiteScripts extends Maintenance { $page = null; foreach ( $result['query']['allpages'] as $page ) { - if ( substr( $page['title'], -3 ) === '.js' ) { + if ( str_ends_with( $page['title'], '.js' ) ) { strtok( $page['title'], ':' ); $pages[] = strtok( '' ); } diff --git a/maintenance/includes/Benchmarker.php b/maintenance/includes/Benchmarker.php index 9ff58f408b8..6590e8f6dd4 100644 --- a/maintenance/includes/Benchmarker.php +++ b/maintenance/includes/Benchmarker.php @@ -225,7 +225,7 @@ abstract class Benchmarker extends Maintenance { protected function loadFile( $file ) { $content = file_get_contents( $file ); // Detect GZIP compression header - if ( substr( $content, 0, 2 ) === "\037\213" ) { + if ( str_starts_with( $content, "\037\213" ) ) { $content = gzdecode( $content ); } return $content; diff --git a/maintenance/update.php b/maintenance/update.php index f1f10af6560..dfca637942f 100755 --- a/maintenance/update.php +++ b/maintenance/update.php @@ -99,7 +99,7 @@ class UpdateMediaWiki extends Maintenance { } $this->fileHandle = null; - if ( substr( $this->getOption( 'schema', '' ), 0, 2 ) === "--" ) { + if ( str_starts_with( $this->getOption( 'schema', '' ), '--' ) ) { $this->fatalError( "The --schema option requires a file as an argument.\n" ); } elseif ( $this->hasOption( 'schema' ) ) { $file = $this->getOption( 'schema' ); diff --git a/maintenance/updateCredits.php b/maintenance/updateCredits.php index 030e56bc1b5..78dab8e521b 100644 --- a/maintenance/updateCredits.php +++ b/maintenance/updateCredits.php @@ -64,7 +64,7 @@ foreach ( $lines as $line ) { if ( empty( $line ) ) { continue; } - if ( substr( $line, 0, 5 ) === '[BOT]' ) { + if ( str_starts_with( $line, '[BOT]' ) ) { continue; } $contributors[$line] = true; diff --git a/tests/phpunit/includes/api/ApiOptionsTest.php b/tests/phpunit/includes/api/ApiOptionsTest.php index 2929de602e8..47b3bb2d2f8 100644 --- a/tests/phpunit/includes/api/ApiOptionsTest.php +++ b/tests/phpunit/includes/api/ApiOptionsTest.php @@ -153,7 +153,7 @@ class ApiOptionsTest extends ApiTestCase { foreach ( $options as $key => $value ) { if ( isset( $kinds[$key] ) ) { $mapping[$key] = $kinds[$key]; - } elseif ( substr( $key, 0, 7 ) === 'userjs-' ) { + } elseif ( str_starts_with( $key, 'userjs-' ) ) { $mapping[$key] = 'userjs'; } else { $mapping[$key] = 'unused'; diff --git a/tests/phpunit/includes/db/DatabaseTestHelper.php b/tests/phpunit/includes/db/DatabaseTestHelper.php index 4261a9851b2..15cb1f398c4 100644 --- a/tests/phpunit/includes/db/DatabaseTestHelper.php +++ b/tests/phpunit/includes/db/DatabaseTestHelper.php @@ -141,7 +141,7 @@ class DatabaseTestHelper extends Database { $check = $m[1]; } - if ( substr( $check, 0, strlen( $this->testName ) ) !== $this->testName ) { + if ( !str_starts_with( $check, $this->testName ) ) { throw new MWException( 'function name does not start with test class. ' . $fname . ' vs. ' . $this->testName . '. ' . 'Please provide __METHOD__ to database methods.' ); diff --git a/tests/phpunit/structure/AutoLoaderStructureTest.php b/tests/phpunit/structure/AutoLoaderStructureTest.php index 73bec120105..e45229768e8 100644 --- a/tests/phpunit/structure/AutoLoaderStructureTest.php +++ b/tests/phpunit/structure/AutoLoaderStructureTest.php @@ -88,7 +88,7 @@ class AutoLoaderStructureTest extends MediaWikiIntegrationTestCase { foreach ( $expected as $class => $file ) { // Only prefix $IP if it doesn't have it already. // Generally local classes don't have it, and those from extensions and test suites do. - if ( substr( $file, 0, 1 ) != '/' && substr( $file, 1, 1 ) != ':' ) { + if ( $file[0] !== '/' && $file[1] !== ':' ) { $filePath = self::fixSlashes( "$IP/$file" ); } else { $filePath = self::fixSlashes( $file ); diff --git a/tests/phpunit/unit/includes/registration/ExtensionRegistryTest.php b/tests/phpunit/unit/includes/registration/ExtensionRegistryTest.php index 337c83044f1..12ace7d8a35 100644 --- a/tests/phpunit/unit/includes/registration/ExtensionRegistryTest.php +++ b/tests/phpunit/unit/includes/registration/ExtensionRegistryTest.php @@ -135,7 +135,7 @@ class ExtensionRegistryTest extends MediaWikiUnitTestCase { if ( $before ) { foreach ( $before as $key => $value ) { // mw prefixed globals does not exist normally - if ( substr( $key, 0, 2 ) == 'mw' ) { + if ( str_starts_with( $key, 'mw' ) ) { $GLOBALS[$key] = $value; } else { $this->setGlobal( $key, $value ); @@ -161,7 +161,7 @@ class ExtensionRegistryTest extends MediaWikiUnitTestCase { // Remove mw prefixed globals if ( $before ) { foreach ( $before as $key => $value ) { - if ( substr( $key, 0, 2 ) == 'mw' ) { + if ( str_starts_with( $key, 'mw' ) ) { unset( $GLOBALS[$key] ); } }