Support streaming uploads on the Veeam SOSAPI route - #6236
Conversation
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
| write(chunk, _enc, cb) { | ||
| totalLength += chunk.length; | ||
| if (totalLength > parsedContentLength) { | ||
| log.error('data stream exceed announced size', { parsedContentLength, overflow: totalLength }); | ||
| return cb( | ||
| errorInstances.InvalidRequest.customizeDescription( | ||
| 'request body exceeds the announced content-length', | ||
| ), | ||
| ); | ||
| } | ||
| return resolve(result); | ||
| }); | ||
| let totalLength = 0; | ||
| const chunks = []; | ||
| const collector = new Writable({ | ||
| write(chunk, _enc, cb) { | ||
| totalLength += chunk.length; | ||
| if (totalLength > parsedContentLength) { | ||
| log.error('data stream exceed announced size', { parsedContentLength, overflow: totalLength }); | ||
| return cb(errors.InternalError); | ||
| } | ||
| chunks.push(chunk); | ||
| return cb(); | ||
| }, | ||
| final(cb) { | ||
| settle(null, Buffer.concat(chunks).toString()); | ||
| cb(); | ||
| }, | ||
| }); | ||
| const dataStream = prepareStream(request, request.streamingV4Params, log, settle); | ||
| pipeline(dataStream, collector).catch(err => settle(err)); | ||
| chunks.push(chunk); | ||
| return cb(); | ||
| }, |
There was a problem hiding this comment.
The cb here is the Node.js stream.Writable API contract: a write(chunk, encoding, callback) implementation must invoke the callback to signal completion and backpressure — Node streams have no async/await form of this hook (promise support only covers pipeline/finished, which this function already awaits). Same pattern as every Writable/Transform implementation in the codebase (e.g. the object data path in storeObject). Not refactorable — suggest dismissing as won't-fix.
4a95275 to
1afcfa8
Compare
benzekrimaha
left a comment
There was a problem hiding this comment.
Nothing blocking on my side just tiny nits
The route prepared its request stream with arsenal's
s3middleware/prepareStream, which only decodes
STREAMING-AWS4-HMAC-SHA256-PAYLOAD — and was called with misaligned
arguments (vault receiving the logger, log the callback), breaking that
case too. Unsigned streaming uploads with a trailing checksum
(STREAMING-UNSIGNED-PAYLOAD-TRAILER), the default for aws CLI >= 2.23
and current AWS SDKs, went through undecoded: the aws-chunked framing
exceeded the announced decoded length and the route replied 500
InternalError ("data stream exceed announced size").
Switch receiveData() to the checksum-aware object-path prepareStream():
both streaming encodings are now decoded, and x-amz-checksum-* header
and trailing checksums are validated with the same semantics as
PutObject. Also return InvalidRequest (400) instead of InternalError
when the body exceeds the announced content-length, as this indicates
a malformed client request.
Issue: CLDSRV-962
Replace the promise-executor and .then() bridge with await on the pipeline. Transform errors delivered through the errCb side-channel are bridged into a promise raced against the pipeline completion; promise settlement semantics replace the jsutil.once guard. Issue: CLDSRV-962
51c28af to
f5538f6
Compare
|
/approve |
Build failedThe build for commit did not succeed in branch bugfix/CLDSRV-962/veeam-route-streaming-uploads The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
This pull request did not target the following hotfix branch(es) so they
Please check the status of the associated issue CLDSRV-962. Goodbye delthas. The following options are set: approve |
The Veeam SOSAPI route prepared its request stream with arsenal's
s3middleware/prepareStream, which only decodesSTREAMING-AWS4-HMAC-SHA256-PAYLOAD— and was called with misaligned arguments (vaultreceiving the logger,logthe callback), breaking that case too.Unsigned streaming uploads with a trailing checksum (
STREAMING-UNSIGNED-PAYLOAD-TRAILER, the default for aws CLI ≥ 2.23 and current AWS SDKs) went through undecoded: the aws-chunked framing exceeded the announced decoded length and the route replied 500InternalError(data stream exceed announced size), so any modern client PUTtingcapacity.xml/system.xmlfailed.This switches
receiveData()to the checksum-aware object-pathprepareStream():x-amz-checksum-*header and trailing checksums are validated with the same semantics as PutObject (BadDigeston mismatch);InvalidRequest(400) instead ofInternalError, as this indicates a malformed client request.The Veeam route was the last consumer of the arsenal helper in CloudServer.
Issue: CLDSRV-962