Repository navigation
Upgrade vendored darts-clone to v0.32h and harden OCD dictionary loading - #1372
Merged
Merged
Conversation
Add the upstream v0.32h header and BSD license under a separate dependency directory without changing the active build configuration.
Use OpenCC's existing size_t id_type for the vendored v0.32h header so the serialized Darts array layout stays compatible with the current OCD format.
Point Bazel, CMake, Windows CLI Zig builds, and npm packaging at the compatible vendored v0.32h dependency. Add local Bazel module metadata so @darts-clone resolves from the repository copy.
Delete the unused darts-clone 0.32 header after all build and package paths were switched to the compatible v0.32h vendor directory.
Apply the darts.h validation hardening from google/sentencepiece@d685ef31 and wire it into OpenCC's OCD loader. Validate serialized Darts arrays for unit alignment, root/offset bounds, and lexicon value bounds with regression coverage for malformed dictionaries.
Owner
Collaborator
Author
|
You can upstream this updated version immediately in bcr.1: |
frankslin
added a commit
that referenced
this pull request
Jul 9, 2026
…t OCD output (#1414) OCD files embed a BinaryDict section whose integer fields were written as native size_t, so 32-bit and 64-bit builds produced different bytes for the same dictionary, and files did not load across word sizes. All fields are now written as uint32_t (matching what 32-bit builds already produced), and the deserializer detects the legacy 8-byte layout, so files written by old 64-bit builds still load, on any platform. Investigation: - PR #1372/#1373 made the darts trie section word-size independent: vendored darts.h id_type was fixed to uint32_t, the dartsSize field became uint32_t, and a legacy 64-bit reader with validation was added. Those changes covered only the darts array; the trailing BinaryDict section (key/value buffers plus offset tables) kept writing native size_t fields, a layout that dates back to 641bfe9 which deliberately chose size_t to silence MSVC x64 warnings. - As a result, .ocd files produced on 32-bit platforms (including wasm32 and armv7) differed byte-wise from 64-bit output for the same input, and could not be loaded across word sizes at all: BinaryDict::NewFromFile read sizeof(size_t) fields of whatever platform was running. - The existing legacy-format tests could not catch this because SerializeAsLegacy64 copied the BinaryDict section verbatim. On a 64-bit host the fixture carried 8-byte fields and the reader consumed 8-byte fields, so the test was self-consistent yet word-size dependent; the same fixture would have failed on a 32-bit host. - Layout detection reuses the high-bytes-zero probe already proven for dartsSize: an 8-byte numItems always has zero bytes at [4..7] for any realistic dictionary, while the fixed-width layout places keyTotalLength there, which is non-zero whenever the dictionary has entries. The only all-zero-probe fixed-width file is an empty dictionary, exactly 12 bytes long, which is disambiguated by size. - Endianness is intentionally out of scope: the whole OCD/OCD2 family is native-endian (darts array, marisa trie, SerializedValues), so this change keeps BinaryDict consistent with the rest of the format on big-endian platforms rather than fixing one section in isolation. Verification: - bazel test //src/... : all 18 targets pass, including the new binary_dict_test cases (fixed-width layout size assertion, empty-dict round trip, legacy 64-bit loading, out-of-bounds rejection in both layouts) and darts_dict_test with the widened legacy fixture. - dict_converter round trip text -> ocd -> text reproduces the input, and a hex dump of the generated .ocd confirms 4-byte numItems/keyTotalLength/ valueTotalLength and per-entry fields after the darts section. Detailed Changes: - **src/BinaryDict.cpp**: - Write all integer fields as uint32_t; reject values above UINT32_MAX. - Detect the field width on load via DetectFieldWidth and parse both layouts with a single bounds-checked reader. - Route NewFromFile through NewFromBuffer to share the detection logic. - **src/DartsDict.cpp**: - Update layout-detection comments; no behavior change. - **Tests**: - Make crafted BinaryDict files explicit about field width instead of depending on the host size_t, and cover both layouts. - Assert the serialized layout size is fully determined by content. - Cover the empty-dictionary round trip and legacy 64-bit loading. - Widen the BinaryDict tail in SerializeAsLegacy64 so legacy OCD fixtures match real files from old 64-bit builds end to end.
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.
Summary
deps/darts-clone-0.32h/and adapt itsid_typetosize_tso the serialized OCD array layout stays compatiblewith the existing format.
vendor directory; remove the old vendored header.
DartsDictloading with three validation checks ported fromgoogle/sentencepiece@d685ef31: unit-size alignment of the serialized array,
root/offset bounds via
doubleArray->validate(), and lexicon value bounds.Malformed
.ocdfiles now throwInvalidFormatinstead of silentlyproducing undefined behavior.
NOTE: The .ocd format has always been platform-dependent — id_type is size_t,
so files generated on 64-bit and 32-bit builds are mutually incompatible. This is a
pre-existing limitation predating this PR; .ocd2 (marisa-trie, the default format) is
unaffected.
Test plan
DartsDictTestandConfigTestsuites pass.DartsDictTestcases cover the added rejection paths:RejectsMisalignedDartsSize— array size not a multiple of unit sizeRejectsInvalidDartsRoot— corrupted root unitRejectsInvalidDartsValue— out-of-bounds value in a leaf unitbazel build //src:opencc_lib //src:opencc-DENABLE_DARTS=ON