Read three keys for what they are, not for one shape each

src/Otl.php

@@ -2592,6 +2592,9 @@ function GSUBsubstitute($pos, $substitute, $Type, $GlyphPos = null)
 				} elseif ($this->shaper == 'M') {
 					$newOTLdata[$i]['myanmar_category'] = $this->OTLdata[$pos]['myanmar_category'];
 					$newOTLdata[$i]['myanmar_position'] = $this->OTLdata[$pos]['myanmar_position'];
+				} elseif ($this->shaper == 'E') {
+					$newOTLdata[$i]['sea_category'] = $this->OTLdata[$pos]['sea_category'];
+					$newOTLdata[$i]['sea_position'] = $this->OTLdata[$pos]['sea_position'];
 				}
 				if (isset($this->OTLdata[$pos]['mask'])) {
 					$newOTLdata[$i]['mask'] = $this->OTLdata[$pos]['mask'];
@@ -2784,6 +2787,9 @@ function GSUBsubstitute($pos, $substitute, $Type, $GlyphPos = null)
 			} elseif ($this->shaper == 'M') {
 				$newOTLdata[0]['myanmar_category'] = $this->OTLdata[$pos]['myanmar_category'];
 				$newOTLdata[0]['myanmar_position'] = $this->OTLdata[$pos]['myanmar_position'];
+			} elseif ($this->shaper == 'E') {
+				$newOTLdata[0]['sea_category'] = $this->OTLdata[$pos]['sea_category'];
+				$newOTLdata[0]['sea_position'] = $this->OTLdata[$pos]['sea_position'];
 			}
 			if (isset($this->OTLdata[$pos]['mask'])) {
 				$newOTLdata[0]['mask'] = $this->OTLdata[$pos]['mask'];

src/TTFontFile.php

@@ -1312,6 +1312,27 @@ function _getClassDefinitionTable($offset = 0)
 		return $GlyphByClass;
 	}
 
+	/**
+	 * Add the Private Use Area glyphs a replacement names to the list magic_reverse_dir reads.
+	 *
+	 * A replacement is one glyph code where the substitution replaces one glyph with one, and a
+	 * space-separated list where it replaces one with several - the dotless form of a letter and the
+	 * dots to draw under it, say.
+	 *
+	 * @param string[] $rtlpua  the list being built
+	 * @param string   $replace one glyph code, or several separated by spaces
+	 */
+	private function addPuaGlyphs(array &$rtlpua, $replace)
+	{
+		foreach (explode(' ', $replace) as $glyph) {
+			// Unanchored: a flattened contextual rule carries its backreferences in the same token as
+			// the glyph code, so '0FB93\2' names 0FB93
+			if (preg_match('/(0[EF][A-F0-9]{3})/', $glyph, $matched)) {
+				$rtlpua[] = $matched[1];
+			}
+		}
+	}
+
 	/**
 	 * GSUB - Glyph Substitution
 	 */
@@ -2122,7 +2143,9 @@ function _getGSUBtables()
 					foreach ($volt as $v) {
 						// isol fina fin2 fin3 medi med2 for Syriac
 						// ISOLATED FORM :: FINAL :: INITIAL :: MEDIAL :: MED2 :: FIN2 :: FIN3
-						if (strpos('isol fina init medi fin2 fin3 med2', $v['tag']) !== false) {
+						// A contextual entry carries the feature's own tag but keeps its replacements in
+						// ['rules'], so it has no ['replace'] for this branch to read
+						if (strpos('isol fina init medi fin2 fin3 med2', $v['tag']) !== false && !isset($v['context'])) {
 
 							$key = $v['match'];
 							$key = preg_replace('/[\(\)]*/', '', $key);
@@ -2153,31 +2176,20 @@ function _getGSUBtables()
 							if (isset($v['ignore']) && $v['ignore']) {
 								$rtl[$key]['ignore'][$kk] = $v['ignore'];
 							}
-							$rtlpua[] = $sub;
+							$this->addPuaGlyphs($rtlpua, $sub);
 
 						} else { // Add any other glyphs which are in PUA
 							if (isset($v['context']) && $v['context']) {
 								foreach ($v['rules'] as $vs) {
 									$matchCount = count($vs['match']);
 									for ($i = 0; $i < $matchCount; $i++) {
 										if (isset($vs['replace'][$i]) && preg_match('/^0[A-F0-9]{4}$/', $vs['match'][$i])) {
-											if (preg_match('/^0[EF][A-F0-9]{3}$/', $vs['replace'][$i])) {
-												$rtlpua[] = $vs['replace'][$i];
-											}
+											$this->addPuaGlyphs($rtlpua, $vs['replace'][$i]);
 										}
 									}
 								}
 							} else {
-								preg_match_all('/\((0[A-F0-9]{4})\)/', $v['match'], $m);
-								$matchCount = count($m[0]);
-								for ($i = 0; $i < $matchCount; $i++) {
-									$sb = explode(' ', $v['replace']);
-									foreach ($sb as $sbg) {
-										if (preg_match('/(0[EF][A-F0-9]{3})/', $sbg, $mr)) {
-											$rtlpua[] = $mr[1];
-										}
-									}
-								}
+								$this->addPuaGlyphs($rtlpua, $v['replace']);
 							}
 						}
 					}
@@ -2213,9 +2225,11 @@ function _getGSUBtables()
 					}
 
 					// First get 'locl' substitutions (reversed!)
