Pygmentize: Treat Shellbox network loss like non-zero exit code

Prior to the shellbox migration, if during the parsing of a page,
pygmentize failed (i.e. non-zero exit from its local shell command,
pretty much the only way a php shell exec could fail), then
SyntaxHighlight would fallback to outputting a preformatted plain
`<pre>`.

The logic still exists in the code, and is still triggered for cases
where the command reached shellbox and its result was "successfully"
communicated to MediaWiki (HTTP 200), with the boxed result reporting
the non-zero exit code on the shellbox server.

However, the more likely scenario in the new setup is that the command
times out or never reaches the server in the first place, in which
case we don't get any shell exit code. Instead, we get a Shellbox
exception since the result is unknowable.

Instead of fatalling the entire pageview with a PHP exception and
HTTP 500 from MW, use the same graceful fallback.

Bug: T292663
Change-Id: Icaa8c34ff97ad8a99d044beab529ef943071269c
This commit is contained in:
Timo Tijhof 2023-06-03 14:16:19 +01:00
parent ca0ed556a9
commit 682fe922f9
2 changed files with 99 additions and 10 deletions

View file

@ -22,6 +22,7 @@ namespace MediaWiki\SyntaxHighlight;
use MediaWiki\MediaWikiServices;
use Shellbox\Command\BoxedCommand;
use Shellbox\ShellboxError;
/**
* Wrapper around the `pygmentize` command
@ -291,22 +292,33 @@ class Pygmentize {
foreach ( $options as $k => $v ) {
$optionPairs[] = "{$k}={$v}";
}
$result = self::boxedCommand()
->params(
self::getPath(),
'-l', $lexer,
'-f', 'html',
'-O', implode( ',', $optionPairs ),
'file'
)
->inputFileFromString( 'file', $code )
->execute();
self::recordShellout( 'highlight' );
try {
$result = self::boxedCommand()
->params(
self::getPath(),
'-l', $lexer,
'-f', 'html',
'-O', implode( ',', $optionPairs ),
'file'
)
->inputFileFromString( 'file', $code )
->execute();
} catch ( ShellboxError $exception ) {
// If we have trouble sending or receiving over the network to
// Shellbox, we technically don't know if the command succeed or failed,
// but, treat the highlight() command as recoverable by wrapping this in
// PygmentsException. This permits the Parser tag to fallback to
// plainCodeWrap(), thus avoiding a fatal on pageviews (T292663).
throw new PygmentsException( 'ShellboxError', 0, $exception );
}
$output = $result->getStdout();
if ( $result->getExitCode() != 0 ) {
throw new PygmentsException( $output );
}
return $output;
}

View file

@ -0,0 +1,77 @@
<?php
use MediaWiki\Shell\CommandFactory;
use MediaWiki\SyntaxHighlight\SyntaxHighlight;
use Shellbox\Command\BoxedCommand;
use Shellbox\Command\BoxedResult;
use Shellbox\ShellboxError;
/**
* @covers MediaWiki\SyntaxHighlight\SyntaxHighlight
* @covers MediaWiki\SyntaxHighlight\Pygmentize
*/
class PygmentizeTest extends MediaWikiIntegrationTestCase {
protected function setUp(): void {
parent::setUp();
$this->setMwGlobals( [
// Run with the default useBundled=true
'wgPygmentizePath' => false,
// Silence wfWarn for the expected Shellbox error
'wgDevelopmentWarnings' => false,
] );
}
private function stubShellbox( ?BoxedResult $result, ?Exception $e ) {
$factory = $this->createStub( CommandFactory::class );
$command = new class ( $result, $e ) extends BoxedCommand {
private $result;
private $e;
public function __construct( $result, $e ) {
$this->result = $result;
$this->e = $e;
}
public function execute(): BoxedResult {
if ( $this->e ) {
throw $this->e;
}
return $this->result;
}
};
$factory->method( 'createBoxed' )->willReturn( $command );
$this->setService( 'ShellCommandFactory', $factory );
}
public static function provideHighlight() {
yield 'basic' => [
( new BoxedResult )
->stdout( '<div class="mw-highlight><code>x</code></div>' )
->exitCode( 0 ),
null,
'<div class="mw-highlight mw-highlight-lang-json mw-content-ltr" dir="ltr"><code>x</code></div>'
];
yield 'pre-fallback for non-zero exit' => [
( new BoxedResult )
->stdout( 'Boo' )
->exitCode( 42 ),
null,
'<div class="mw-highlight mw-highlight-lang-json mw-content-ltr" dir="ltr"><pre>"example"</pre></div>'
];
yield 'pre-fallback for network error (T292663)' => [
null,
new ShellboxError( 'Wazaaaa', 0 ),
'<div class="mw-highlight mw-highlight-lang-json mw-content-ltr" dir="ltr"><pre>"example"</pre></div>'
];
}
/**
* @dataProvider provideHighlight
*/
public function testHighlightBasic( ?BoxedResult $result, ?Exception $e, string $expect ) {
$this->stubShellbox( $result, $e );
$status = SyntaxHighlight::highlight( '"example"', 'json' );
$this->assertSame( $expect, $status->getValue() );
}
}