jerry-024 commented on PR #8074:
URL: https://github.com/apache/paimon/pull/8074#issuecomment-4601775856
## Review Summary
Overall well-designed PR — tokenizer names validated against explicit
allowlists at all three layers (Java, Rust, Python), good defense in depth.
### Must Fix
**JSON injection in JNI convenience constructors**
(`TantivyIndexWriter.java:42-52`, `TantivySearcher.java:147-159`)
The 6-arg `TantivyIndexWriter` constructor and
`TantivySearcher.defaultConfigJson()` build JSON via raw string concatenation
without escaping `tokenizerName`:
```java
"{\"tokenizer\":\"" + tokenizerName + "\",\"ngramMinGram\":" + ...
```
A crafted tokenizer name like
`simple","ngramMaxGram":999999,"tokenizer":"ngram` would inject arbitrary
fields into the JSON parsed by Rust's serde (last key wins by default),
bypassing Java-side validation entirely.
The main production path through
`TantivyFullTextIndexOptions.toNativeConfigJson()` properly escapes via
`appendJsonString()`, but these convenience constructors are public API and
bypass it.
**Suggested fix:** Route all JSON construction through
`toNativeConfigJson()`, or remove the convenience constructors and force
callers through `TantivyFullTextIndexOptions`.
### Nice to Have (not blocking)
1. **No upper bound on `ngramMaxGram` / `maxTokenLength`** — validated as `>
0` but no ceiling. `max-gram=1000000` would generate O(text_length × max_gram)
tokens. Consider adding caps (e.g., max_gram ≤ 10, max_token_length ≤ 1024).
2. **Cross-language tokenizer_name contract** — the `tokenizer_name()` /
`_needs_custom` logic is independently implemented in Rust and Python. They
agree today, but a comment documenting the contract would help prevent drift.
3. **Duplicated JSON construction** — `TantivyIndexWriter` and
`TantivySearcher` both manually build the same JSON template. Could be
consolidated into one place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
