feat: Add filename field to Metadata#517
Conversation
Add an optional filename field to Metadata, transmitted via the x-sn-filename header. When present, the server emits a Content-Disposition: attachment; filename="<filename>" header on GET and HEAD responses for browser/download-tool compatibility. - Add filename field to Metadata struct with serde and header support - Map to GCS native contentDisposition field - Emit Content-Disposition on GET, HEAD, and batch HEAD responses - Add .filename() builder methods to Rust client PutBuilder and InitiateMultipartBuilder - Add filename parameter to Python client put() and initiate_multipart_upload() Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Adding the filename field to Metadata pushed the Insert variant past clippy's large_enum_variant threshold (232 bytes vs 24 for the next largest). Box it to keep the enum small on the stack. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
The filename escaping only escaped double quotes but not backslashes, allowing a stored filename with a backslash-quote sequence to terminate the quoted-string and inject extra Content-Disposition parameters. Centralize format_content_disposition and parse_content_disposition in objectstore-types so all call sites share correct escaping of both backslash and double-quote characters (in the right order). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Content-Disposition is used by the multipart framing itself to delimit parts, so setting it on individual batch response parts would conflict. Clients should read the filename from the x-sn-filename metadata header instead, which to_headers() already emits. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Add filename assertions to existing e2e tests in both clients to verify the field roundtrips through PUT and GET (including batch GET in the Rust client). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
lcian
changed the title
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0699e52. Configure here.
| session = client.session(test_usecase, org=42, project=1337) | ||
|
|
||
| object_key = session.put(b"test data", origin="203.0.113.42") | ||
| object_key = session.put(b"test data", origin="203.0.113.42", filename="report.pdf") |
There was a problem hiding this comment.
could we add tests for header injection, i.e.:
- backslash and double quotes in the filename: need escaping in quoted headers (which we use)
- forward slash in the filename: would create a path and we likely don't want that
.and..as filename: as above- control characters (0x00–0x1F, 0x7F) in the filename: not permitted in header values by 7230
i currently can't think of other things we should reject.
same for rust client. i think we should sanitize this in the client and validate server-side.
There was a problem hiding this comment.
Updated the approach as we discussed offline, and added the requested tests.
Control characters are structurally impossible because the value itself needs to be turned into HeaderValue for transmission, both from the client to the server, as well as the other way around, so the respective party will just fail serializing.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7f0a254. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
2 participants
For Debug Files, the current Sentry endpoint returns a
Content-Disposition: attachment; filename="<filename>"header, which prompts browsers to save the file under the given name.Objectstore doesn't currently return this, so if we want downloads from e.g. the UI to go directly to Objectstore, the user will likely be prompted with the Objectstore key as the filename, which doesn't necessarily match the logical filename we show e.g. in the UI itself.
This introduces a first-class
Metadatafield to the server and clients, which is send by clients asx-sn-filenameand rendered in GET/HEAD responses via such aContent-Dispositionheader.This field can be an arbitrary
String.When deserializing it into a
Content-Disposition, we perform escaping and normalization of path separators and filenames containing only dots.Close FS-134