Skip to content

fix(NATSRS-009): CU-86akbhh82 2 review findings in object_store.rs #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −5
Draft

fix(NATSRS-009): CU-86akbhh82 2 review findings in object_store.rs #137

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions nats/src/object_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,13 @@ impl ObjectStore {
/// ```
pub fn info(&self, object_name: &str) -> io::Result<ObjectInfo> {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 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

// LoOkup the stream to get the bound subject.
let object_name = encode_object_name(object_name);
if !is_valid_object_name(&object_name) {
if !is_valid_object_name(object_name) {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"invalid object name",
));
}
let object_name = encode_object_name(object_name);

// Grab last meta value we have.
let stream_name = format!("OBJ_{}", &self.name);
Expand Down Expand Up @@ -424,16 +424,16 @@ impl ObjectStore {
ObjectMeta: From<T>,
{
let object_meta: ObjectMeta = meta.into();

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 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

let object_name = encode_object_name(&object_meta.name);
if !is_valid_object_name(&object_name) {
if !is_valid_object_name(&object_meta.name) {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"invalid object name",
));
}
let object_name = encode_object_name(&object_meta.name);

// Fetch any existing object info, if there is any for later use.
let maybe_existing_object_info = match self.info(&object_name) {
let maybe_existing_object_info = match self.info(&object_meta.name) {
Ok(object_info) => Some(object_info),
Err(_) => None,
};
Expand Down
Loading