From 6142fcd5264ff605321657e75aae2701e3dd2108 Mon Sep 17 00:00:00 2001 From: Valerio Bozzolan Date: Mon, 18 Dec 2023 11:37:23 +0100 Subject: [PATCH 1/9] Fix Subversion "commit" support in PHP 8.1 Summary: Premising that "arc commit" is a beautiful Workflow dedicated to svn repositories, I tried it at work, causing the usual PHP 8.1 deprecation warning: $ arc diff $ arc commit ERROR 8192: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated at [arcanist/src/workflow/ArcanistWorkflow.php:1520] arcanist(head=master, ref.master=e46025f7a914) #0 preg_replace(string, string, NULL) called at [/src/workflow/ArcanistWorkflow.php:1520] #1 ArcanistWorkflow::normalizeRevisionID(NULL) called at [/src/workflow/ArcanistCommitWorkflow.php:68] #2 ArcanistCommitWorkflow::run() called at [/scripts/arcanist.php:427] Usage Exception: Unable to identify the revision in the working copy. Use '--revision ' to select a revision. This bug happens at least when Arcanist does not find any related Revision ID. It seems there is a method that always normalizes the Revision ID, but sometime that is unknown (null). And so, NULL ends inside a preg_replace(). It's probably OK to have a normalize method that accept wild things, including NULL. So, fixed that specific method. Closes T15693 Test Plan: This revision was tested in production in my company. Take a random Subversion repository. Edit a line. Run "arc diff". Then run "arc commit". No warnings. Reviewers: O1 Blessed Committers, aklapper Reviewed By: O1 Blessed Committers, aklapper Subscribers: tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15693 Differential Revision: https://we.phorge.it/D25498 --- src/workflow/ArcanistWorkflow.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index d0863772..3b8242cb 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -1516,7 +1516,14 @@ abstract class ArcanistWorkflow extends Phobject { } } + /** + * @param string|null $revision_id + * @return string + */ final protected function normalizeRevisionID($revision_id) { + if ($revision_id === null) { + return ''; + } return preg_replace('/^D/i', '', $revision_id); } From 6c7caf3572f487b74ab0e28d860a449cc9495e6a Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Sat, 13 Jan 2024 11:20:58 +0100 Subject: [PATCH 2/9] Correct manual upload of Differential patch with a leading BOM Summary: Strip a leading UTF-8 Byte Order Mark to avoid silently dropping the first file in a manually uploaded patch. This change only strips the UTF-8 BOM as UTF-8 is acceptable input. (Probably non-existing) handling of any other BOMs as first bytes in a diff remains unchanged. Closes T15452 Test Plan: Go to `/differential/diff/create/` and upload the test case in T15452 via `Raw Diff From File`. See two files listed on resulting page `/differential/diff/1/` instead of previously only one file. Optionally, confirm that byte length of `$diff` is three bytes less now (via `strlen($diff)`). Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15452 Differential Revision: https://we.phorge.it/D25514 --- src/parser/ArcanistDiffParser.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php index 53a46a7a..f80917ed 100644 --- a/src/parser/ArcanistDiffParser.php +++ b/src/parser/ArcanistDiffParser.php @@ -187,6 +187,11 @@ final class ArcanistDiffParser extends Phobject { } public function parseDiff($diff) { + // Remove leading UTF-8 Byte Order Mark (BOM) + if (substr($diff, 0, 3) == pack('CCC', 0xEF, 0xBB, 0xBF)) { + $diff = substr($diff, 3); + } + if (!strlen(trim($diff))) { throw new Exception(pht("Can't parse an empty diff!")); } From 8ef1ead6aca014e7f23c79c1b11441a0f8a436b7 Mon Sep 17 00:00:00 2001 From: Christopher Speck Date: Mon, 5 Feb 2024 19:26:48 -0500 Subject: [PATCH 3/9] T15064: Several arcanist PHP 8.1 compat issues on Windows Summary: Ran into `strlen`/`strpos` issues while using Arcanist on Windows: - Running `arc version` when not in a project/repository directory blew up (may not be specific to Windows). - Determining mime type of binary file in repository fails. Refs T15064 Test Plan: Ran diff including a binary file. Ran `arc version` when not in a project/repository directory. Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15064 Differential Revision: https://we.phorge.it/D25534 --- src/filesystem/Filesystem.php | 6 +++--- src/workingcopyidentity/ArcanistWorkingCopyIdentity.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/filesystem/Filesystem.php b/src/filesystem/Filesystem.php index 981d4feb..dd6e984a 100644 --- a/src/filesystem/Filesystem.php +++ b/src/filesystem/Filesystem.php @@ -275,7 +275,7 @@ final class Filesystem extends Phobject { $trap->destroy(); if (!$ok) { - if (strlen($err)) { + if ($err !== null && strlen($err)) { throw new FilesystemException( $to, pht( @@ -307,7 +307,7 @@ final class Filesystem extends Phobject { * @task file */ public static function remove($path) { - if (!strlen($path)) { + if ($path == null || !strlen($path)) { // Avoid removing PWD. throw new Exception( pht( @@ -673,7 +673,7 @@ final class Filesystem extends Phobject { } // If we come back with an encoding, strip it off. - if (strpos($mime_type, ';') !== false) { + if ($mime_type !== null && strpos($mime_type, ';') !== false) { list($type, $encoding) = explode(';', $mime_type, 2); $mime_type = $type; } diff --git a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php index d2c86fd0..82a98cbb 100644 --- a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php +++ b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php @@ -289,7 +289,7 @@ final class ArcanistWorkingCopyIdentity extends Phobject { } public function readLocalArcConfig() { - if (strlen($this->localMetaDir)) { + if ($this->localMetaDir !== null && strlen($this->localMetaDir)) { $local_path = Filesystem::resolvePath('arc/config', $this->localMetaDir); $console = PhutilConsole::getConsole(); From 174bf094ef9fe26770903c912cf29b6c78760b26 Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Wed, 28 Feb 2024 12:29:14 +0100 Subject: [PATCH 4/9] Replace all phurl.io short URIs with target URIs Summary: As of February 2024, phurl.io (which is not run by Phorge.it) shows HTTP 503 errors instead of redirecting to the target URIs. Thus replace any URIs pointing to phurl.io with the corresponding target URIs, based on the mapping listed on https://secure.phabricator.com/phurl/. Closes T15746 Test Plan: Carefully read the diff. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15746 Differential Revision: https://we.phorge.it/D25542 --- src/init/lib/PhutilMissingSymbolException.php | 3 ++- src/runtime/ArcanistRuntime.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/init/lib/PhutilMissingSymbolException.php b/src/init/lib/PhutilMissingSymbolException.php index 68926550..43434728 100644 --- a/src/init/lib/PhutilMissingSymbolException.php +++ b/src/init/lib/PhutilMissingSymbolException.php @@ -18,7 +18,8 @@ final class PhutilMissingSymbolException extends Exception { 'moved, your library map may need to be rebuilt. You can rebuild '. 'the map by running "arc liberate".'. "\n\n". - 'For more information, see: https://phurl.io/u/newclasses', + 'For more information, see: '. + 'https://we.phorge.it/book/contrib/article/adding_new_classes/', $symbol, $type, $reason); diff --git a/src/runtime/ArcanistRuntime.php b/src/runtime/ArcanistRuntime.php index 8e3d220f..93f205cc 100644 --- a/src/runtime/ArcanistRuntime.php +++ b/src/runtime/ArcanistRuntime.php @@ -188,7 +188,7 @@ final class ArcanistRuntime { tsprintf( '%s <__%s__>', pht('Learn More:'), - 'https://phurl.io/u/noninteractive')); + 'https://secure.phabricator.com/T13491')); throw new PhutilArgumentUsageException( pht('Missing required "--" in argument list.')); From 7c5e607e9752a5e5f1e3ddd9bd1907077acb9253 Mon Sep 17 00:00:00 2001 From: sten Date: Tue, 14 Nov 2023 09:43:23 +0000 Subject: [PATCH 5/9] Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json. Summary: This change updates PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use phpunit's junit output rather than json. --log-json was deprecated in PHPUnit 5.7, and removed in PHPUnit 6.0.0 (2017-02-03). Fixes T15667 Test Plan: Download an example PHP repo, with arcconfig set to 'PhpunitTestEngine'. Example: ``` git clone https://github.com/campbsb/example-phorge-php-project.git ``` Install phpunit using composer, and test it ``` cd example-phorge-php-project php ~/composer.phar update phpunit ``` In a PHP phorge repo, set the following in .arcconfig: ``` "unit.engine": "PhpunitTestEngine", "phpunit_config": "phpunit.xml", ``` Make a non-breaking change to src/Service.php (eg add a space), and run 'arc unit' Reviewers: O1 Blessed Committers, avivey, speck Reviewed By: O1 Blessed Committers, avivey, speck Subscribers: avivey, Ekubischta, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15667 Differential Revision: https://we.phorge.it/D25472 --- src/unit/engine/PhpunitTestEngine.php | 14 +-- .../ArcanistPhpunitTestResultParser.php | 91 +------------------ 2 files changed, 10 insertions(+), 95 deletions(-) diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php index 8206b787..4cb89006 100644 --- a/src/unit/engine/PhpunitTestEngine.php +++ b/src/unit/engine/PhpunitTestEngine.php @@ -52,7 +52,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine { if (!Filesystem::pathExists($test_path)) { continue; } - $json_tmp = new TempFile(); + $xml_tmp = new TempFile(); $clover_tmp = null; $clover = null; if ($this->getEnableCoverage() !== false) { @@ -64,10 +64,10 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine { $stderr = '-d display_errors=stderr'; - $futures[$test_path] = new ExecFuture('%C %C %C --log-json %s %C %s', - $this->phpunitBinary, $config, $stderr, $json_tmp, $clover, $test_path); + $futures[$test_path] = new ExecFuture('%C %C %C --log-junit %s %C %s', + $this->phpunitBinary, $config, $stderr, $xml_tmp, $clover, $test_path); $tmpfiles[$test_path] = array( - 'json' => $json_tmp, + 'xml' => $xml_tmp, 'clover' => $clover_tmp, ); } @@ -81,7 +81,7 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine { $results[] = $this->parseTestResults( $test, - $tmpfiles[$test]['json'], + $tmpfiles[$test]['xml'], $tmpfiles[$test]['clover'], $stderr); } @@ -99,8 +99,8 @@ final class PhpunitTestEngine extends ArcanistUnitTestEngine { * * @return array */ - private function parseTestResults($path, $json_tmp, $clover_tmp, $stderr) { - $test_results = Filesystem::readFile($json_tmp); + private function parseTestResults($path, $xml_tmp, $clover_tmp, $stderr) { + $test_results = Filesystem::readFile($xml_tmp); return id(new ArcanistPhpunitTestResultParser()) ->setEnableCoverage($this->getEnableCoverage()) ->setProjectRoot($this->projectRoot) diff --git a/src/unit/parser/ArcanistPhpunitTestResultParser.php b/src/unit/parser/ArcanistPhpunitTestResultParser.php index 5ccff970..0a33fa55 100644 --- a/src/unit/parser/ArcanistPhpunitTestResultParser.php +++ b/src/unit/parser/ArcanistPhpunitTestResultParser.php @@ -25,80 +25,19 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser { return array($result); } - $report = $this->getJsonReport($test_results); - // coverage is for all testcases in the executed $path $coverage = array(); if ($this->enableCoverage !== false) { $coverage = $this->readCoverage(); } - $last_test_finished = true; + $xunit_result_parser = new ArcanistXUnitTestResultParser(); + $results = $xunit_result_parser->parseTestResults($test_results); - $results = array(); - foreach ($report as $event) { - switch (idx($event, 'event')) { - case 'test': - break; - case 'testStart': - $last_test_finished = false; - // fall through - default: - continue 2; // switch + loop - } - - $status = ArcanistUnitTestResult::RESULT_PASS; - $user_data = ''; - - if ('fail' == idx($event, 'status')) { - $status = ArcanistUnitTestResult::RESULT_FAIL; - $user_data .= idx($event, 'message')."\n"; - foreach (idx($event, 'trace') as $trace) { - $user_data .= sprintf( - "\n%s:%s", - idx($trace, 'file'), - idx($trace, 'line')); - } - } else if ('error' == idx($event, 'status')) { - if (strpos(idx($event, 'message'), 'Skipped Test') !== false) { - $status = ArcanistUnitTestResult::RESULT_SKIP; - $user_data .= idx($event, 'message'); - } else if (strpos( - idx($event, 'message'), - 'Incomplete Test') !== false) { - $status = ArcanistUnitTestResult::RESULT_SKIP; - $user_data .= idx($event, 'message'); - } else { - $status = ArcanistUnitTestResult::RESULT_BROKEN; - $user_data .= idx($event, 'message'); - foreach (idx($event, 'trace') as $trace) { - $user_data .= sprintf( - "\n%s:%s", - idx($trace, 'file'), - idx($trace, 'line')); - } - } - } - - $name = preg_replace('/ \(.*\)/s', '', idx($event, 'test')); - - $result = new ArcanistUnitTestResult(); - $result->setName($name); - $result->setResult($status); - $result->setDuration(idx($event, 'time')); + foreach ($results as $result) { $result->setCoverage($coverage); - $result->setUserData($user_data); - - $results[] = $result; - $last_test_finished = true; } - if (!$last_test_finished) { - $results[] = id(new ArcanistUnitTestResult()) - ->setName(idx($event, 'test')) // use last event - ->setUserData($this->stderr) - ->setResult(ArcanistUnitTestResult::RESULT_BROKEN); - } return $results; } @@ -161,28 +100,4 @@ final class ArcanistPhpunitTestResultParser extends ArcanistTestResultParser { return $reports; } - /** - * We need this non-sense to make json generated by phpunit - * valid. - * - * @param string $json String containing JSON report - * @return array JSON decoded array - */ - private function getJsonReport($json) { - - if (empty($json)) { - throw new Exception( - pht( - 'JSON report file is empty, it probably means that phpunit '. - 'failed to run tests. Try running %s with %s option and then run '. - 'generated phpunit command yourself, you might get the answer.', - 'arc unit', - '--trace')); - } - - $json = preg_replace('/}{\s*"/', '},{"', $json); - $json = '['.$json.']'; - return phutil_json_decode($json); - } - } From f6261dc614a78d243ecf61c93137c81b51d19ee6 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Fri, 15 Mar 2024 10:49:33 +0200 Subject: [PATCH 6/9] Arc liberate: support traits Summary: Looks like this is all that's needed? Ref T15751 Test Plan: R12 has some scenarios for testing this. Also ran `arc liberate --clean` on arc and phorge repos, and the generated map did not change. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15751 Differential Revision: https://we.phorge.it/D25551 --- src/moduleutils/PhutilLibraryMapBuilder.php | 7 +++- src/symbols/PhutilSymbolLoader.php | 10 ++++-- support/lib/extract-symbols.php | 36 ++++++++++----------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/moduleutils/PhutilLibraryMapBuilder.php b/src/moduleutils/PhutilLibraryMapBuilder.php index df05050a..5d359bbc 100644 --- a/src/moduleutils/PhutilLibraryMapBuilder.php +++ b/src/moduleutils/PhutilLibraryMapBuilder.php @@ -332,11 +332,16 @@ final class PhutilLibraryMapBuilder extends Phobject { 'xmap' => array(), ); + $type_translation = array( + 'interface' => 'class', + 'trait' => 'class', + ); + // Detect duplicate symbols within the library. foreach ($symbol_map as $file => $info) { foreach ($info['have'] as $type => $symbols) { foreach ($symbols as $symbol => $declaration) { - $lib_type = ($type == 'interface') ? 'class' : $type; + $lib_type = idx($type_translation, $type, $type); if (!empty($library_map[$lib_type][$symbol])) { $prior = $library_map[$lib_type][$symbol]; throw new Exception( diff --git a/src/symbols/PhutilSymbolLoader.php b/src/symbols/PhutilSymbolLoader.php index a5851a5e..7f733ee7 100644 --- a/src/symbols/PhutilSymbolLoader.php +++ b/src/symbols/PhutilSymbolLoader.php @@ -398,6 +398,12 @@ final class PhutilSymbolLoader { } + private static function classLikeExists($name) { + return class_exists($name, false) || + interface_exists($name, false) || + trait_exists($name, false); + } + /** * @task internal */ @@ -411,7 +417,7 @@ final class PhutilSymbolLoader { return; } } else { - if (class_exists($name, false) || interface_exists($name, false)) { + if (self::classLikeExists($name)) { return; } } @@ -431,7 +437,7 @@ final class PhutilSymbolLoader { $load_failed = pht('function'); } } else { - if (!class_exists($name, false) && !interface_exists($name, false)) { + if (!self::classLikeExists($name)) { $load_failed = pht('class or interface'); } } diff --git a/support/lib/extract-symbols.php b/support/lib/extract-symbols.php index a1cd28fa..6744cd90 100755 --- a/support/lib/extract-symbols.php +++ b/support/lib/extract-symbols.php @@ -112,18 +112,6 @@ foreach ($namespaces as $namespace) { $namespace, $path, pht('namespace `%s` statements', 'use')); } -$possible_traits = $root->selectDescendantsOfType('n_CLASS_DECLARATION'); -foreach ($possible_traits as $possible_trait) { - $attributes = $possible_trait->getChildByIndex(0); - // Can't use getChildByIndex here because not all classes have attributes - foreach ($attributes->getChildren() as $attribute) { - if (strtolower($attribute->getConcreteString()) === 'trait') { - phutil_fail_on_unsupported_feature($possible_trait, $path, pht('traits')); - } - } -} - - // -( Marked Externals )------------------------------------------------------ @@ -256,17 +244,29 @@ foreach ($calls as $call) { // Find classes declared by this file. - // This is "class X ... { ... }". -$classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION'); -foreach ($classes as $class) { - $class_name = $class->getChildByIndex(1); - $have[] = array( - 'type' => 'class', +function build_have_element_for_class_declaration(XHPASTNode $class_node) { + $class_name = $class_node->getChildByIndex(1); + + $type = 'class'; + $attributes = $class_node->getChildByIndex(0); + foreach ($attributes->getChildren() as $attribute) { + if (strtolower($attribute->getConcreteString()) === 'trait') { + $type = 'trait'; + } + } + + return array( + 'type' => $type, 'symbol' => $class_name, ); } +$classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION'); +foreach ($classes as $class) { + $have[] = build_have_element_for_class_declaration($class); +} + // Find classes used by this file. We identify these: // From ef73b12b580ee10bd901299419f4edac20f06c1a Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Fri, 22 Mar 2024 11:25:57 +0100 Subject: [PATCH 7/9] Fix "strpos(): Non-string needles will be interpreted as strings" in PhutilSortVector Summary: Code checking if the needle string `$value` is somewhere in the haystack `"\0"` makes no sense for a single byte (if it did, then `strcmp` instead of `strpos` should have been used) and the created exception output implies that it's supposed to check that a string does not contain NULL bytes. Thus switch the order of arguments passed to `strpos()` to be correct. ``` EXCEPTION: (RuntimeException) strpos(): Non-string needles will be interpreted as strings in the future. Use an explicit chr() call to preserve the current behavior at [/src/error/PhutilErrorHandler.php:261] #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [/src/error/PhutilErrorHandler.php:261] #1 <#2> strpos(string, integer) called at [/src/utils/PhutilSortVector.php:33] ``` Closes T15755 Test Plan: Read the surrounding code carefully. Reviewers: O1 Blessed Committers, valerio.bozzolan, speck Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15755 Differential Revision: https://we.phorge.it/D25557 --- src/utils/PhutilSortVector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/PhutilSortVector.php b/src/utils/PhutilSortVector.php index 00146070..1bddd22e 100644 --- a/src/utils/PhutilSortVector.php +++ b/src/utils/PhutilSortVector.php @@ -30,7 +30,7 @@ final class PhutilSortVector } public function addString($value) { - if (strlen($value) && (strpos("\0", $value) !== false)) { + if (strlen($value) && (strpos($value, "\0") !== false)) { throw new Exception( pht( 'String components of a sort vector must not contain NULL bytes.')); From 6718b32a64f2279a408edb96d7527016e5786e73 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Fri, 12 Apr 2024 21:26:29 +0200 Subject: [PATCH 8/9] Improve PHPDoc of id() Summary: Improve PHPDoc of id() Test Plan: Check the types that are returned by id Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25576 --- src/utils/utils.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/utils/utils.php b/src/utils/utils.php index 0b86cd59..7043d9b8 100644 --- a/src/utils/utils.php +++ b/src/utils/utils.php @@ -13,8 +13,9 @@ * * id(new Thing())->doStuff(); * - * @param wild Anything. - * @return wild Unmodified argument. + * @template T + * @param T $x Anything + * @return T Unmodified argument. */ function id($x) { return $x; From 7f28d7266f81985096219a11b949561d70f052e4 Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Fri, 10 May 2024 12:04:39 +0200 Subject: [PATCH 9/9] Fix PHP 8.1 "preg_match(null)" exception for missing Content-Type Summary: When the `Content-Type` HTTP header is empty or missing, `null` is passed to `preg_match()` which is deprecated behavior since PHP 8.1. Thus only call `preg_match()` when the value is set. ``` ERROR 8192: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [$HOME/arcanist/src/future/http/status/HTTPFutureHTTPResponseStatus.php:24] ``` Closes T15821 Test Plan: Visit something using `HTTPFutureHTTPResponseStatus`, like, a profile image. No crashes. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15821 Differential Revision: https://we.phorge.it/D25632 --- src/future/http/status/HTTPFutureHTTPResponseStatus.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/future/http/status/HTTPFutureHTTPResponseStatus.php b/src/future/http/status/HTTPFutureHTTPResponseStatus.php index 469f9a0c..44636dca 100644 --- a/src/future/http/status/HTTPFutureHTTPResponseStatus.php +++ b/src/future/http/status/HTTPFutureHTTPResponseStatus.php @@ -21,7 +21,8 @@ final class HTTPFutureHTTPResponseStatus extends HTTPFutureResponseStatus { $content_type = BaseHTTPFuture::getHeader($headers, 'Content-Type'); $match = null; - if (preg_match('/;\s*charset=([^;]+)/', $content_type, $match)) { + if (phutil_nonempty_string($content_type) && + preg_match('/;\s*charset=([^;]+)/', $content_type, $match)) { $encoding = trim($match[1], "\"'"); try { $excerpt = phutil_utf8_convert($excerpt, 'UTF-8', $encoding);