fix(NATSRS-009): CU-86akbhh82 2 review findings in object_store.rs - #137
flamingo[bot] wants to merge 1 commit into
Conversation
| @@ -365,13 +365,13 @@ impl ObjectStore { | |||
| /// ``` | |||
| pub fn info(&self, object_name: &str) -> io::Result<ObjectInfo> { | |||
There was a problem hiding this comment.
🦩 🟠 Regex compiled correctly via lazy_static, but is_valid_bucket_name double work with encode_object_name ordering bug
In ObjectStore::info, reordered validation so is_valid_object_name is called on the raw object_name parameter before it is shadowed/reassigned by encode_object_name(object_name). The base64 encoding now happens only after validation succeeds, so the regex validates caller-supplied input instead of the encoded string.
🤖 Prompt for AI agents
In nats/src/object_store.rs around line 366, review and complete this code-review fix: Regex compiled correctly via lazy_static, but is_valid_bucket_name double work with encode_object_name ordering bug.
What the draft fix changed: In `ObjectStore::info`, reordered validation so `is_valid_object_name` is called on the raw `object_name` parameter before it is shadowed/reassigned by `encode_object_name(object_name)`. The base64 encoding now happens only after validation succeeds, so the regex validates caller-supplied input instead of the encoded string.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer
| @@ -424,16 +424,16 @@ impl ObjectStore { | |||
| ObjectMeta: From<T>, | |||
| { | |||
| let object_meta: ObjectMeta = meta.into(); | |||
There was a problem hiding this comment.
🦩 🟠 put() also validates the base64-encoded name instead of the original object name
In ObjectStore::put, reordered validation to call is_valid_object_name(&object_meta.name) (the original, un-encoded name) before computing encode_object_name(&object_meta.name). Also updated the internal self.info(&object_name) call (which previously passed the already base64-encoded name into info, which then double-encoded it) to self.info(&object_meta.name), since info now expects and encodes the raw name itself — this was necessary to keep behavior correct given the info fix in finding 1, otherwise put would have looked up a doubly-encoded, nonexistent subject when checking for an existing object.
🤖 Prompt for AI agents
In nats/src/object_store.rs around line 426, review and complete this code-review fix: put() also validates the base64-encoded name instead of the original object name.
What the draft fix changed: In `ObjectStore::put`, reordered validation to call `is_valid_object_name(&object_meta.name)` (the original, un-encoded name) before computing `encode_object_name(&object_meta.name)`. Also updated the internal `self.info(&object_name)` call (which previously passed the already base64-encoded name into `info`, which then double-encoded it) to `self.info(&object_meta.name)`, since `info` now expects and encodes the raw name itself — this was necessary to keep behavior correct given the `info` fix in finding 1, otherwise `put` would have looked up a doubly-encoded, nonexistent subject when checking for an existing object.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 80 medium — react 👍/👎 to teach the reviewer
Closes 2 review findings in
nats/src/object_store.rs.Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
nats/src/object_store.rs:366nats/src/object_store.rs:426What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
4fa2224e-8528-443d-a51a-823c78308977Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akbhh82 NATS async-nats and CI review findings (9 PRs)