Page MenuHomePhorge
Create Task
Maniphest T15372

"RuntimeException: Undefined array key" when pasting "a {{{a}}} a" comment (due to regex in PhutilRemarkupInterpreterBlockRule.php)
Open, Needs TriagePublic
Actions

Description

Upstreaming from https://phabricator.wikimedia.org/T284397

  1. Phorge at 8d2b481bb5a83d1a4ce82014cabdc83fa3ca432d, PHP 8.2.5 (though irrelevant here)
  2. Go to an existing task.
  3. Write the comment a {{{a}}} a
[2023-05-09 12:12:57] EXCEPTION: (RuntimeException) Undefined array key 1 at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=renderDashboardTabPanelContent, ref.master=d47289622650, ref.renderDashboardTabPanelContent=d47289622650), phorge(head=master, ref.master=8d2b481bb5a8)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php:243]
  #1 <#2> PhutilRemarkupEngine::splitTextIntoBlocks(array) called at [<phorge>/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php:136]
  #2 <#2> PhutilRemarkupEngine::preprocessText(string) called at [<phorge>/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php:106]
  #3 <#2> PhutilRemarkupEngine::markupText(string) called at [<phorge>/src/infrastructure/markup/PhabricatorMarkupEngine.php:614]
  #4 <#2> PhabricatorMarkupEngine::extractPHIDsFromMentions(PhabricatorUser, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2059]
  #5 <#2> PhabricatorApplicationTransactionEditor::buildSubscribeTransaction(ManiphestTask, array, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2221]
  #6 <#2> PhabricatorApplicationTransactionEditor::expandSupportTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1222]
  #7 <#2> PhabricatorApplicationTransactionEditor::applyTransactions(ManiphestTask, array) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:2031]
  #8 <#2> PhabricatorEditEngine::buildCommentResponse(ManiphestTask) called at [<phorge>/src/applications/transactions/editengine/PhabricatorEditEngine.php:998]
  #9 <#2> PhabricatorEditEngine::buildResponse() called at [<phorge>/src/applications/maniphest/controller/ManiphestTaskEditController.php:12]
  #10 <#2> ManiphestTaskEditController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #11 phlog(RuntimeException) called at [<phorge>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27]
  #12 PhabricatorAjaxRequestExceptionHandler::handleRequestThrowable(AphrontRequest, RuntimeException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:751]
  #13 AphrontApplicationConfiguration::handleThrowable(RuntimeException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:296]
  #14 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:203]
  #15 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

Desired situation (without any crash):

Phorge Cowsay Figlet showcase.png (891×837 px, 66 KB)

Revisions and Commits

rP Phorge
ClosedD25415 Make InterpreterBlockRule regex only match on valid interpreter names
Needs RevisionD25234 Fix exception and error message rendering some markup matching figlet/cowsay regex
D25559rP9ebe85621919 Update unit test of InterpreterBlockRule for "a {{{a}}}"

Related Objects

Event Timeline

aklapper created this task.May 9 2023, 12:14
Herald added subscribers: Cigaryno, Matthew, chris and 2 others. · View Herald TranscriptMay 9 2023, 12:14
aklapper added a comment.May 9 2023, 12:17

This is not triggered for {{{a}}} a or with a a {{{a}}} a, only for 1 string in front of a string with three squares.

The problem is in getMatchingLineCount() in PhutilRemarkupInterpreterBlockRule.php:
preg_match(self::START_BLOCK_PATTERN) being /^([\w]+)\s*(?:\(([^)]+)\)\s*)?{{{/ is matched ($num_lines++ setting 1), and then while (isset($lines[$cursor])) is true ($num_lines++ setting 2).
Afterwards in PhutilRemarkEngine.php, $block['num_lines'] is one larger than sizeof($blocks), so for ($ii = $min; $ii < $max; $ii++) { $lines .= $text[$ii]; } reaches a non-existing array field.

It returns $num_lines = 0 for the non-testcases above.

The correct fix seems correcting self::START_BLOCK_PATTERN (if anyone is more into regexes than I am) not to trigger for that one-string-prefix situation.

avivey removed a project: Phorge.May 9 2023, 12:52
valerio.bozzolan subscribed.May 9 2023, 13:27
aklapper mentioned this in T15371: RuntimeException in preg_replace_callback: Text disappears due to Remarkup parsing.May 9 2023, 13:37
aklapper added a comment.May 13 2023, 18:20

Historical commits with some comments about that regex: https://secure.phabricator.com/D7338, https://secure.phabricator.com/D7012

valerio.bozzolan added a project: Bug Reports.May 19 2023, 18:01
aklapper added a revision: D25234: Fix exception and error message rendering some markup matching figlet/cowsay regex.May 20 2023, 15:31
aklapper added a comment.May 20 2023, 15:33

By coincidence, chatting with valerio.bozzolan yesterday made me realize that this rendering rule is merely in place for figlet and cowsay.

Currently, entering a {{{a}}} a markup as a new comment shows as preview the confusing error message "No interpreter found":

phantasstick.png (951×1 px, 59 KB)

Currently, trying to render such markup for existing comments somehow imported from other systems throws a RunTimeException.

valerio.bozzolan updated the task description. (Show Details)May 20 2023, 20:17

Thanks again, I really loved this follow-up on your hack

avivey mentioned this in D25234: Fix exception and error message rendering some markup matching figlet/cowsay regex.May 26 2023, 06:34
aklapper mentioned this in D25415: Make InterpreterBlockRule regex only match on valid interpreter names.Aug 19 2023, 18:22
aklapper added a comment.Aug 19 2023, 18:22

FYI I proposed a partial patch in D25415 which makes the issue more contained at least.

aklapper mentioned this in rP7868ab3754fa: Make InterpreterBlockRule regex only match on valid interpreter names.Sep 4 2023, 11:12
valerio.bozzolan added a revision: D25415: Make InterpreterBlockRule regex only match on valid interpreter names.Mon, Mar 25, 07:12
valerio.bozzolan added a revision: D25559: Update unit test of InterpreterBlockRule for "a {{{a}}}".Mon, Mar 25, 07:18
valerio.bozzolan added a commit: rP9ebe85621919: Update unit test of InterpreterBlockRule for "a {{{a}}}".Mon, Mar 25, 08:28
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0