Repository navigation
fix(rs-client): Validate resumable upload streams through EOF - #754
Conversation
Exercise sources that yield the declared length and then fail, through both stream and reader uploads, including empty uploads. Co-authored-by: Codex <noreply@openai.com>
HTTP can stop polling once Content-Length bytes have been sent, leaving source completion errors unobserved. Check the declared length and successful EOF before yielding the final chunk, and validate empty uploads before sending the request. Share the check between stream and reader uploads while preserving the buffered-body path. Co-authored-by: Codex <noreply@openai.com>
Semver Impact of This PRThis PR lands in these workspaces:
📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes
🤖 This preview updates automatically when you update the PR. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #754 +/- ##
==========================================
+ Coverage 91.80% 91.82% +0.02%
==========================================
Files 118 118
Lines 22469 22567 +98
==========================================
+ Hits 20628 20723 +95
- Misses 1841 1844 +3
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-authored-by: Codex <noreply@openai.com>
Keep each stream with its declared length and derive buffer lengths from their bytes. Move body construction and source validation into UploadChunk so the request builder no longer stores an independent length or selects validation by body type. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
|
Leaving the full review to @jan-auer, but wanted to call out the following: even with this fix, the server could persist a prefix of the body that has already been sent, which in the case you mentioned (a Gzip decoder that errors with a checksum failure) could likely contain corrupt data. |
jan-auer
left a comment
jan-auer
left a comment
There was a problem hiding this comment.
Note that when something like a checksum error occurs at the very end, the entire upload is still online and waiting for completion or cancellation. Whether the upload can be resumed remains with the caller, so there's nothing we can do in the client with this.
Co-authored-by: Codex <noreply@openai.com>
|
Yes, I agree that relay should handle the cleanup. |
* origin/main: feat(clients): Send Content-Length for uploads of known size (#751) fix(stresstest): Compare read payloads without buffering them in memory (#753) ci(release): Bump craft to 3.9.0 (#760) fix(rs-client): Validate resumable upload streams through EOF (#754) # Conflicts: # clients/rust/src/resumable.rs
Resumable uploads can return success after sending the declared byte count without observing a subsequent source error. For example, a gzip decoder can produce all expected bytes before reporting a checksum failure.
Validate the exact length and successful source EOF before sending the final chunk. Earlier chunks continue streaming immediately; zero-length sources are validated before the request. The same contract applies to
put_streamandput_read, while buffered uploads retain their existing path.The first commit adds the regression test and intentionally fails against the previous implementation. The second adds the fix and focused length and streaming tests.
Discovered in getsentry/relay#6572. Relay can remove its local workaround after consuming a fixed client release.
Refs FS-596
Changelog Entry
Reject resumable uploads with incomplete or invalid source streams.