diff --git a/.changeset/zero-byte-s3-create.md b/.changeset/zero-byte-s3-create.md new file mode 100644 index 00000000..bc4909d9 --- /dev/null +++ b/.changeset/zero-byte-s3-create.md @@ -0,0 +1,5 @@ +--- +"@tus/s3-store": patch +--- + +Complete empty multipart uploads in `create` when `Upload-Length` is 0 so the object exists before `onUploadFinish` runs. diff --git a/packages/s3-store/src/index.ts b/packages/s3-store/src/index.ts index 20482e77..ecf3b8c7 100644 --- a/packages/s3-store/src/index.ts +++ b/packages/s3-store/src/index.ts @@ -576,6 +576,13 @@ export class S3Store extends DataStore { * Creates a multipart upload on S3 attaching any metadata to it. * Also, a `${file_id}.info` file is created which holds some information * about the upload itself like: `upload-id`, `upload-length`, etc. + * + * When `Upload-Length` is 0, `@tus/server` treats the upload as immediately + * final and calls `onUploadFinish` without `write()` (unless using + * creation-with-upload). Complete the empty multipart here so the object + * exists before finish hooks run (`finishMultipartUpload` already uploads a + * zero-byte part when `parts` is empty). `write()` is idempotent if the + * multipart was already completed for that final offset. */ public async create(upload: Upload) { log(`[${upload.id}] initializing multipart upload`) @@ -604,6 +611,14 @@ export class S3Store extends DataStore { await this.saveMetadata(upload, res.UploadId as string) log(`[${upload.id}] multipart upload created (${res.UploadId})`) + if (upload.size === 0 && !upload.sizeIsDeferred) { + const metadata = await this.getMetadata(upload.id) + await this.finishMultipartUpload(metadata, []) + await this.completeMetadata(metadata.file) + await this.clearCache(upload.id) + log(`[${upload.id}] zero-byte multipart upload completed`) + } + return upload } @@ -621,7 +636,24 @@ export class S3Store extends DataStore { public async write(src: stream.Readable, id: string, offset: number): Promise { // Metadata request needs to happen first const metadata = await this.getMetadata(id) - const parts = await this.retrieveParts(id) + let parts: Array + try { + parts = await this.retrieveParts(id) + } catch (error) { + // create() may already have completed a zero-byte multipart upload. + // creation-with-upload still calls write() with an empty body; treat that + // as a no-op when the requested offset is already the final size. + if ( + isS3NotFoundError(error) && + metadata.file.size !== undefined && + offset === metadata.file.size + ) { + src.resume() + await streamProm.finished(src).catch(() => undefined) + return offset + } + throw error + } // biome-ignore lint/style/noNonNullAssertion: it's fine const partNumber: number = parts.length > 0 ? parts[parts.length - 1].PartNumber! : 0 const nextPartNumber = partNumber + 1 diff --git a/packages/s3-store/src/test/index.ts b/packages/s3-store/src/test/index.ts index 4efa6118..166977cb 100644 --- a/packages/s3-store/src/test/index.ts +++ b/packages/s3-store/src/test/index.ts @@ -1,14 +1,13 @@ import path from 'node:path' import assert from 'node:assert/strict' import {Readable} from 'node:stream' -import stream from 'node:stream/promises' import {NoSuchUpload} from '@aws-sdk/client-s3' import sinon from 'sinon' import {S3Store} from '@tus/s3-store' import * as shared from '../../../utils/dist/test/stores.js' -import {StreamLimiter, Upload} from '@tus/utils' +import {Upload} from '@tus/utils' const fixturesPath = path.resolve('../', '../', 'test', 'fixtures') const storePath = path.resolve('../', '../', 'test', 'output', 's3-store') @@ -254,17 +253,9 @@ describe('S3DataStore', () => { offset: 0, }) + // @tus/server marks size-0 uploads final on create and does not call write(). await store.create(upload) - const offset = await stream.pipeline( - Readable.from(Buffer.alloc(size)), - new StreamLimiter(999), - async (stream) => { - return store.write(stream as StreamLimiter, upload.id, upload.offset) - } - ) - assert.equal(offset, size, 'Write should return 0 offset') - // Check .info file via getUpload const finalUpload = await store.getUpload(upload.id) assert.equal(finalUpload.offset, size, '.info file should show 0 offset') @@ -283,6 +274,33 @@ describe('S3DataStore', () => { } }) + it('should allow creation-with-upload for a zero byte file', async function () { + const store = this.datastore as S3Store + const size = 0 + const upload = new Upload({ + id: shared.testId('zero-byte-creation-with-upload'), + size, + offset: 0, + }) + + await store.create(upload) + + // PostHandler still calls write() when Content-Type is application/offset+octet-stream + const offset = await store.write(Readable.from(Buffer.alloc(size)), upload.id, 0) + assert.equal(offset, size) + + const finalUpload = await store.getUpload(upload.id) + assert.equal(finalUpload.offset, size) + + // @ts-expect-error private + const s3Client = store.client + const headResult = await s3Client.getObject({ + Bucket: s3ClientConfig.bucket, + Key: upload.id, + }) + assert.equal(headResult.ContentLength, size) + }) + it('should report a missing multipart upload as a missing file when removing it', async function () { const store = this.datastore as S3Store const id = shared.testId('missing-multipart-upload')