Validate TemplateProcessing on deserialization (fix panic on crafted tokenizer.json)#2152
Open
v-code01 wants to merge 1 commit into
Open
Validate TemplateProcessing on deserialization (fix panic on crafted tokenizer.json)#2152v-code01 wants to merge 1 commit into
v-code01 wants to merge 1 commit into
Conversation
A `TemplateProcessing` deserialized from a `tokenizer.json` bypassed the
builder's `validate` step: serde routed through `From<TemplateProcessingDeserializer>`,
which constructed the struct directly without checking that every special
token referenced by the `single`/`pair` templates is present in
`special_tokens`.
The builder enforces this invariant precisely because `apply_template`
indexes `self.special_tokens.0[id]` on the assumption it always holds
("We already checked existence above"). A crafted config whose template
references, say, `[CLS]` while leaving `special_tokens` empty therefore
sails through loading and panics at encode time on the missing key.
Switch the serde bridge from `from` to `try_from` and route through
`TemplateProcessingBuilder`, so the same validation that guards the
programmatic path also guards the deserialization path. Invalid configs
now surface a clean `Err` at load instead of a panic at encode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
TemplateProcessingdeserialized from atokenizer.jsonskips the builder's validation. serde routes throughFrom<TemplateProcessingDeserializer>, which builds the struct directly without verifying that every special token referenced by thesingle/pairtemplates actually appears inspecial_tokens.That invariant is not optional —
apply_templateindexes the special-tokens map directly:The "already checked" only holds on the programmatic path (
TemplateProcessingBuilder::build→validate). On the deserialization path it does not, so a crafted config whose template references a token missing fromspecial_tokensloads fine and then panics at encode time on the missing map key.Repro
A
tokenizer.jsonpost-processor like:{ "type": "TemplateProcessing", "single": [{ "SpecialToken": { "id": "[CLS]", "type_id": 0 } }], "pair": [], "special_tokens": {} }deserializes without error, then panics inside
apply_templateon the firstencode.Fix
Switch the serde bridge from
fromtotry_fromand route throughTemplateProcessingBuilder, so the samevalidatethat guards the programmatic path also guards deserialization. Invalid configs now return a cleanErrat load time instead of panicking later.Tests
Added
deserialize_missing_special_tokens_is_rejected: a template referencing[CLS]with emptyspecial_tokensnow returnsErrfromserde_json::from_strinstead of panicking on encode. Existingtemplate_processing_serde(valid round-trip) andmissing_special_tokensstill pass. Fulltokenizerslib suite green (203 passed);cargo fmt/clippyclean on the changed file.cc @ArthurZucker @McPatate