Kuromoji's DictionaryBuilder silently truncates readings of 128 or more chars

Kuromoji lets an application build its own system dictionary from IPADIC- or UniDic-format CSV files with the public, @lucene.experimental DictionaryBuilder, and load it with the JapaneseTokenizer constructor that takes custom dictionaries. The builder stores each entry's reading and pronunciation behind a one-byte header, (byte) (length << 1 | kanaFlag), and never checks the length. A stored reading or pronunciation of 128 or more chars therefore keeps only its length mod 128: 128 chars come back as "" and 136 as the first 8, each field on its own, and a pronunciation stored after a wrapped reading is decoded from the reading's own bytes. ReadingAttribute passes the wrong values on, and JapaneseReadingFormFilter indexes a wrapped reading as the term. The same method rejects a stored base form of 16 or more chars and the ids its format cannot hold with an IllegalArgumentException; the reading and pronunciation lengths are the unguarded limits. The bundled dictionary (longest reading 32 chars) and the default analyzer are not affected. Present since kuromoji's first release, 3.6.0; an 8-line check fixes it.

• System dictionary: kuromoji's main dictionary of known words, compiled from CSV rows. Each row has the surface (the word as written), ids and costs, part-of-speech fields, the base form, the reading (how the word is read, in katakana) and the pronunciation (how it is said, for example with ー for a long vowel).
• Custom system dictionary: one an application compiles itself with DictionaryBuilder.build (ja/dict/DictionaryBuilder.java:58-73) and loads with the custom-dictionary JapaneseTokenizer constructor (ja/JapaneseTokenizer.java:191-217). The bundled dictionary is compiled the same way at Lucene build time.
• Stored only when needed: a reading is stored only when it differs from the katakana form of the surface, and a pronunciation only when it differs from the reading (ja/dict/TokenInfoDictionaryEntryWriter.java:110-114). Otherwise the reader derives them.

1. putEntry writes the reading as buffer.put((byte) (reading.length() << 1 | 1)) plus one byte per char for an all-katakana reading, or (byte) (reading.length() << 1) plus two bytes per char otherwise (TokenInfoDictionaryEntryWriter.java:151-161); the pronunciation the same way (:163-177). No range check. A few lines earlier, a stored base form of 16 or more chars throws IllegalArgumentException (:139-141).
2. The reader decodes the header as (b & 0xff) >>> 1 (TokenInfoMorphData.java:104-108, :130-134) and finds the pronunciation by skipping the reading by that decoded length (pronunciationOffset, :160-174).

1. Header 136 << 1 | 1 = 273; one byte keeps 273 - 256 = 17; 17 >>> 1 = 8, kana. The reading comes back as its first 8 chars, コクリツケンキュ.
2. The pronunciation really starts after all 136 reading bytes, but the reader skips only 8 and lands on the reading's 9th char, ウ, stored as byte 0x06: a header for 3 UTF-16 chars. The next 6 bytes are the reading's chars 10-15 (カイハツホウ = 0x0B 0x04 0x2F 0x24 0x3B 0x06), read as U+0B04 U+2F24 U+3B06.
3. A 128-char reading has the header 257, kept as 1: length 0, "".

The checks saw the same pattern for a 128-char hiragana reading (UTF-16 path: ""), the UniDic input format (8-char prefix), a 147-char reading (19-char prefix) and, on every release from 3.6.0 to 9.0.0, a 2-char pronunciation カン after a 128-char reading, which came back as カカカカカ. The fact check added an all-kana row: a 136-char katakana or hiragana surface whose reading equals it (so the reading is not stored) and the 136-char ー pronunciation; the reading comes back intact and the pronunciation as its first 8 chars, コクリツケンキュ.

