diff --git a/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt b/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt index 4ab1a905..fe5b0767 100644 --- a/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt +++ b/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt @@ -50,6 +50,7 @@ fun generate(inputDirectory: File, outputDirectory: File): GenerationResult { + "language is where every key and every argument count comes from" ) + refuseIllegalIdentifiers(base) refuseIdentifierCollisions(base) val others = translations.filter { it.language != BASE_LANGUAGE } @@ -80,6 +81,21 @@ fun generate(inputDirectory: File, outputDirectory: File): GenerationResult { ) } +private fun refuseIllegalIdentifiers(base: Translation) { + val illegal = illegalIdentifierKeys(base.entries.keys) + + if (illegal.isEmpty()) { + return + } + + throw TranslationException( + "strings_$BASE_LANGUAGE.properties has keys that do not form a usable name -- a key is " + + "letters and digits with . _ - between words, nothing else. A key carrying " + + "anything more would be written into the generated source as code: " + + illegal.sorted().joinToString(", ") + ) +} + private fun refuseIdentifierCollisions(base: Translation) { val collisions = identifierCollisions(base.entries.keys) diff --git a/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt b/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt index a7dd1f81..9be4add2 100644 --- a/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt +++ b/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt @@ -59,5 +59,24 @@ fun identifierFor(key: String): String { fun identifierCollisions(keys: Collection): Map> = keys.groupBy { identifierFor(it) }.filterValues { it.size > 1 } +/** + * Keys whose identifier would not be a legal Kotlin name. + * + * [identifierFor] strips only `.`, `_` and `-`; every other character a `Properties` file can + * carry in a key -- a space, a brace, a quote, a `=`, a newline decoded from a `\uXXXX` escape + * -- passes through into identifier position unchanged. Emitted there it does not merely fail + * to compile: the member is written as `val : String get() = lookup(...)`, so a + * key that reads `x: String get() = "" ... init { ... } ... val y` breaks out of the template + * and leaves a property initializer or an init block of the contributor's choosing, which + * runs when `Strings` is instantiated in the application. The value is already escaped before + * it becomes a literal; the identifier was not checked at all. So a generated name that is + * not letters-then-letters-and-digits is refused before any source is written. + */ +fun illegalIdentifierKeys(keys: Collection): List = + keys.filterNot { IDENTIFIER.matches(identifierFor(it)) } + +/** A letter, then letters and digits: the plain Kotlin identifier this generator emits. */ +private val IDENTIFIER = Regex("[A-Za-z][A-Za-z0-9]*") + /** `{0}`, `{12}`, `{0,number}` -- a brace, digits, then either a brace or a comma. */ private val PLACEHOLDER = Regex("""\{(\d+)\s*[,}]""") diff --git a/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt b/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt index dc3f4899..62b8a83c 100644 --- a/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt +++ b/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt @@ -92,6 +92,32 @@ class GenerationTest { assertTrue(failure.message!!.contains("guiStartupDesign"), failure.message) } + @Test + fun aKeyCarryingKotlinSyntaxBreaksTheBuildRatherThanInjectingCode() { + /* + * The key, not the value, becomes the member name, and only . _ - are stripped from + * it. A \uXXXX escape lets a contributed .properties file carry a key that, dropped + * into `val : String get() = ...`, closes the template and opens a property + * initializer -- Kotlin that runs when Strings is built in every client. The base + * file is the vector: only base keys become members. This must fail the build the way + * an orphan or a collision does, not ship. + * + * The escapes below decode to the key `inject: Int = run{ pwn() }`. + */ + val (input, output) = directories() + write( + input, + "strings_en.properties", + "ok=fine\n" + + "inject\\u003a\\u0020Int\\u0020\\u003d\\u0020run\\u007b\\u0020pwn()\\u0020\\u007d=value\n", + ) + + val failure = assertThrows(TranslationException::class.java) { generate(input, output) } + + assertTrue(failure.message!!.contains("inject"), failure.message) + assertFalse(generated(output).exists(), "source was written despite an unusable key") + } + @Test fun aMissingBaseFileBreaksTheBuild() { val (input, output) = directories() diff --git a/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt b/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt index 02f842af..12dbcb85 100644 --- a/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt +++ b/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt @@ -103,4 +103,32 @@ class TranslationRulesTest { fun distinctKeysReportNoCollision() { assertTrue(identifierCollisions(listOf("a.b", "c.d")).isEmpty()) } + + @Test + fun aKeyThatWouldNotFormALegalIdentifierIsRejected() { + /* + * identifierFor strips only . _ -, so every other character a Properties file can + * carry in a key -- a space, a brace, a quote, a newline decoded from a \uXXXX escape + * -- would reach identifier position verbatim. Emitted there it does not merely fail + * to compile: it breaks out of `val : String get() = ...` into whatever the key + * says, which is Kotlin that then runs. So the generated name is held to the shape of + * an identifier before it is ever written. + */ + val illegal = illegalIdentifierKeys( + listOf("settings.save", "a b", "x\": String get() = \"\"", "init { pwn() }\n val") + ) + + assertEquals( + listOf("a b", "init { pwn() }\n val", "x\": String get() = \"\""), + illegal.sorted(), + ) + } + + @Test + fun ordinaryKeysFormLegalIdentifiersAndAreNotRejected() { + // A digit-leading key is fine: identifierFor prefixes it, which is a letter again. + assertTrue( + illegalIdentifierKeys(listOf("settings.save", "gui.startup_design", "2.metres")).isEmpty() + ) + } }