+					// Contextual entries are skipped here as they are above: their replacements are in
+					// ['rules'], not ['replace']
 					$loclsubs = [];
 					foreach ($volt as $v) {
-						if (strpos('locl', $v['tag']) !== false) {
+						if (strpos('locl', $v['tag']) !== false && !isset($v['context'])) {
 							$key = $v['match'];
 							$key = preg_replace('/[\(\)]*/', '', $key);
 							$sub = $v['replace'];

tests/Mpdf/Shaper/SeaTest.php

@@ -0,0 +1,103 @@
+<?php
+
+namespace Mpdf\Shaper;
+
+use Mpdf\TextRecordingMpdf;
+
+/**
+ * The South East Asian shaper categorises every character of a run before it groups them into
+ * clusters, then applies 'locl' and 'ccmp' before it reorders each cluster. A substitution in that
+ * pass makes glyphs the categorising pass never saw, and Otl handed them on without a
+ * ['sea_category'] of their own - which the reorderer reads on the last glyph of a broken cluster.
+ *
+ * Lanna Alif is the font throughout. Its 'ccmp' ligates a Sakot with the consonant after it into a
+ * subscript form, and the glyph that comes out has no codepoint of its own, so it is mapped into the
+ * Private Use Area as the subset is built.
+ *
+ * What the read turns on is that the last glyph of the cluster came out of that pass, not how long
+ * the cluster is. Lanna Alif carries U+25CC, so a dotted circle is inserted ahead of the ligature
+ * and the cluster is two elements; Noto Sans Tai Tham carries none, and the same cluster is one.
+ * Both raised the warning.
+ */
+class SeaTest extends \Yoast\PHPUnitPolyfills\TestCases\TestCase
+{
+
+	/** U+1A20 TAI THAM LETTER HIGH KA */
+	const HIGH_KA = 0x1A20;
+
+	/** U+1A60 TAI THAM SIGN SAKOT, which subscripts the consonant after it */
+	const SAKOT = 0x1A60;
+
+	/** U+25CC DOTTED CIRCLE, inserted in front of a cluster with no base consonant */
+	const DOTTED_CIRCLE = 0x25CC;
+
+	/** The subscript High Ka that Lanna Alif's 'ccmp' substitutes for Sakot + High Ka */
+	const SAKOT_HIGH_KA = 0xF001;
+
+	/**
+	 * A Sakot with no consonant in front of it is a broken cluster, and the reorderer reads the
+	 * category of its last glyph before deciding whether to reorder anything. By then the Sakot and
+	 * the consonant after it are one ligated glyph the categorising pass never saw, so the read raised
+	 * a warning per run.
+	 */
+	public function testAGlyphSubstitutedBeforeTheReorderingCarriesACategoryIntoIt()
+	{
+		$raised = [];
+
+		set_error_handler(function ($number, $message, $file) use (&$raised) {
+			if (false !== strpos($file, 'Sea.php')) {
+				$raised[] = $message;
+			}
+
+			return true;
+		});
+
+		$drawn = $this->drawn([self::SAKOT, self::HIGH_KA]);
+
+		restore_error_handler();
+
+		$this->assertSame([], $raised);
+		$this->assertSame([self::DOTTED_CIRCLE, self::SAKOT_HIGH_KA], $drawn);
+	}
+
+	/**
+	 * The same Sakot inside a well-formed cluster, which is what the rule is for: the consonant in
+	 * front of it is the base, no dotted circle is needed, and the pair after it is subscripted under
+	 * it.
+	 */
+	public function testASakotInsideAClusterSubscriptsTheConsonantAfterIt()
+	{
+		$this->assertSame(
+			[self::HIGH_KA, self::SAKOT_HIGH_KA],
+			$this->drawn([self::HIGH_KA, self::SAKOT, self::HIGH_KA])
+		);
+	}
+
+	/**
+	 * A Sakot on its own ligates with nothing, so no glyph the categorising pass missed ever reaches
+	 * the reorderer and the character is drawn as it was written.
+	 */
+	public function testASakotWithNothingAfterItIsDrawnAsItself()
+	{
+		$this->assertSame([self::SAKOT], $this->drawn([self::SAKOT]));
+	}
+
+	/**
+	 * @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();
+		$mpdf->WriteHTML('<p style="font-family:lannaalif">' . $html . '</p>');
+
+		return array_values(unpack('N*', mb_convert_encoding($mpdf->drawnText[0], 'UTF-32BE', 'UTF-8')));
+	}
+
+}

tests/Mpdf/TTFontFileTest.php

@@ -83,6 +83,68 @@ public function testGetMetricsReadsMarkGlyphSetsCoverage()
 		$this->assertSame([' 00DCA| 00DD2| 00DD3', ' 0E00A'], $this->ttf->MarkGlyphSets);
 	}
 
+	/**
+	 * The Indic pre-pass that indexes 'locl' substitutions by their replacement read ['replace'] on
+	 * every entry tagged 'locl'. A contextual entry keeps its replacements in ['rules'] and has no
+	 * such key, so it read null once per font load.
+	 *
+	 * The subset is the contextual 'locl' of Noto Sans Devanagari 2.005 (OFL 1.1): a chained context
+	 * under the Santali language system whose backtrack is U+0905 or U+0906 and whose nested lookup
+	 * substitutes a Santali nukta for U+093C. Nothing is indexed from it either way, so the
+	 * diagnostic is all there was to fix.
+	 */
+	public function testAContextualLoclEntryIsNotReadForATopLevelReplacement()
+	{
+		$raised = $this->diagnosticsWhileParsing('NotoSansDevanagari-ContextualLocl-Subset.ttf');
+
+		$this->assertSame([], $raised);
+		$this->assertSame('NotoSansDevanagari-Regular', $this->ttf->fullName);
+	}
+
+	/**
+	 * A joining form can be a Multiple Substitution, so the replacement recorded for one can name more
+	 * than one glyph. The whole string was pushed into the Private Use Area list as if it named one,
+	 * and hexdec() raised PHP 8's invalid-characters deprecation on it and returned a number far
+	 * outside the PUA, so every glyph in it was dropped.
+	 *
+	 * The subset is Noto Sans Arabic 2.012 (OFL 1.1) cut down to U+0628 and U+06CC. The Farsi Yeh's
+	 * initial and medial forms are Multiple Substitutions replacing it with the dotless form and the
+	 * pair of dots to draw under it - E007 here, which has no codepoint of its own and is the glyph
+	 * the whole-string push lost.
+	 */
+	public function testAJoiningFormOfMoreThanOneGlyphContributesEachOfThem()
+	{
+		$raised = $this->diagnosticsWhileParsing('NotoSansArabic-MultipleForm-Subset.ttf');
+
+		$this->assertSame([], $raised);
+		$this->assertSame('\x{0E000}-\x{0E003}\x{0E005}-\x{0E007}', $this->ttf->rtlPUAstr);
+	}
+
+	/**
+	 * Parse a font, collecting every diagnostic PHP raised doing it. Deprecations are not converted to
+	 * exceptions, so a handler is what sees them.
+	 *
+	 * @return string[]
+	 */
+	private function diagnosticsWhileParsing($file)
+	{
+		$raised = [];
+
+		set_error_handler(function ($number, $message, $path, $line) use (&$raised) {
+			$raised[] = sprintf('%s in %s:%d', $message, basename($path), $line);
+
+			return true;
+		});
+
+		try {
+			$this->ttf->getMetrics(__DIR__ . '/../data/ttf/' . $file, uniqid('', true), 0, false, false, 0xFF);
+		} finally {
+			restore_error_handler();
+		}
+
+		return $raised;
+	}
+
 	/**
 	 * debugfont mode validates each table's version as it goes, so it walks the byte offsets by a
 	 * different route than normal mode and has its own chance to lose its place