fix(attachments): Serialize ID without hyphens #5501
Conversation
jjbayer
changed the title
| } | ||
|
|
||
| #[derive(Clone, Default, PartialEq, Empty, FromValue, ProcessValue)] | ||
| pub struct AttachmentId(Uuid); |
There was a problem hiding this comment.
This was largely copied from TraceId.
| return { | ||
| "trace_id": uuid.uuid4().hex, | ||
| "attachment_id": str(uuid.uuid4()), | ||
| "attachment_id": uuid.uuid4().hex, |
There was a problem hiding this comment.
The input doesn't need to be hyphen-free, but it makes comparisons with the output easier.
| pytest.param( | ||
| {"meta_length": None}, | ||
| 273, | ||
| 269, |
There was a problem hiding this comment.
No more hyphens -> saved some chars.
| def create_attachment_envelope(project_config): | ||
| return Envelope( | ||
| headers={ | ||
| "event_id": "515539018c9b4260a6f999572f1661ee", |
There was a problem hiding this comment.
Drive-by fix: trace attachment envelopes should not have an event ID.
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| let key = Uuid::from_slice(&trace_item.trace_item.item_id) | ||
| .map_err(Error::from) | ||
| .reject(&trace_item)? | ||
| .as_simple() | ||
| .to_string(); | ||
|
|
||
| #[cfg(debug_assertions)] |
There was a problem hiding this comment.
Bug: The code attempts to deserialize a 32-byte hex string UUID using Uuid::from_slice(), which expects a 16-byte binary slice, causing attachment uploads to fail.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The serialization of attachment_id was changed to a 32-byte UTF-8 hex string, but the deserialization logic was not updated. In upload.rs, the code attempts to deserialize trace_item.trace_item.item_id using Uuid::from_slice(), which expects a 16-byte raw binary slice. This mismatch causes a slice length error. The error is caught and the item is rejected, causing the attachment upload to fail silently. As a result, trace attachments are never stored or forwarded.
💡 Suggested Fix
Update the deserialization logic in upload.rs. Instead of using Uuid::from_slice() on the byte representation of the string, parse the hex string to a UUID. This can be done by converting the item_id byte slice to a string and then using Uuid::parse_str().
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: relay-server/src/services/upload.rs#L336-L342
Potential issue: The serialization of `attachment_id` was changed to a 32-byte UTF-8 hex
string, but the deserialization logic was not updated. In `upload.rs`, the code attempts
to deserialize `trace_item.trace_item.item_id` using `Uuid::from_slice()`, which expects
a 16-byte raw binary slice. This mismatch causes a slice length error. The error is
caught and the item is rejected, causing the attachment upload to fail silently. As a
result, trace attachments are never stored or forwarded.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7755060
Serialize attachment IDs without hyphens. This is the Sentry way, and aligns better with Event IDs and Trace IDs.
This also means that objectstore keys are now hyphen-free, which would be a breaking change if this wasn't an experimental feature.