Skip to content

Commit 5677e66

Browse files
committed
CLDSRV-863: emit trailer in TrailingChecksumTransform
1 parent 464f12d commit 5677e66

3 files changed

Lines changed: 72 additions & 17 deletions

File tree

lib/auth/streamingV4/trailingChecksumTransform.js

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ const { errors } = require('arsenal');
33
const { maximumAllowedPartSize } = require('../../../constants');
44

55
/**
6-
* This class is designed to handle the chunks sent in a streaming
7-
* unsigned playload trailer request. In this iteration, we are not checking
8-
* the checksums, but we are removing them from the stream.
9-
* S3C-9732 will deal with checksum verification.
6+
* This class handles the chunked-upload body format used by
7+
* STREAMING-UNSIGNED-PAYLOAD-TRAILER requests. It strips the chunk-size
8+
* headers and trailing checksum trailer from the stream, forwarding only
9+
* the raw object data. The trailer name and value are emitted via a
10+
* 'trailer' event so that ChecksumTransform can validate the checksum.
1011
*/
1112
class TrailingChecksumTransform extends Transform {
1213
/**
@@ -20,6 +21,10 @@ class TrailingChecksumTransform extends Transform {
2021
this.bytesToDiscard = 0; // when trailing \r\n are present, we discard them but they can be in different chunks
2122
this.bytesToRead = 0; // when a chunk is advertised, the size is put here and we forward all bytes
2223
this.streamClosed = false;
24+
this.readingTrailer = false;
25+
this.trailerBuffer = Buffer.alloc(0);
26+
this.trailerName = null;
27+
this.trailerValue = null;
2328
}
2429

2530
/**
@@ -30,9 +35,16 @@ class TrailingChecksumTransform extends Transform {
3035
* @return {function} executes callback with err if applicable
3136
*/
3237
_flush(callback) {
33-
if (!this.streamClosed) {
38+
if (!this.streamClosed && this.readingTrailer && this.trailerBuffer.length === 0) {
39+
// Nothing came after "0\r\n", don't fail.
40+
// If the x-amz-trailer header was present then the trailer is required and ChecksumTransform will fail.
41+
return callback();
42+
} else if (!this.streamClosed && this.readingTrailer && this.trailerBuffer.length !== 0) {
43+
this.log.error('stream ended without trailer "\r\n"');
44+
return callback(errors.IncompleteBody);
45+
} else if (!this.streamClosed && !this.readingTrailer) {
3446
this.log.error('stream ended without closing chunked encoding');
35-
return callback(errors.InvalidArgument);
47+
return callback(errors.IncompleteBody);
3648
}
3749
return callback();
3850
}
@@ -66,6 +78,49 @@ class TrailingChecksumTransform extends Transform {
6678
continue;
6779
}
6880

81+
// after the 0-size chunk, read the trailer line (e.g. "x-amz-checksum-crc32:YABb/g==")
82+
if (this.readingTrailer) {
83+
const combined = Buffer.concat([this.trailerBuffer, chunk]);
84+
const lineBreakIndex = combined.indexOf('\r\n');
85+
if (lineBreakIndex === -1) {
86+
if (combined.byteLength > 1024) {
87+
this.log.error('trailer line too long');
88+
return callback(errors.MalformedTrailerError);
89+
}
90+
// The trailer is not complete yet, continue.
91+
this.trailerBuffer = combined;
92+
return callback();
93+
}
94+
this.trailerBuffer = Buffer.alloc(0);
95+
const fullTrailer = combined.subarray(0, lineBreakIndex);
96+
if (fullTrailer.length === 0) {
97+
// The trailer is empty, stop reading.
98+
this.readingTrailer = false;
99+
this.streamClosed = true;
100+
return callback();
101+
}
102+
let trailerLine = fullTrailer.toString();
103+
// Some clients terminate the trailer with \n\r\n instead of
104+
// just \r\n, producing a trailing \n in the parsed line.
105+
if (trailerLine.endsWith('\n')) {
106+
trailerLine = trailerLine.slice(0, -1);
107+
}
108+
const colonIndex = trailerLine.indexOf(':');
109+
if (colonIndex > 0) {
110+
this.trailerName = trailerLine.slice(0, colonIndex);
111+
this.trailerValue = trailerLine.slice(colonIndex + 1);
112+
this.emit('trailer', this.trailerName, this.trailerValue);
113+
} else {
114+
this.log.error('incomplete trailer missing ":"', { trailerLine });
115+
return callback(errors.IncompleteBody);
116+
}
117+
this.readingTrailer = false;
118+
this.streamClosed = true;
119+
// The trailer \r\n is the last bytes of the stream per the AWS
120+
// chunked upload format, so any remaining bytes are discarded.
121+
return callback();
122+
}
123+
69124
// we are now looking for the chunk size field
70125
// no need to look further than 10 bytes since the field cannot be bigger: the max
71126
// chunk size is 5GB (see constants.maximumAllowedPartSize)
@@ -100,9 +155,9 @@ class TrailingChecksumTransform extends Transform {
100155
}
101156
this.chunkSizeBuffer = Buffer.alloc(0);
102157
if (dataSize === 0) {
103-
// TODO: check if the checksum is correct (S3C-9732)
104-
// last chunk, no more data to read, the stream is closed
105-
this.streamClosed = true;
158+
// last chunk, no more data to read; enter trailer-reading mode
159+
// bytesToDiscard = 2 below will consume the \r\n after "0"
160+
this.readingTrailer = true;
106161
}
107162
if (dataSize > maximumAllowedPartSize) {
108163
this.log.error('chunk size too big', { dataSize });

tests/functional/raw-node/test/trailingChecksums.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ const HttpRequestAuthV4 = require('../utils/HttpRequestAuthV4');
66
const bucket = 'testunsupportedchecksumsbucket';
77
const objectKey = 'key';
88
const objData = Buffer.alloc(1024, 'a');
9-
// note this is not the correct checksum in objDataWithTrailingChecksum
109
const objDataWithTrailingChecksum = '10\r\n0123456789abcdef\r\n' +
1110
'10\r\n0123456789abcdef\r\n' +
12-
'0\r\nx-amz-checksum-crc64nvme:YeIDuLa7tU0=\r\n';
11+
'0\r\nx-amz-checksum-crc64nvme:skQv82y5rgE=\r\n';
1312
const objDataWithoutTrailingChecksum = '0123456789abcdef0123456789abcdef';
1413

1514
const config = require('../../config.json');

tests/unit/auth/TrailingChecksumTransform.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const async = require('async');
44
const { Readable } = require('stream');
55

66
const TrailingChecksumTransform = require('../../../lib/auth/streamingV4/trailingChecksumTransform');
7-
const { stripTrailingChecksumStream } = require('../../../lib/api/apiUtils/object/prepareStream');
87
const { DummyRequestLogger } = require('../helpers');
98

109
const log = new DummyRequestLogger();
@@ -174,22 +173,24 @@ describe('TrailingChecksumTransform class', () => {
174173
it('should propagate _flush error via errCb when stream closes without chunked encoding', done => {
175174
const incompleteData = '10\r\n01234\r6789abcd\r\n\r\n';
176175
const source = new ChunkedReader([Buffer.from(incompleteData)]);
177-
source.headers = { 'x-amz-content-sha256': 'STREAMING-UNSIGNED-PAYLOAD-TRAILER' };
178-
const stream = stripTrailingChecksumStream(source, log, err => {
179-
assert.deepStrictEqual(err, errors.InvalidArgument);
176+
const stream = new TrailingChecksumTransform(log);
177+
stream.on('error', err => {
178+
assert.deepStrictEqual(err, errors.IncompleteBody);
180179
done();
181180
});
181+
source.pipe(stream);
182182
stream.resume();
183183
});
184184

185185
it('should propagate _transform error via errCb for invalid chunk size', done => {
186186
const badData = '500000000000\r\n';
187187
const source = new ChunkedReader([Buffer.from(badData)]);
188-
source.headers = { 'x-amz-content-sha256': 'STREAMING-UNSIGNED-PAYLOAD-TRAILER' };
189-
const stream = stripTrailingChecksumStream(source, log, err => {
188+
const stream = new TrailingChecksumTransform(log);
189+
stream.on('error', err => {
190190
assert.deepStrictEqual(err, errors.InvalidArgument);
191191
done();
192192
});
193+
source.pipe(stream);
193194
stream.resume();
194195
});
195196

0 commit comments

Comments
 (0)