diff --git a/src/internal/client.ts b/src/internal/client.ts index 823b6e7a..512a1963 100644 --- a/src/internal/client.ts +++ b/src/internal/client.ts @@ -1814,8 +1814,6 @@ export class TypedClient { } } - partNumber++ - // now start to upload missing part const options: RequestOption = { method: 'PUT', @@ -1838,6 +1836,8 @@ export class TypedClient { } eTags.push({ part: partNumber, etag }) + + partNumber++ } return await this.completeMultipartUpload(bucketName, objectName, uploadId, eTags) diff --git a/tests/unit/upload-stream-partnumber-test.js b/tests/unit/upload-stream-partnumber-test.js new file mode 100644 index 00000000..8825693b --- /dev/null +++ b/tests/unit/upload-stream-partnumber-test.js @@ -0,0 +1,121 @@ +/* + * MinIO Javascript Library for Amazon S3 Compatible Cloud Storage, (C) 2015 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as Crypto from 'node:crypto' +import * as Stream from 'node:stream' + +import { assert } from 'chai' + +import * as Minio from '../../src/minio.ts' + +// Regression tests for #1482: uploadStream must assign multipart part numbers +// consistently between the resume-skip branch and the upload branch. It used to +// increment partNumber *before* uploading, so a fresh upload started at part 2 +// and a resumed upload numbered re-uploaded chunks off-by-one relative to the +// parts it skipped, corrupting the completed multipart upload. +describe('uploadStream multipart part numbering (#1482)', () => { + const partSize = 64 + + // A byte-mode stream that emits `count` chunks of `partSize` bytes, one per + // event-loop tick, so BlockStream2 yields them individually to uploadStream. + function bodyStream(count) { + const readable = new Stream.Readable({ read() {} }) + ;(async () => { + for (let i = 0; i < count; i++) { + readable.push(Buffer.alloc(partSize, 'x')) + await new Promise(setImmediate) + } + readable.push(null) + })() + return readable + } + + function makeClient() { + return new Minio.Client({ + endPoint: 'localhost', + port: 9000, + useSSL: false, + accessKey: 'accesskey', + secretKey: 'secretkey', + }) + } + + // Stub the network-touching internals so we can observe the part numbers + // uploadStream assigns without talking to a server. + function instrument(client, oldParts = []) { + const record = { sentPartNumbers: [], completedParts: null } + client.findUploadId = async () => (oldParts.length ? 'existing-upload-id' : undefined) + client.initiateNewMultipartUpload = async () => 'new-upload-id' + client.listParts = async () => oldParts + client.makeRequestAsyncOmit = async (options) => { + const partNumber = Number(new URLSearchParams(options.query).get('partNumber')) + record.sentPartNumbers.push(partNumber) + return { headers: { etag: `"etag-${partNumber}"` } } + } + client.completeMultipartUpload = async (bucketName, objectName, uploadId, eTags) => { + record.completedParts = eTags.map((e) => e.part) + return { etag: 'final-etag' } + } + return record + } + + it('numbers a fresh multipart upload starting at part 1', async () => { + const client = makeClient() + const record = instrument(client) + + await client.uploadStream('bucket', 'object', {}, bodyStream(3), partSize) + + assert.deepEqual(record.sentPartNumbers, [1, 2, 3]) + assert.deepEqual(record.completedParts, [1, 2, 3]) + }) + + it('numbers re-uploaded parts consistently with skipped parts when resuming', async () => { + const client = makeClient() + + // A prior upload already stored the first two (identical) chunks as parts 1 and 2. + const chunkETag = Crypto.createHash('md5').update(Buffer.alloc(partSize, 'x')).digest('hex') + const oldParts = [ + { part: 1, etag: chunkETag }, + { part: 2, etag: chunkETag }, + ] + const record = instrument(client, oldParts) + + await client.uploadStream('bucket', 'object', {}, bodyStream(3), partSize) + + // parts 1 and 2 are skipped; only the third chunk is uploaded, as part 3. + assert.deepEqual(record.sentPartNumbers, [3]) + assert.deepEqual(record.completedParts, [1, 2, 3]) + }) + + it('re-uploads a mismatched old part at its own part number', async () => { + const client = makeClient() + + // Part 1 still matches, but part 2's stored etag is stale, so the second + // chunk must be re-uploaded — at part 2, not shifted to part 3. + const matchingETag = Crypto.createHash('md5').update(Buffer.alloc(partSize, 'x')).digest('hex') + const oldParts = [ + { part: 1, etag: matchingETag }, + { part: 2, etag: 'stale-etag' }, + ] + const record = instrument(client, oldParts) + + await client.uploadStream('bucket', 'object', {}, bodyStream(3), partSize) + + // part 1 skipped; part 2 re-uploaded at part 2 (not 3); part 3 uploaded. + assert.deepEqual(record.sentPartNumbers, [2, 3]) + assert.deepEqual(record.completedParts, [1, 2, 3]) + }) +})