From d943fc5f493748ff03ade1542c45bb600db23eae Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 13 May 2026 15:29:03 +0200 Subject: [PATCH 1/2] fix(translation): use {N} placeholders so URLs survive CJK translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PUA codepoint placeholders (+) were being dropped by ML Kit's Japanese/Chinese/Korean models, eliding image URLs from translated text (issue #1180). On-device probing across 8 source languages and 12 placeholder styles shows ICU MessageFormat tokens `{0}`, `{1}`, … are the only style preserved intact by every model we tested. build() now scans the source for any user-supplied `{N}` and starts the dictionary counter above the max, so a note containing `printf("%s", arg{0})` can't be clobbered by encode/decode. Adds an on-device regression test with the exact post from the bounty issue and JVM tests for the new collision-avoidance behaviour. --- .../amethyst/TranslationsTest.kt | 14 ++++++ .../service/lang/TranslationDictionary.kt | 30 +++++++++--- .../service/lang/TranslationDictionaryTest.kt | 48 +++++++++++++++---- 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/amethyst/src/androidTestPlay/java/com/vitorpamplona/amethyst/TranslationsTest.kt b/amethyst/src/androidTestPlay/java/com/vitorpamplona/amethyst/TranslationsTest.kt index 07bacf12d..f7e4901a8 100644 --- a/amethyst/src/androidTestPlay/java/com/vitorpamplona/amethyst/TranslationsTest.kt +++ b/amethyst/src/androidTestPlay/java/com/vitorpamplona/amethyst/TranslationsTest.kt @@ -167,6 +167,20 @@ class TranslationsTest { ) } + @Test + fun testJapaneseImageUrlIssue1180() { + // Regression for https://github.com/vitorpamplona/amethyst/issues/1180 — image URL at the end + // of a Japanese post must survive ja→en auto-translation intact. + assertTranslateContains( + "https://image.nostr.build/3cd07f308c13dd9197d03430a75bf5326f86d2ab68674af971e915f55a638ecd.jpg", + "「正方形のカードを並べて盤面にして、その上でカードゲームとボードゲーム同時にやったらおもろくね？」" + + "という誰でも思いつきはしそうなアイデアを、ちゃんとルール練り上げて実装まで漕ぎ着けてしまったやつ。" + + "ブースの人が「こんなところに気を遣ったんですわ〜」っていうのに全部ウンウンわかるそれそれと思わされたので買った " + + "https://image.nostr.build/3cd07f308c13dd9197d03430a75bf5326f86d2ab68674af971e915f55a638ecd.jpg ", + "en", + ) + } + @Test fun testHttp() { val text = "https://m.primal.net/MdDd.png \nRunning... \uD83D\uDE01 nostr:npub126ntw5mnermmj0znhjhgdk8lh2af72sm8qfzq48umdlnhaj9kuns3le9ll nostr:npub1getal6ykt05fsz5nqu4uld09nfj3y3qxmv8crys4aeut53unfvlqr80nfm" diff --git a/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt b/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt index 5a504ebbe..0f9566ef2 100644 --- a/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt +++ b/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt @@ -30,11 +30,12 @@ import java.util.regex.Pattern * can be unit-tested without ML Kit / Android runtime. */ internal object TranslationDictionary { - // Range U+E000..U+F8FF gives 6400 placeholder slots. PUA codepoints don't appear in normal - // user text, the translator has no rule for them so it passes them through, and using one - // codepoint per placeholder means the translator can't split or reorder it. - const val PLACEHOLDER_BASE: Int = 0xE000 - const val PLACEHOLDER_LIMIT: Int = 0xF8FF - PLACEHOLDER_BASE + // Placeholders use ICU MessageFormat positional syntax: {0}, {1}, .... ML Kit's translation + // models are trained on localization corpora and preserve these tokens intact across all source + // languages we measured (ja, zh, ko, ar, hi, ru, pt, de). Prior schemes failed: U+E000+ PUA + // codepoints were dropped by CJK models (issue #1180); "B0/C0/A0" markers collided with user + // text. 10k slots per note is effectively unbounded. + const val PLACEHOLDER_LIMIT: Int = 9999 // Texts shorter than this, or with no letter codepoints (emoji-only, punctuation), are skipped // before any ML Kit work — language identification is unreliable on them. @@ -51,6 +52,11 @@ internal object TranslationDictionary { // brackets ("# [0]"), so we shield them via the placeholder dictionary. val nip08RefRegex: Pattern = Pattern.compile("#\\[\\d+]") + // Matches placeholders the user already wrote in their note (e.g. a code snippet containing + // `{0}`). We start our own counter above the max user-supplied index so encode/decode can't + // clobber it. + private val placeholderRegex: Pattern = Pattern.compile("\\{(\\d+)\\}") + fun isWorthTranslating(text: String): Boolean { if (text.length < MIN_TRANSLATABLE_LENGTH) return false for (cp in text.codePoints()) { @@ -61,7 +67,7 @@ internal object TranslationDictionary { fun build(text: String): Map { val dict = LinkedHashMap() - var counter = 0 + var counter = firstFreeIndex(text) fun addUnique(value: String) { if (value.isEmpty()) return @@ -84,6 +90,16 @@ internal object TranslationDictionary { return dict } + private fun firstFreeIndex(text: String): Int { + var max = -1 + val matcher = placeholderRegex.matcher(text) + while (matcher.find()) { + val idx = matcher.group(1)?.toIntOrNull() ?: continue + if (idx > max) max = idx + } + return max + 1 + } + private inline fun Pattern.forEachMatch( text: String, block: (String) -> Unit, @@ -119,6 +135,6 @@ internal object TranslationDictionary { fun placeholder(index: Int): String { require(index in 0..PLACEHOLDER_LIMIT) { "placeholder index $index out of range" } - return String(Character.toChars(PLACEHOLDER_BASE + index)) + return "{$index}" } } diff --git a/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt b/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt index a415c5241..78604d18f 100644 --- a/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt +++ b/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt @@ -60,14 +60,11 @@ class TranslationDictionaryTest { // ----- placeholder ----- @Test - fun `placeholder is a single Unicode Private Use Area codepoint`() { - val p0 = TranslationDictionary.placeholder(0) - val p1 = TranslationDictionary.placeholder(1) - assertEquals(1, p0.codePointCount(0, p0.length)) - assertEquals(1, p1.codePointCount(0, p1.length)) - assertEquals(0xE000, p0.codePointAt(0)) - assertEquals(0xE001, p1.codePointAt(0)) - assertNotEquals(p0, p1) + fun `placeholder is the ICU MessageFormat positional token for that index`() { + assertEquals("{0}", TranslationDictionary.placeholder(0)) + assertEquals("{1}", TranslationDictionary.placeholder(1)) + assertEquals("{42}", TranslationDictionary.placeholder(42)) + assertNotEquals(TranslationDictionary.placeholder(0), TranslationDictionary.placeholder(1)) } @Test @@ -167,7 +164,7 @@ class TranslationDictionaryTest { val encoded = TranslationDictionary.encode(text, dict) assertFalse("URL must be removed from encoded text", encoded.contains("https://t.me/mygroup")) - assertTrue("encoded text must contain the placeholder", encoded.codePoints().anyMatch { it in 0xE000..0xF8FF }) + assertTrue("encoded text must contain a {N} placeholder", Regex("\\{\\d+}").containsMatchIn(encoded)) val decoded = TranslationDictionary.decode(encoded, dict) assertEquals(text, decoded) @@ -185,7 +182,11 @@ class TranslationDictionaryTest { val decoded = TranslationDictionary.decode(translated, dict)!! assertTrue("decoded must contain #[0]", decoded.contains("#[0]")) assertTrue("decoded must contain the nostr ref", decoded.contains("nostr:nevent1qqsabcdefgh023456")) - assertFalse("decoded must not leak placeholder codepoints", decoded.codePoints().anyMatch { it in 0xE000..0xF8FF }) + // The dictionary picks indices above any user-supplied {N}; user text here has none, so the + // dict starts at {0}. After decode, no {N} placeholder we issued should remain. + for (token in dict.keys) { + assertFalse("decoded leaked placeholder $token", decoded.contains(token)) + } } @Test @@ -280,4 +281,31 @@ class TranslationDictionaryTest { assertFalse(encoded.contains("nostr:npub126ntw5mnermmj0znhjhgdk8lh2af72sm8qfzq48umdlnhaj9kuns3le9ll")) assertFalse(encoded.contains("nostr:npub1getal6ykt05fsz5nqu4uld09nfj3y3qxmv8crys4aeut53unfvlqr80nfm")) } + + @Test + fun `build allocates indices above any user-supplied placeholders`() { + // User's note contains literal {0} and {3} (e.g. a code snippet). Our dictionary must not + // reuse those indices, or decode would clobber the user's own text. + val text = "Code is printf(\"%s\", arg{0}) or fmt(\"%s\", arg{3}); see https://docs.example.com" + val dict = TranslationDictionary.build(text) + assertTrue("dict should include the URL", dict.containsValue("https://docs.example.com")) + for (token in dict.keys) { + assertFalse("$token collides with user-supplied {0}", token == "{0}") + assertFalse("$token collides with user-supplied {3}", token == "{3}") + } + } + + @Test + fun `round-trip preserves user-supplied placeholders in source text`() { + // The user wrote {0} and {3}; after encode/decode they must round-trip intact alongside our + // own placeholder for the URL. + val text = "Code: printf(\"%s\", arg{0}) and fmt(\"%s\", arg{3}); see https://docs.example.com" + val dict = TranslationDictionary.build(text) + val encoded = TranslationDictionary.encode(text, dict) + // Encoded form still contains the user's {0} and {3} verbatim — we never touched them. + assertTrue(encoded.contains("arg{0}")) + assertTrue(encoded.contains("arg{3}")) + val decoded = TranslationDictionary.decode(encoded, dict) + assertEquals(text, decoded) + } } From 963232534931c71dc8502819f79e9e58a33d6eea Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 13 May 2026 15:46:06 +0200 Subject: [PATCH 2/2] Code review: - tighten placeholder dictionary after review - Skip the placeholder scan when the source has no `{` (avoids Matcher allocation on the common path). - Clamp `firstFreeIndex` to PLACEHOLDER_LIMIT so adversarial user text like `{99999}` can't push our counter past the table limit and make placeholder() throw for the rest of the note. New JVM test covers it. - split clamp and max-update in firstFreeIndex --- .../service/lang/TranslationDictionary.kt | 10 ++++++--- .../service/lang/TranslationDictionaryTest.kt | 21 ++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt b/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt index 0f9566ef2..34299e797 100644 --- a/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt +++ b/amethyst/src/play/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionary.kt @@ -25,9 +25,9 @@ import java.util.regex.Pattern /** * Pure-JVM helpers that protect non-translatable substrings (URLs, Lightning invoices, NIP-19 - * references, NIP-08 positional references) by swapping them with single Unicode Private Use Area - * codepoints around a translator. Extracted out of [LanguageTranslatorService] so the round-trip - * can be unit-tested without ML Kit / Android runtime. + * references, NIP-08 positional references) by swapping them with ICU MessageFormat positional + * tokens ({0}, {1}, ...) around a translator. Extracted out of [LanguageTranslatorService] so the + * round-trip can be unit-tested without ML Kit / Android runtime. */ internal object TranslationDictionary { // Placeholders use ICU MessageFormat positional syntax: {0}, {1}, .... ML Kit's translation @@ -91,10 +91,14 @@ internal object TranslationDictionary { } private fun firstFreeIndex(text: String): Int { + if (text.indexOf('{') < 0) return 0 var max = -1 val matcher = placeholderRegex.matcher(text) while (matcher.find()) { val idx = matcher.group(1)?.toIntOrNull() ?: continue + // Clamp at the table limit so adversarial user text like `{99999}` can't push our + // counter past PLACEHOLDER_LIMIT and make every subsequent placeholder() throw. + if (idx > PLACEHOLDER_LIMIT) continue if (idx > max) max = idx } return max + 1 diff --git a/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt b/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt index 78604d18f..3eaa020bd 100644 --- a/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt +++ b/amethyst/src/testPlay/java/com/vitorpamplona/amethyst/service/lang/TranslationDictionaryTest.kt @@ -214,9 +214,9 @@ class TranslationDictionaryTest { @Test fun `decode does not corrupt user text containing the old B0 C0 A0 placeholders`() { - // Regression for the pre-rewrite bug: old placeholders "B0", "C0", "A0" collided with arbitrary - // user content. The new PUA placeholders are invisible codepoints that cannot occur in normal text, - // so a sentence mentioning "B0" or "C0" should round-trip unchanged when there's nothing to replace. + // Regression for a pre-rewrite bug where placeholders were "B0", "C0", "A0" — those tokens + // collided with arbitrary user content. Today's {N} tokens can't collide with B0/C0/A0 + // strings, but this case is cheap to keep as a guard. val text = "Pricing tier B0 vs C0 vs A0 — see https://docs.example.com/tiers" val dict = TranslationDictionary.build(text) val encoded = TranslationDictionary.encode(text, dict) @@ -295,6 +295,21 @@ class TranslationDictionaryTest { } } + @Test + fun `build is resilient to adversarial user placeholders beyond the table limit`() { + // Adversarial note carries `{99999}` — well past PLACEHOLDER_LIMIT. If we naively started + // our counter at max+1, the very next call to placeholder() would throw and translation + // would fail entirely. Counter must stay within the allowed range. + val text = "weird {99999} placeholder; see https://docs.example.com" + val dict = TranslationDictionary.build(text) + assertTrue("dict should include the URL", dict.containsValue("https://docs.example.com")) + // {99999} is out of range and never enters our dict, so it survives encode/decode untouched. + val encoded = TranslationDictionary.encode(text, dict) + assertTrue("user's {99999} must survive encode", encoded.contains("{99999}")) + val decoded = TranslationDictionary.decode(encoded, dict) + assertEquals(text, decoded) + } + @Test fun `round-trip preserves user-supplied placeholders in source text`() { // The user wrote {0} and {3}; after encode/decode they must round-trip intact alongside our