Stop an Arabic form's context walk at the edge of the run (#204)

src/Shaper/Arabic.php

@@ -302,13 +302,16 @@ private static function glyphs($char, $type, &$chars, $i, $scriptTag, $usetags,
 		if ($retk != -1) {
 			$match = true;
 			// If GSUB includes a Backtrack or Lookahead condition (e.g. font ArabicTypesetting)
+			// The walk over ignored glyphs stops at the edge of the run, and a position it runs out before
+			// reaching is not held, as in HarfBuzz's match_backtrack() and match_lookahead(). Past the edge
+			// the glyph is null, which inList() finds in any pattern from PHP 8, so the walk would not end.
 			if (isset($arabGlyphs[$char]['prel'][$retk]) && $arabGlyphs[$char]['prel'][$retk]) {
 				$ig = 1;
 				foreach ($arabGlyphs[$char]['prel'][$retk] as $k => $v) { // $k starts 0, 1...
 					if (!isset($chars[$i - $ig - $k])) {
 						$match = false;
 					} elseif (!GlyphString::inList($v, $chars[$i - $ig - $k])) {
-						while (GlyphString::inList($arabGlyphs[$char]['ignore'][$retk], $chars[$i - $ig - $k])) {  // ignore
+						while (isset($chars[$i - $ig - $k]) && GlyphString::inList($arabGlyphs[$char]['ignore'][$retk], $chars[$i - $ig - $k])) {
 							$ig++;
 						}
 						if (!isset($chars[$i - $ig - $k])) {
@@ -325,7 +328,7 @@ private static function glyphs($char, $type, &$chars, $i, $scriptTag, $usetags,
 					if (!isset($chars[$i + $ig + $k])) {
 						$match = false;
 					} elseif (!GlyphString::inList($v, $chars[$i + $ig + $k])) {
-						while (GlyphString::inList($arabGlyphs[$char]['ignore'][$retk], $chars[$i + $ig + $k])) {  // ignore
+						while (isset($chars[$i + $ig + $k]) && GlyphString::inList($arabGlyphs[$char]['ignore'][$retk], $chars[$i + $ig + $k])) {
 							$ig++;
 						}
 						if (!isset($chars[$i + $ig + $k])) {

tests/Mpdf/ArabicContextEdgeTest.php

@@ -0,0 +1,90 @@
+<?php
+
+namespace Mpdf;
+
+/**
+ * An Arabic joining form whose chained rule has a backtrack or lookahead is matched by walking over
+ * the glyphs the rule's lookup ignores, and a word that starts or ends in those glyphs takes the walk
+ * to the edge of the run. It carried on past it, and from PHP 8 never returned (#204). Running out of
+ * glyphs there means the rule does not match, as HarfBuzz has it.
+ *
+ * No font in the corpus writes a backtrack or lookahead into its rtlSUB table.
+ * NotoSansArabic-ContextEdge-Synthetic is NotoSansArabic-Joining-Subset (Noto Sans Arabic 2.012, OFL
+ * 1.1) with a FATHA added, and its GSUB replaced by 'init' and 'fina' Chaining Context Substitutions
+ * (Type 6 Format 3) flagged IgnoreMarks: beh takes the dotless initial form after a low alef, and the
+ * dotless final form before one. Its 'ccmp' is dropped, because the shaper would take beh apart into a
+ * dotless beh and a dot before resolving joining, and a glyph that is not a character joins nothing
+ * (#209).
+ * `hb-shape` 14.3.1 draws the glyphs these tests expect.
+ */
+class ArabicContextEdgeTest extends \Yoast\PHPUnitPolyfills\TestCases\TestCase
+{
+
+	const BEH = 0x0628;
+
+	const FATHA = 0x064E;
+
+	const LOW_ALEF = 0x08AD;
+
+	/** The forms have no codepoint, so they are mapped into the Private Use Area */
+	const DOTLESS_BEH_FINAL = 0xE002;
+
+	const DOTLESS_BEH_INITIAL = 0xE004;
+
+	/**
+	 * @param int[] $codepoints
+	 *
+	 * @return int[] the codepoints of the line as it is handed to the drawing code, in visual order
+	 */
+	private function drawn($codepoints)
+	{
+		$html = '';
+		foreach ($codepoints as $codepoint) {
+			$html .= sprintf('&#x%04X;', $codepoint);
+		}
+
+		$mpdf = new TextRecordingMpdf([
+			'mode' => 'utf-8',
+			'fontDir' => [__DIR__ . '/../data/ttf'],
+			'fontdata' => ['notosansarabiccontextedgesynthetic' => [
+				'R' => 'NotoSansArabic-ContextEdge-Synthetic.ttf',
+				'useOTL' => 0xFF,
+			]],
+			'default_font' => 'notosansarabiccontextedgesynthetic',
+		]);
+		$mpdf->WriteHTML('<p dir="rtl">' . $html . '</p>');
+
+		return array_values(unpack('N*', mb_convert_encoding($mpdf->drawnText[0], 'UTF-32BE', 'UTF-8')));
+	}
+
+	public function dataRuns()
+	{
+		return [
+			'a word ending in a mark, where the lookahead runs out' => [
+				[self::BEH, self::BEH, self::FATHA],
+				[self::FATHA, self::BEH, self::BEH],
+			],
+			'a word starting with a mark, where the backtrack runs out' => [
+				[self::FATHA, self::BEH, self::BEH],
+				[self::BEH, self::BEH, self::FATHA],
+			],
+			'the lookahead met past the mark' => [
+				[self::BEH, self::BEH, self::FATHA, self::LOW_ALEF],
+				[self::LOW_ALEF, self::FATHA, self::DOTLESS_BEH_FINAL, self::BEH],
+			],
+			'the backtrack met past the mark' => [
+				[self::LOW_ALEF, self::FATHA, self::BEH, self::BEH],
+				[self::BEH, self::DOTLESS_BEH_INITIAL, self::FATHA, self::LOW_ALEF],
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider dataRuns
+	 */
+	public function testAFormsContextIsMatchedUpToTheEdgeOfTheRun($codepoints, $expected)
+	{
+		$this->assertSame($expected, $this->drawn($codepoints));
+	}
+
+}

tests/Mpdf/Fixtures/arabic-shape.php

@@ -0,0 +1,34 @@
+<?php
+
+/*
+ * Runs Arabic::shape() over each of a set of runs and prints, as JSON, each run's [hex, form] pairs
+ * under its name. Run in a process of its own, under a time limit, so a shaper that never returns
+ * ends here instead of in the suite.
+ *
+ * Usage: php arabic-shape.php <base64 of the JSON [{name: [hexes, rtlSUB]}, usetags, GDEF marks]>
+ */
+
+require __DIR__ . '/../../../vendor/autoload.php';
+
+set_time_limit(5);
+error_reporting(E_ERROR);
+
+list($runs, $usetags, $marks) = json_decode(base64_decode($argv[1]), true);
+
+$forms = [];
+foreach ($runs as $name => $run) {
+	list($hexes, $glyphs) = $run;
+
+	$info = [];
+	foreach ($hexes as $hex) {
+		$info[] = ['hex' => $hex, 'uni' => hexdec($hex)];
+	}
+
+	\Mpdf\Shaper\Arabic::shape($info, $glyphs, $marks, $usetags, 'arab');
+
+	foreach ($info as $char) {
+		$forms[$name][] = [$char['hex'], $char['form']];
+	}
+}
+
+echo json_encode($forms);

tests/Mpdf/Shaper/ArabicTest.php

@@ -512,6 +512,93 @@ public function dataContextNamingAPlaneSixteenGlyph()
 		];
 	}
 
+	/**
+	 * The walk over the glyphs a chained rule's lookup ignores read past the edge of the run: a notice
+	 * for each glyph on PHP 7, and from PHP 8 a walk that never returned (#204). Here the notice is what
+	 * fails, so a regression cannot hang the suite.
+	 *
+	 * @dataProvider dataContextWalkedToTheEdgeOfTheRun
+	 */
+	public function testAFormsContextWalkReadsNothingPastTheEdgeOfTheRun($hexes, $glyphs, $expected)
+	{
+		set_error_handler(function ($number, $message, $file, $line) {
+			throw new \ErrorException($message, 0, $number, $file, $line);
+		});
+
+		try {
+			$forms = $this->shape($hexes, self::ALL_FORMS, 'arab', self::FATHA, $glyphs);
+		} finally {
+			restore_error_handler();
+		}
+
+		$this->assertSame($expected, $forms);
+	}
+
+	/**
+	 * The same cases, all in one child process under a time limit, for a regression that reads past the
+	 * edge without a notice. The limit is the child's own rather than a `timeout` around it, so it holds
+	 * on Windows too, and the child reports nothing short of a fatal error, so a walk warning on every
+	 * step cannot fill the stderr pipe and leave it blocked instead of timed out.
+	 */
+	public function testAFormsContextWalkReturnsAtTheEdgeOfTheRun()
+	{
+		$runs = [];
+		$expected = [];
+		foreach ($this->dataContextWalkedToTheEdgeOfTheRun() as $name => $case) {
+			$runs[$name] = [$case[0], $case[1]];
+			$expected[$name] = $case[2];
+		}
+
+		// base64, because Windows argument quoting does not survive the JSON's double quotes
+		$arg = base64_encode(json_encode([$runs, self::ALL_FORMS, ' ' . self::FATHA]));
+		$command = escapeshellarg(PHP_BINARY) . ' -d display_errors=stderr '
+			. escapeshellarg(__DIR__ . '/../Fixtures/arabic-shape.php') . ' ' . $arg;
+
+		$process = proc_open($command, [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes, null, null, ['bypass_shell' => true]);
+		$output = stream_get_contents($pipes[1]);
+		$errors = stream_get_contents($pipes[2]);
+		fclose($pipes[1]);
+		fclose($pipes[2]);
+		$this->assertSame(0, proc_close($process), $errors);
+
+		$this->assertSame($expected, json_decode($output, true));
+	}
+
+	public function dataContextWalkedToTheEdgeOfTheRun()
+	{
+		// The forms are the presentation forms rather than names, because the run is left holding them
+		// and shape() reads each back as hex
+		$ignoreFatha = '((?:(?: ' . self::FATHA . '))*)';
+
+		return [
+			'a backtrack that runs out at the start of the run' => [
+				[self::FATHA, self::DAL],
+				[self::DAL => ['0FEA9', '0FEAA', 'prel' => [0 => [self::BEH]], 'ignore' => [0 => $ignoreFatha]]],
+				[[self::FATHA, 0], [self::DAL, 0]],
+			],
+			'a lookahead that runs out at the end of the run' => [
+				[self::BEH, self::FATHA],
+				[self::BEH => ['0FE8F', 'postl' => [0 => [self::DAL]], 'ignore' => [0 => $ignoreFatha]]],
+				[[self::BEH, 0], [self::FATHA, 0]],
+			],
+			'a lookahead whose second position runs out after the first is met' => [
+				[self::BEH, self::FATHA, self::DAL, self::FATHA],
+				[self::BEH => ['0FE8F', '0FE90', '0FE91', 'postl' => [2 => [self::DAL, self::DAL]], 'ignore' => [2 => $ignoreFatha]]],
+				[[self::BEH, 0], [self::FATHA, 0], [self::DAL, 0], [self::FATHA, 0]],
+			],
+			'a backtrack met past the ignored glyphs' => [
+				[self::BEH, self::FATHA, self::DAL],
+				[self::DAL => ['0FEA9', '0FEAA', 'prel' => [1 => [self::BEH]], 'ignore' => [1 => $ignoreFatha]]],
+				[[self::BEH, 0], [self::FATHA, 0], ['0FEAA', 1]],
+			],
+			'a lookahead met past the ignored glyphs' => [
+				[self::BEH, self::FATHA, self::DAL],
+				[self::BEH => ['0FE8F', '0FE90', '0FE91', 'postl' => [2 => [self::DAL]], 'ignore' => [2 => $ignoreFatha]]],
+				[['0FE91', 2], [self::FATHA, 0], [self::DAL, 0]],
+			],
+		];
+	}
+
 	/**
 	 * @return array one [hex, form] pair per character, in logical order
 	 */