Page MenuHomePhorge

Make InterpreterBlockRule regex only match on valid interpreter names

Authored by aklapper on Aug 19 2023, 18:22.
Referenced Files
Unknown Object (File)
Thu, Sep 21, 09:43
Unknown Object (File)
Sun, Sep 17, 05:28
Unknown Object (File)
Sun, Sep 17, 01:36
Unknown Object (File)
Fri, Sep 15, 23:39
Unknown Object (File)
Fri, Sep 15, 13:09
Unknown Object (File)
Wed, Sep 13, 12:41
Unknown Object (File)
Tue, Sep 12, 00:28
Unknown Object (File)
Tue, Sep 12, 00:17
"Love" token, awarded by valerio.bozzolan.



With this patch, the underlying exception described in T15372#8537 still remains. However, with this patch, the bug is more contained as it is not triggered when not calling an interpreter (cowsay, figlet), so Phorge does not crash rendering noValidInterpreter {{{foo}}} bar lines but renders them as is (for whatever reasons such lines may exist).

See T15372

Test Plan

Enter strings into a comment:

  • invalid {{{saysay}}} foo now renders as plain text instead of crashing
  • invalid (invalid) {{{saysay}}} foo now renders as plain text instead of crashing
  • cowsay (invalid) {{{saysay}}} foo will still crash as before

Diff Detail

rP Phorge
Lint Not Applicable
Tests Not Applicable

Event Timeline

This is very promising. Just added an inline comment


We can maybe try to avoid end($array) since it's supposed to modify the internal pointer of the array and here it works as expected only for truly dark magic from advanced PHP .

Also we can maybe call preg_quote() over each elements in order to sanitize the input before building the regex.

Also it's probably nice to just use implode() for concatenation.

Also interestingly we have a "method pull" function called mpull() that just creates an array with a function call.


$interpreter_names_regex = mpull($interpreters, 'getInterpreterName');
$interpreter_names_regex = array_map('preg_quote', $interpreter_names_regex);
$interpreter_names_regex = implode('|', $interpreter_names_regex);

Do you somehow like this?

Simplify code with neat functions as proposed by Valerio

Nice catch. I agree that we should allow to have texts like asd {{{ lol }}} whey without any nuclear implosion needed by design just because we are trying to give support to cowsay or figlet.

I tested and this seems good to me.

For the records, at the moment we have 3 block interpreters:

  • cowsay
  • figlet
  • phutil_test_block_interpreter (!)

I was not aware of the last one. Trying that:

phutil_test_block_interpreter (asd=ASD,lol) {{{ EHEHEHEH }}}

It renders as (the blockquote is mine):

Content: ( EHEHEHEH ) Argv: (asd=ASD&lol=1)

What a super-interesting new feature we discovered! Awesome. Now we can concentrate on the root cause of:

T15372: "RuntimeException: Undefined array key" when pasting "a {{{a}}} a" comment (due to regex in PhutilRemarkupInterpreterBlockRule.php)


✅ From PhutilClassMapQuery: «This class automatically caches class maps and does not need to be wrapped in caching logic.»

This revision is now accepted and ready to land.Mon, Sep 4, 10:51