revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00
Code Issues Releases Wiki Activity

Fix "Undefined index" exception setting Meme text

Browse source 
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) Undefined index: above at [<arcanist>/src/error/PhutilErrorHandler.php:251]
 arcanist(), phorge()
   #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phorge>/src/applications/macro/engine/PhabricatorMemeEngine.php:276]
```

Closes T15637

Test Plan:
Create a meme called "angrycat" from the /macro/ page, and try a comment like this, expecting no nuclear implosion:

    {meme, src=angrycat, below=}
    {meme, src=angrycat, above=}
    {meme, src=angrycat, below=, above=}
    {meme, src=angrycat, below=  , above=  }
    {meme, src=angrycat, below=asd}
    {meme, src=angrycat, above=asd}
    {meme, src=angrycat, above=asd, below=dsa}
    {meme, src=angrycat, above=   asd   , below=   dsa  }

Also carefully read the code with your big eyes, keeping in mind that strlen does not accept passing `null` in PHP 8, and looking at what we did in rPb4cfe56f03b44615ac9251aed8d74bf13b085051.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15637

Differential Revision: https://we.phorge.it/D25437
This commit is contained in:
Andre Klapper 2023-09-16 14:13:21 +02:00
parent 2d635fb76e
commit 7b0021a03c
1 changed files with 2 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/applications/macro/engine/PhabricatorMemeEngine.php
View file

@ -273,7 +273,7 @@ final class PhabricatorMemeEngine extends Phobject {
$size = $metrics['size']; $size = $metrics['size'];
$above = $this->getAboveText(); $above = $this->getAboveText();
if (strlen($above)) { if ($above !== null && phutil_nonempty_string(trim($above))) {
$x = (int)floor(($dx - $metrics['text']['above']['width']) / 2); $x = (int)floor(($dx - $metrics['text']['above']['width']) / 2);
$y = $metrics['text']['above']['height'] + 12; $y = $metrics['text']['above']['height'] + 12;
@ -281,7 +281,7 @@ final class PhabricatorMemeEngine extends Phobject {
} }
$below = $this->getBelowText(); $below = $this->getBelowText();
if (strlen($below)) { if ($below !== null && phutil_nonempty_string(trim($below))) {
$x = (int)floor(($dx - $metrics['text']['below']['width']) / 2); $x = (int)floor(($dx - $metrics['text']['below']['width']) / 2);
$y = $dy - 12 - $metrics['text']['below']['descend']; $y = $dy - 12 - $metrics['text']['below']['descend'];