harden proto storage — security, correctness, performance, tests - #89
Merged
Conversation
adds two security checks to prevent DoS through the proto API: - MAX_DESCRIPTOR_BYTES (10MB) rejects oversized descriptors before decoding in PROTO.REGISTER - MAX_FIELD_PATH_DEPTH (16) rejects deeply nested field paths in get_field, set_field, and clear_field includes unit tests for both limits plus edge cases (double-dot and trailing-dot paths).
replaces O(n) linear scan across all registered schemas with a HashMap<String, MessageDescriptor> that's populated during register() and restore(). common path (every get_field/set_field/validate call) now does a single hash lookup instead of iterating all pools.
adds an early return for the common case of reading a top-level field (e.g. "name") that borrows directly from the decoded message instead of cloning the entire DynamicMessage. mirrors the pattern already used in resolve_field_path_mut.
covers edge cases not previously tested: - set_field on nested paths (inner.value) - clear_field on nested paths - auto-initialization of intermediate messages on nested set - u64::MAX returns bulk string (too large for i64) - u64 that fits in i64 returns integer frame
proto_get now returns remaining TTL alongside type_name and data. the SETFIELD/DELFIELD handlers in both sharded and concurrent modes pass the existing TTL through to the write-back ProtoSet, so a key's expiry is no longer silently reset to no-expiry on field mutation.
adds sharded-mode tests verifying that SETFIELD and DELFIELD preserve the key's TTL instead of resetting it. also covers nested field paths (inner.value) for both set and del, and duplicate schema registration rejection. concurrent-mode tests cover nested paths but skip TTL verification since proto values route through engine shards while TTL checks the concurrent keyspace.
kacy
added a commit
that referenced
this pull request
Feb 11, 2026
* fix: add descriptor size and field path depth limits adds two security checks to prevent DoS through the proto API: - MAX_DESCRIPTOR_BYTES (10MB) rejects oversized descriptors before decoding in PROTO.REGISTER - MAX_FIELD_PATH_DEPTH (16) rejects deeply nested field paths in get_field, set_field, and clear_field includes unit tests for both limits plus edge cases (double-dot and trailing-dot paths). * perf: add message descriptor cache for O(1) find_message replaces O(n) linear scan across all registered schemas with a HashMap<String, MessageDescriptor> that's populated during register() and restore(). common path (every get_field/set_field/validate call) now does a single hash lookup instead of iterating all pools. * perf: optimize resolve_field_path for single-segment reads adds an early return for the common case of reading a top-level field (e.g. "name") that borrows directly from the decoded message instead of cloning the entire DynamicMessage. mirrors the pattern already used in resolve_field_path_mut. * test: add nested path and u64 overflow unit tests covers edge cases not previously tested: - set_field on nested paths (inner.value) - clear_field on nested paths - auto-initialization of intermediate messages on nested set - u64::MAX returns bulk string (too large for i64) - u64 that fits in i64 returns integer frame * fix: preserve TTL across SETFIELD and DELFIELD operations proto_get now returns remaining TTL alongside type_name and data. the SETFIELD/DELFIELD handlers in both sharded and concurrent modes pass the existing TTL through to the write-back ProtoSet, so a key's expiry is no longer silently reset to no-expiry on field mutation. * test: add integration tests for TTL preservation and nested paths adds sharded-mode tests verifying that SETFIELD and DELFIELD preserve the key's TTL instead of resetting it. also covers nested field paths (inner.value) for both set and del, and duplicate schema registration rejection. concurrent-mode tests cover nested paths but skip TTL verification since proto values route through engine shards while TTL checks the concurrent keyspace.
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
1 participant
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
what was tested
cargo test -p emberkv-core --features protobuf --lib— 332 tests pass (31 schema-specific)cargo test -p ember-protocol— 1 test passescargo test -p ember-integration-tests— 109 tests pass (40 proto-specific)cargo clippy --workspace --features protobuf -- -D warnings— cleancargo checkwithout protobuf feature — no regressionsdesign considerations
proto_getas aDurationand threads it through to the write-backProtoSet. this adds ~8 bytes to theProtoValueenum variant but avoids a second shard roundtrip to fetch TTL separately.HashMap<String, MessageDescriptor>onSchemaRegistry, populated duringregister()andrestore(). thepoolfield onRegisteredSchemais kept for test helpers that build dynamic messages directly.