1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Fix PHP 8.1 exceptions which block adding an embedded File preview as a Comment

Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
Receiving null 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.

Closes T15389

Test Plan:
Applied these three changes; afterwards writing `{F1234}` in the Task Comment text field finally rendered the image preview in the preview area, plus I could successfully add the Comment.

Additional cute tests:

```
Test alt:

{F1234,alt=0}
{F1234,alt=null}
{F1234,alt=123}
{F1234,alt=0.1}
{F1234,alt=[]}

Test width:

{F1234,width=0}
{F1234,width=10}
{F1234,width=10.1}
{F1234,width=10%}
{F1234,width=49.99%}
{F1234,width=100%}

Test height:

{F1234,height=0}
{F1234,height=10}
{F1234,height=10.1}
{F1234,height=10%}
{F1234,height=49.99%}
{F1234,height=100%}

Test mix width/height:

{F1234,height=0,width=0}
{F1234,height=10,width=15}
{F1234,height=10.1,width=15}
{F1234,height=10%,width=15%}
{F1234,height=49.99%,width=15%}
{F1234,height=100%,width=15%}
```

If you have no nuclear implosion and if dimensions are somehow respected and the alt text works, well done! Test passed.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15389

Differential Revision: https://we.phorge.it/D25221
This commit is contained in:
Andre Klapper 2023-05-23 11:54:39 +02:00
parent d95200da91
commit 3999a28674

View file

@ -197,7 +197,7 @@ final class PhabricatorEmbedFileRemarkupRule
$alt = $options['alt']; $alt = $options['alt'];
} }
if (!strlen($alt)) { if (!phutil_nonempty_string($alt)) {
$alt = $file->getAltText(); $alt = $file->getAltText();
} }
@ -346,10 +346,11 @@ final class PhabricatorEmbedFileRemarkupRule
} }
private function parseDimension($string) { private function parseDimension($string) {
$string = trim($string); if ($string !== null) {
$string = trim($string);
if (preg_match('/^(?:\d*\\.)?\d+%?$/', $string)) { if (preg_match('/^(?:\d*\\.)?\d+%?$/', $string)) {
return $string; return $string;
}
} }
return null; return null;