1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 08:12:40 +01:00

Fix an issue where Herald may fail to extract content from an empty commit

Summary:
Ref T13667. The Herald "content added" rule (and other similar rules) do not correctly extract content from empty commits.

When we load an empty raw diff, return an empty changed content map.

Ref T13588. Also fix some PHP8.1 null/string stuff

Test Plan:
  - Ran "bin/repository reparse --publish <commit>", with an empty commit hash and a nonempty commit hash.
  - Reviewed Herald transcripts for general sanity.

Maniphest Tasks: T13667, T13588

Differential Revision: https://secure.phabricator.com/D21761
This commit is contained in:
epriestley 2022-04-20 09:43:59 -07:00
parent 2188473fa7
commit de980cc54e
9 changed files with 52 additions and 22 deletions

View file

@ -267,6 +267,11 @@ final class HeraldCommitAdapter
$raw = $diff_file->loadFileData(); $raw = $diff_file->loadFileData();
// See T13667. This happens when a commit is empty and affects no files.
if (!strlen($raw)) {
return false;
}
$parser = new ArcanistDiffParser(); $parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($raw); $changes = $parser->parseDiff($raw);
@ -290,6 +295,10 @@ final class HeraldCommitAdapter
} }
} }
if ($this->commitDiff === false) {
return array();
}
if ($this->commitDiff instanceof Exception) { if ($this->commitDiff instanceof Exception) {
$ex = $this->commitDiff; $ex = $this->commitDiff;
$ex_class = get_class($ex); $ex_class = get_class($ex);

View file

@ -88,7 +88,12 @@ final class DiffusionPathChangeQuery extends Phobject {
$change->setFileType($raw_change['fileType']); $change->setFileType($raw_change['fileType']);
$change->setCommitIdentifier($commit->getCommitIdentifier()); $change->setCommitIdentifier($commit->getCommitIdentifier());
$change->setTargetPath(ltrim($raw_change['targetPathName'], '/')); $target_path = $raw_change['targetPathName'];
if ($target_path !== null) {
$target_path = ltrim($target_path, '/');
}
$change->setTargetPath($target_path);
$change->setTargetCommitIdentifier($raw_change['targetCommitIdentifier']); $change->setTargetCommitIdentifier($raw_change['targetCommitIdentifier']);
$id = $raw_change['pathID']; $id = $raw_change['pathID'];

View file

@ -207,7 +207,7 @@ abstract class DiffusionRequest extends Phobject {
*/ */
private function initializeFromDictionary(array $data) { private function initializeFromDictionary(array $data) {
$blob = idx($data, 'blob'); $blob = idx($data, 'blob');
if (strlen($blob)) { if (phutil_nonempty_string($blob)) {
$blob = self::parseRequestBlob($blob, $this->supportsBranches()); $blob = self::parseRequestBlob($blob, $this->supportsBranches());
$data = $blob + $data; $data = $blob + $data;
} }
@ -518,6 +518,7 @@ abstract class DiffusionRequest extends Phobject {
$result['path'] = $blob; $result['path'] = $blob;
} }
if ($result['path'] !== null) {
$parts = explode('/', $result['path']); $parts = explode('/', $result['path']);
foreach ($parts as $part) { foreach ($parts as $part) {
// Prevent any hyjinx since we're ultimately shipping this to the // Prevent any hyjinx since we're ultimately shipping this to the
@ -526,6 +527,7 @@ abstract class DiffusionRequest extends Phobject {
throw new Exception(pht('Invalid path URI.')); throw new Exception(pht('Invalid path URI.'));
} }
} }
}
return $result; return $result;
} }

View file

@ -128,7 +128,7 @@ final class PhabricatorMacroQuery
$this->authorPHIDs); $this->authorPHIDs);
} }
if (strlen($this->nameLike)) { if (($this->nameLike !== null) && strlen($this->nameLike)) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'm.name LIKE %~', 'm.name LIKE %~',
@ -142,7 +142,7 @@ final class PhabricatorMacroQuery
$this->names); $this->names);
} }
if (strlen($this->namePrefix)) { if (($this->namePrefix !== null) && strlen($this->namePrefix)) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'm.name LIKE %>', 'm.name LIKE %>',

View file

@ -148,7 +148,7 @@ final class PassphraseCredentialQuery
(int)$this->allowConduit); (int)$this->allowConduit);
} }
if (strlen($this->nameContains)) { if (phutil_nonempty_string($this->nameContains)) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'LOWER(c.name) LIKE %~', 'LOWER(c.name) LIKE %~',

View file

@ -633,7 +633,7 @@ final class PhabricatorRepositoryQuery
$this->uuids); $this->uuids);
} }
if (strlen($this->datasourceQuery)) { if (phutil_nonempty_string($this->datasourceQuery)) {
// This handles having "rP" match callsigns starting with "P...". // This handles having "rP" match callsigns starting with "P...".
$query = trim($this->datasourceQuery); $query = trim($this->datasourceQuery);
if (preg_match('/^r/', $query)) { if (preg_match('/^r/', $query)) {

View file

@ -281,9 +281,11 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
public function getSubversionBaseURI($commit = null) { public function getSubversionBaseURI($commit = null) {
$subpath = $this->getDetail('svn-subpath'); $subpath = $this->getDetail('svn-subpath');
if (!strlen($subpath)) {
if (!phutil_nonempty_string($subpath)) {
$subpath = null; $subpath = null;
} }
return $this->getSubversionPathURI($subpath, $commit); return $this->getSubversionPathURI($subpath, $commit);
} }
@ -301,7 +303,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
$uri = rtrim($uri, '/'); $uri = rtrim($uri, '/');
if (strlen($path)) { if (phutil_nonempty_string($path)) {
$path = rawurlencode($path); $path = rawurlencode($path);
$path = str_replace('%2F', '/', $path); $path = str_replace('%2F', '/', $path);
$uri = $uri.'/'.ltrim($path, '/'); $uri = $uri.'/'.ltrim($path, '/');
@ -574,12 +576,12 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
public function getURI() { public function getURI() {
$short_name = $this->getRepositorySlug(); $short_name = $this->getRepositorySlug();
if (strlen($short_name)) { if (phutil_nonempty_string($short_name)) {
return "/source/{$short_name}/"; return "/source/{$short_name}/";
} }
$callsign = $this->getCallsign(); $callsign = $this->getCallsign();
if (strlen($callsign)) { if (phutil_nonempty_string($callsign)) {
return "/diffusion/{$callsign}/"; return "/diffusion/{$callsign}/";
} }
@ -593,7 +595,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
public function getCommitURI($identifier) { public function getCommitURI($identifier) {
$callsign = $this->getCallsign(); $callsign = $this->getCallsign();
if (strlen($callsign)) { if (phutil_nonempty_string($callsign)) {
return "/r{$callsign}{$identifier}"; return "/r{$callsign}{$identifier}";
} }
@ -736,25 +738,25 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
return $this->getCommitURI($commit); return $this->getCommitURI($commit);
} }
if (strlen($path)) { if (phutil_nonempty_string($path)) {
$path = ltrim($path, '/'); $path = ltrim($path, '/');
$path = str_replace(array(';', '$'), array(';;', '$$'), $path); $path = str_replace(array(';', '$'), array(';;', '$$'), $path);
$path = phutil_escape_uri($path); $path = phutil_escape_uri($path);
} }
$raw_branch = $branch; $raw_branch = $branch;
if (strlen($branch)) { if (phutil_nonempty_string($branch)) {
$branch = phutil_escape_uri_path_component($branch); $branch = phutil_escape_uri_path_component($branch);
$path = "{$branch}/{$path}"; $path = "{$branch}/{$path}";
} }
$raw_commit = $commit; $raw_commit = $commit;
if (strlen($commit)) { if (phutil_nonempty_string($commit)) {
$commit = str_replace('$', '$$', $commit); $commit = str_replace('$', '$$', $commit);
$commit = ';'.phutil_escape_uri($commit); $commit = ';'.phutil_escape_uri($commit);
} }
if (strlen($line)) { if (phutil_nonempty_string($line)) {
$line = '$'.phutil_escape_uri($line); $line = '$'.phutil_escape_uri($line);
} }
@ -862,7 +864,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
public function getDefaultBranch() { public function getDefaultBranch() {
$default = $this->getDetail('default-branch'); $default = $this->getDetail('default-branch');
if (strlen($default)) { if (phutil_nonempty_string($default)) {
return $default; return $default;
} }

View file

@ -583,6 +583,14 @@ final class PhabricatorMarkupEngine extends Phobject {
$engine->setConfig('viewer', $viewer); $engine->setConfig('viewer', $viewer);
foreach ($content_blocks as $content_block) { foreach ($content_blocks as $content_block) {
if ($content_block === null) {
continue;
}
if (!strlen($content_block)) {
continue;
}
$engine->markupText($content_block); $engine->markupText($content_block);
$phids = $engine->getTextMetadata( $phids = $engine->getTextMetadata(
PhabricatorMentionRemarkupRule::KEY_MENTIONED, PhabricatorMentionRemarkupRule::KEY_MENTIONED,

View file

@ -105,6 +105,10 @@ function phutil_tag_div($class, $content = null) {
} }
function phutil_escape_html($string) { function phutil_escape_html($string) {
if ($string === null) {
return '';
}
if ($string instanceof PhutilSafeHTML) { if ($string instanceof PhutilSafeHTML) {
return $string; return $string;
} else if ($string instanceof PhutilSafeHTMLProducerInterface) { } else if ($string instanceof PhutilSafeHTMLProducerInterface) {