• Who is hit. Applications with a custom system dictionary (IPADIC or UNIDIC format; both go through the same writer). The JapaneseTokenizer constructor that takes one says it "provides an entry point for users that want to construct custom language models that can be used as input to DictionaryBuilder" (JapaneseTokenizer.java:192-195).
• Not hit. The default JapaneseAnalyzer and JapaneseTokenizer (bundled dictionary, longest reading 32 chars over all 392,127 source rows); the japanese tokenizer factory, which takes only a user dictionary (JapaneseTokenizerFactory.java:86-88, :125-126); user dictionaries, which are separate code.
• Which rows. Only rows whose stored reading or pronunciation has 128 or more chars. A reading is not stored when it equals the katakana form of the surface, and a pronunciation is not stored when it equals the reading (TokenInfoDictionaryEntryWriter.java:110-114). So an all-kana entry whose reading and pronunciation both equal its katakana form is not affected. A long all-kana entry is still hit through a pronunciation that differs from its reading (for example ー for long vowels; executed by the fact check), and an entry with kanji through either field, such as a long proper noun or title. No real-world dictionary was measured for such rows.
• Effect. Silent and deterministic: every match of the row gets an empty or truncated value for the field that wrapped, and a garbage pronunciation when the reading wrapped, through getReading / getPronunciation and ReadingAttribute. JapaneseReadingFormFilter uses only the reading, so it emits a wrong term when the reading wrapped (JapaneseReadingFormFilter.java:63-95); JapaneseCompletionFilter also reads ReadingAttribute (read, not executed). In every executed case there was no error at build or analysis time (a misdecoded header could in principle read past the buffer end and throw; read, not observed); one row is enough.
• Why nobody noticed. Dictionary build tooling for Lucene filters rows by base-form length, because Lucene rejects long base forms; nothing filters readings. Two community walkthroughs of the format (a committer's 2019 slides and a 2019-2020 dictionary checker) list only the ids and the 16-char base form as size limits; the checker also flags an empty reading. No user report in 14 years.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => org.junit.ComparisonFailure: reading expected:<[カカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカカ]> but was:<[]>
       org.junit.Assert.assertEquals(Assert.java:117)
       org.apache.lucene.analysis.ja.dict.TestDictionaryBuilderLongReading.testReadingOf128Chars(TestDictionaryBuilderLongReading.java:50)

• Fail fast is the writer's pattern. putEntry throws IllegalArgumentException for empty POS fields (:72-74), an empty base form (:104-106), leftId != rightId (:117-119), leftId >= 8192 (:120-122), conflicting POS data (:130-133) and a stored base form of 16 or more chars (:139-141); an empty POS part is still a plain assert (:62). TestTokenInfoDictionary.testPutException (:91-105) asserts the pattern for the ids and the column count.
• LUCENE-8863 (#9906; in 8.2.0 through the branch_8x commit bdaf10ffa826, on main 4502065f0365, first released in 9.0.0) replaced assert baseForm.length() < 16, the id asserts and the conflicting-POS assert with IllegalArgumentException, and added the empty-POS and empty-base-form checks. In the thread @mocobeta wrote "+1 for changing assertions to \"real\" exceptions", and @rmuir, about an empty base form, "It sounds like a missing check in the code ... I see it as bug in your dictionary, we should fail :)". The reading length had no assert, so it was left out.
• The limit was known for the base form. The original import (cd372bdc8383, LUCENE-3305, 2012) stored the base form with the same 7-bit header and asserted baseForm.length() < 128; the reading and pronunciation lines next to it had no assert.
• Nothing documents a limit. Not DictionaryBuilder, JaMorphData (getReading: "Reading of the token"; getPronunciation: "Pronunciation of the token"), JapaneseTokenizer, the package docs, MIGRATE.md or CHANGES.txt; no test covers reading or pronunciation lengths.

The strongest case for "intended": DictionaryBuilder is @lucene.experimental and its input only "roughly" follows IPADIC (DictionaryBuilder.java:32-35); the compact format was designed around the bundled dictionary (@rmuir in #5128, about another assert: "an optimization that works for ipadic and naist-jdic, and I knew i was making an assumption doing it"; in #9906: "I don't think its good to try to support N different ways of doing things when only one is tested (the ipadic way)"); readings of 128 or more chars are far outside IPADIC (32 at most); and the dictionary is trusted input.

Why it fails: "experimental" allows the API to change, not silently wrong output; the "ipadic way" remark argues for rejecting input the format cannot hold, which the builder does for the base-form length and the ids; nothing documents a 127-char limit; trusted input rules out a security mark, not a correctness bug. The cross-model reviews rated the argument weak; the other reviewers likewise concluded that the behaviour is not intended or recommended rejecting such rows.

One fix was built and validated; the others are listed for the discussion with maintainers. A credibility check expects a committer to prefer rejecting the row over a format change.

Check inside the HAS_READING and HAS_PRONUNCIATION branches, where the base-form check also sits, so rows that store nothing (a katakana surface that is its own reading) still build. The patch also adds TestTokenInfoDictionary.testPutLongReadingOrPronunciation (26 lines): a 128-char katakana reading, a 128-char hiragana reading and a 128-char pronunciation are rejected; 127 chars round-trip; a 128-char katakana surface that is its own reading still builds.

if ((flags & TokenInfoMorphData.HAS_READING) != 0) {
  // the length is stored in the upper 7 bits of one byte
  if (reading.length() >= 128) {
    throw new IllegalArgumentException("Length of reading " + reading + " is >= 128");
  }
  ...
if ((flags & TokenInfoMorphData.HAS_PRONUNCIATION) != 0) {
  ...
  if (pronunciation.length() >= 128) {
    throw new IllegalArgumentException(
        "Length of pronunciation " + pronunciation + " is >= 128");
  }

Trade-offs

• A custom dictionary that builds today with such rows will fail to build, with a message that names the field and its value, as long base forms have since 8.2.0. Worth a CHANGES.txt line.
• The bundled dictionary cannot change: a stricter sketch (it threw for any reading or pronunciation of 128 or more chars, stored or not) rebuilt the bundled source with no exception and 9 byte-identical files, and this fix throws in a subset of those cases and writes the same bytes. Inferred, not re-run with this fix.

What the checks validated

• Scratch worktree at a9779ef3c193, gradle, seed DEADBEEF: the paste-ready test fails 2 of 4 on stock main, assertions on and off; with the patch the whole :lucene:analysis:kuromoji:test task passes, 31 classes, 200 tests, 0 failures (7 skipped), assertions on and off, counting the 4 paste-ready tests and the new TestTokenInfoDictionary test.
• ecjLintMain, ecjLintTest and checkGoogleJavaFormat for the module pass.

Store the length as a vInt or in two bytes, so long readings survive.

Trade-offs

A format change: the reader changes, a vInt header would take two bytes for readings of 64-127 chars, so the dictionary version needs a bump and custom dictionaries a rebuild. Not run.

@mocobeta suggested in #9906 an option to "ignore invalid entries and just output some WARNING then continue".

Trade-offs

Applies to every check, so it is a separate feature; the default should still fail. Not run.

The finder's first sketch checks every reading and pronunciation, stored or not.

Trade-offs

Rejects rows that work today, such as a 128-char katakana surface whose reading equals it (executed by the independent reproducer).

• Round trip. A hunt unit on compiled CJK dictionaries ran a bounded-exhaustive translation validation of kuromoji's bundled dictionary against its mecab-ipadic source: all 392,127 rows, 1,731,856 connection-cost cells, all 65,536 chars and every unknown-word class, with 0 mismatches. The longest source reading is 32 chars, so the bundled dictionary cannot show the bug.
• Signal. While checking each field's encoding, the finder noticed that the base form has an explicit limit with an exception and the reading and pronunciation lengths, packed into one byte, have none.
• Witness. The finder built a one-row custom dictionary through the public DictionaryBuilder and reduced it to a 3-test public-API JUnit (2 of 3 fail on main with assertions on and off, and on 10.5.2 and 9.12.3). No formal tool (model checker, SMT solver or proof) produced the signal.