revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 14:00:56 +01:00
Code Issues Releases Wiki Activity

Fix regression in PHUIObjectItemView.php:662: allow PhutilURI and other stringlike

Browse source 
Summary:
This fixes this specific exception that can happen with whatever PHP version in some pages:

    Call to phutil_nonempty_string() expected null or a string, got: PhutilURI from PHUIObjectItemView.php:662

The regression was introduced since:

b56d86e48d

The problem is caused by the fact that we are trying to introduce very strict checks, and sometime we are too much strict.

In this specific case we use `phutil_nonempty_stringlike()` since it also allows objects with a `__toString()` method.

In order not to leave these cases to chance, we have added a log line, which can be removed in the future.

If you see this log line, report it in the respective Task. Thank you!

Closes T15306
Ref T15316

Test Plan:
1. {nav Dashboard > Add a Panel}: no crash
2. {nav Diffusion repo > README}: no crash
3. {nav Herald > Create}: no crash

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, aklapper, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15306, T15316

Differential Revision: https://we.phorge.it/D25157
This commit is contained in:
Valerio Bozzolan 2023-05-06 15:55:44 +02:00
parent 651e3f7263
commit e9e2c95413
1 changed files with 58 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

60
src/view/phui/PHUIObjectItemView.php
View file

@ -83,11 +83,29 @@ final class PHUIObjectItemView extends AphrontTagView {
return $this->object;
}
/**
* Set the href attribute
*
* @param string|PhutilURI|null $href
* @return self
*/
public function setHref($href) {
// We have not a very clear idea about what this method should receive
// So, let's log alien stuff for some time
// https://we.phorge.it/T15316
self::requireValidHref($href, 'href');
$this->href = $href;
return $this;
}
/**
* Get the href attribute
*
* @see PHUIObjectItemView::setHref()
* @return string|PhutilURI|null
*/
public function getHref() {
return $this->href;
}
@ -136,7 +154,19 @@ final class PHUIObjectItemView extends AphrontTagView {
return $this;
}
/**
* Set the image href attribute
*
* @param string|PhutilURI|null $image_href
* @return self
*/
public function setImageHref($image_href) {
// We have not a very clear idea about what this method should receive
// So, let's log alien stuff for some time
// https://we.phorge.it/T15316
self::requireValidHref($image_href, 'image_href');
$this->imageHref = $image_href;
return $this;
}
@ -659,8 +689,8 @@ final class PHUIObjectItemView extends AphrontTagView {
$this->getImageIcon());
}
if ($image && (phutil_nonempty_string($this->href) ||
phutil_nonempty_string($this->imageHref))) {
if ($image && (phutil_nonempty_stringlike($this->href) ||
phutil_nonempty_stringlike($this->imageHref))) {
$image_href = ($this->imageHref) ? $this->imageHref : $this->href;
$image = phutil_tag(
'a',
@ -899,4 +929,30 @@ final class PHUIObjectItemView extends AphrontTagView {
return javelin_tag('span', $options, '');
}
/**
* Receive a href attribute and check if it has expected values
*
* TODO: Feel free to remove after 2023, if no more new reports arrive.
*
* https://we.phorge.it/T15316
*
* @param mixed $href Value to be checked
* @param string $variable_name Human reference
*/
private static function requireValidHref($href, $variable_name) {
// We have not a very clear idea about what a "href" should be
if (is_object($href) && !($href instanceof PhutilURI)) {
// We log stuff with a kind stack trace
phlog(new Exception(pht(
'The variable %s received an unexpected type: %s. '.
'Please share this stack trace as comment in Task %s',
$variable_name,
get_class($href),
'https://we.phorge.it/T15316')));
}
}
}