diff --git a/fluxer_api/src/api/infrastructure/IStorageService.ts b/fluxer_api/src/api/infrastructure/IStorageService.ts index 656296de1..09408ef8a 100644 --- a/fluxer_api/src/api/infrastructure/IStorageService.ts +++ b/fluxer_api/src/api/infrastructure/IStorageService.ts @@ -71,9 +71,6 @@ export interface IStorageService { contentLength: number; contentRange?: string | null; contentType?: string | null; - cacheControl?: string | null; - contentDisposition?: string | null; - expires?: Date | null; etag?: string | null; lastModified?: Date | null; } | null>; diff --git a/fluxer_api/src/api/infrastructure/StorageService.ts b/fluxer_api/src/api/infrastructure/StorageService.ts index 2c94cfd1f..e55a744c7 100644 --- a/fluxer_api/src/api/infrastructure/StorageService.ts +++ b/fluxer_api/src/api/infrastructure/StorageService.ts @@ -107,6 +107,7 @@ function extractStreamFromGet(out: GetObjectCommandOutput): Readable { } const REJECTED_SERVER_SIDE_COPY_ERRORS = new Set([ + 'NoSuchBucket', 'NoSuchKey', 'NotFound', 'NotImplemented', @@ -239,6 +240,8 @@ export class StorageService implements IStorageService { contentType, contentLength, expiresAt, + cacheControl, + contentDisposition, }: { bucket: string; key: string; @@ -246,6 +249,8 @@ export class StorageService implements IStorageService { contentType?: string; contentLength?: number; expiresAt?: Date; + cacheControl?: string; + contentDisposition?: string; }): Promise { const size = contentLength ?? (await fs.promises.stat(filePath)).size; if (size <= STREAM_UPLOAD_PART_BYTES) { @@ -258,6 +263,8 @@ export class StorageService implements IStorageService { ContentType: contentType, ContentLength: size, Expires: expiresAt, + CacheControl: cacheControl, + ContentDisposition: contentDisposition, }), ); return; @@ -278,6 +285,8 @@ export class StorageService implements IStorageService { Body: stream, ContentType: contentType, Expires: expiresAt, + CacheControl: cacheControl, + ContentDisposition: contentDisposition, }, partSize: STREAM_UPLOAD_PART_BYTES, queueSize: STREAM_UPLOAD_CONCURRENCY, @@ -426,9 +435,6 @@ export class StorageService implements IStorageService { contentLength: number; contentRange?: string | null; contentType?: string | null; - cacheControl?: string | null; - contentDisposition?: string | null; - expires?: Date | null; etag?: string | null; lastModified?: Date | null; } | null> { @@ -441,9 +447,6 @@ export class StorageService implements IStorageService { contentLength: out.ContentLength ?? 0, contentRange: out.ContentRange ?? null, contentType: out.ContentType ?? null, - cacheControl: out.CacheControl ?? null, - contentDisposition: out.ContentDisposition ?? null, - expires: out.Expires ?? null, etag: out.ETag ?? null, lastModified: out.LastModified ?? null, }; @@ -468,6 +471,10 @@ export class StorageService implements IStorageService { async writeObjectToDisk(bucket: string, key: string, filePath: string): Promise { await this.ensureDirectoryExists(path.dirname(filePath)); + await this.downloadObject(bucket, key, filePath); + } + + private async downloadObject(bucket: string, key: string, filePath: string): Promise { const out = await this.client.send(new GetObjectCommand({Bucket: bucket, Key: key})); const stream = extractStreamFromGet(out); const writeStream = fs.createWriteStream(filePath); @@ -477,6 +484,7 @@ export class StorageService implements IStorageService { writeStream.destroy(); throw error; } + return out; } async copyObject({ @@ -507,7 +515,7 @@ export class StorageService implements IStorageService { }), ); } catch (copyError) { - if (sourceBucket === destinationBucket || !isRejectedServerSideCopy(copyError)) { + if (!isRejectedServerSideCopy(copyError)) { throw copyError; } await this.copyObjectThroughApi( @@ -541,38 +549,28 @@ export class StorageService implements IStorageService { }, copyError: unknown, ): Promise { - const source = await this.streamObject({bucket: sourceBucket, key: sourceKey}); - if (!source) { - throw copyError; - } - Logger.warn( - {sourceBucket, destinationBucket, error: copyError}, - 'Object storage rejected a cross-bucket copy, copying through the API instead', - ); - const upload = new Upload({ - client: this.client, - params: { - Bucket: destinationBucket, - Key: destinationKey, - Body: source.body, - ContentType: newContentType ?? source.contentType ?? undefined, + const downloadedSource = temporaryFile(); + try { + const source = await this.downloadObject(sourceBucket, sourceKey, downloadedSource); + Logger.warn( + {sourceBucket, destinationBucket, error: copyError}, + 'Object storage rejected a server-side copy, copying through the API instead', + ); + await this.uploadObjectFromFile({ + bucket: destinationBucket, + key: destinationKey, + filePath: downloadedSource, + contentType: newContentType ?? source.ContentType, ...(newContentType ? {} : { - CacheControl: source.cacheControl ?? undefined, - ContentDisposition: source.contentDisposition ?? undefined, - Expires: source.expires ?? undefined, + cacheControl: source.CacheControl, + contentDisposition: source.ContentDisposition, + expiresAt: source.Expires, }), - }, - partSize: STREAM_UPLOAD_PART_BYTES, - queueSize: STREAM_UPLOAD_CONCURRENCY, - leavePartsOnError: false, - }); - try { - await upload.done(); - } catch (error) { - source.body.destroy(); - throw error; + }); + } finally { + await fs.promises.unlink(downloadedSource).catch(() => undefined); } } diff --git a/fluxer_api/src/api/infrastructure/StorageServiceCrossBucketCopy.test.ts b/fluxer_api/src/api/infrastructure/StorageServiceCrossBucketCopy.test.ts index 740798af8..6629eec80 100644 --- a/fluxer_api/src/api/infrastructure/StorageServiceCrossBucketCopy.test.ts +++ b/fluxer_api/src/api/infrastructure/StorageServiceCrossBucketCopy.test.ts @@ -187,7 +187,7 @@ describe('StorageService copies on providers that reject cross-bucket CopyObject expect(ceph.objects.has(`${CDN}/attachments/1/4/missing.pdf`)).toBe(false); }); - it('does not retry a failed same-bucket copy through the API', async () => { + it('reports a missing same-bucket source as a missing object', async () => { await expect( storage().copyObject({ sourceBucket: CDN, diff --git a/fluxer_api/src/api/infrastructure/StorageServiceEndpointPath.test.ts b/fluxer_api/src/api/infrastructure/StorageServiceEndpointPath.test.ts index 0749db71b..cb3796fe0 100644 --- a/fluxer_api/src/api/infrastructure/StorageServiceEndpointPath.test.ts +++ b/fluxer_api/src/api/infrastructure/StorageServiceEndpointPath.test.ts @@ -20,7 +20,7 @@ function splitBucket(path: string): {bucket: string; key: string} { return {bucket, key: key.join('/')}; } -function singleBucketStore() { +function singleBucketStore(unresolvedSourceCode = 'NoSuchBucket') { const objects = new Map(); const resolve = (url: URL): {bucket: string; key: string} => { const path = decodeURIComponent(url.pathname.slice(1)); @@ -58,7 +58,7 @@ function singleBucketStore() { } const source = splitBucket(decodeURIComponent(copySource.replace(/^\//u, ''))); if (source.bucket !== REAL_BUCKET) { - return errorResponse('NoSuchBucket', 404); + return errorResponse(unresolvedSourceCode, unresolvedSourceCode === 'AccessDenied' ? 403 : 404); } const object = objects.get(source.key); if (!object) { @@ -127,3 +127,59 @@ describe('StorageService with every bucket inside one real bucket', () => { await expect(service.readObject(REAL_BUCKET, 'attachments/1/3/file.pdf')).resolves.toEqual(PDF); }); }); + +describe.each(['NoSuchBucket', 'AccessDenied'])( + 'StorageService with the bucket in the endpoint hostname, store answers %s', + (code) => { + let objects: ReturnType; + + beforeEach(() => { + objects = singleBucketStore(code); + }); + + it('copies a non-media attachment out of the uploads prefix', async () => { + const service = storage(`https://${REAL_BUCKET}.${HOST}`, true); + await service.uploadObject({bucket: 'fluxer-uploads', key: 'upload 1', body: PDF}); + + await expect( + service.copyObjectWithMetadataStripping({ + sourceBucket: 'fluxer-uploads', + sourceKey: 'upload 1', + destinationBucket: 'fluxer', + destinationKey: 'attachments/1/2/file.pdf', + contentType: 'application/pdf', + }), + ).resolves.toBeNull(); + + expect(objects.get('fluxer/attachments/1/2/file.pdf')).toEqual({body: PDF, contentType: 'application/pdf'}); + }); + + it('copies within the cdn prefix', async () => { + const service = storage(`https://${REAL_BUCKET}.${HOST}`, true); + await service.uploadObject({bucket: 'fluxer', key: 'emojis/1', body: PDF, contentType: 'image/png'}); + + await service.copyObject({ + sourceBucket: 'fluxer', + sourceKey: 'emojis/1', + destinationBucket: 'fluxer', + destinationKey: 'emojis/2', + }); + + expect(objects.get('fluxer/emojis/2')).toEqual({body: PDF, contentType: 'image/png'}); + }); + + it('reports a missing source as a missing object', async () => { + const service = storage(`https://${REAL_BUCKET}.${HOST}`, true); + + await expect( + service.copyObject({ + sourceBucket: 'fluxer', + sourceKey: 'emojis/missing', + destinationBucket: 'fluxer', + destinationKey: 'emojis/3', + }), + ).rejects.toMatchObject({name: 'NoSuchKey'}); + expect(objects.has('fluxer/emojis/3')).toBe(false); + }); + }, +); diff --git a/fluxer_api/src/api/test/mocks/MockStorageService.ts b/fluxer_api/src/api/test/mocks/MockStorageService.ts index 486afd0c4..0531f815e 100644 --- a/fluxer_api/src/api/test/mocks/MockStorageService.ts +++ b/fluxer_api/src/api/test/mocks/MockStorageService.ts @@ -216,9 +216,6 @@ export class MockStorageService implements IStorageService { contentLength: number; contentRange?: string | null; contentType?: string | null; - cacheControl?: string | null; - contentDisposition?: string | null; - expires?: Date | null; etag?: string | null; lastModified?: Date | null; } | null> { @@ -247,9 +244,6 @@ export class MockStorageService implements IStorageService { contentLength: slice.length, contentRange, contentType: obj?.contentType ?? 'application/octet-stream', - cacheControl: null, - contentDisposition: null, - expires: null, etag: computeEtag(data), lastModified: obj?.lastModified ?? null, }; diff --git a/fluxer_docs/src/content/docs/operator/configuration.mdx b/fluxer_docs/src/content/docs/operator/configuration.mdx index ebdeb0564..6a7b3facc 100644 --- a/fluxer_docs/src/content/docs/operator/configuration.mdx +++ b/fluxer_docs/src/content/docs/operator/configuration.mdx @@ -524,6 +524,10 @@ Default `http://localhost:3900`. The S3 API address. `media-proxy` defaults to e An endpoint with a bucket in its path, such as `https://s3.example.com/my-bucket` with `FLUXER_S3_FORCE_PATH_STYLE=true`, keeps every Fluxer bucket inside that one bucket. Each bucket name becomes a key prefix there, so uploads land under `fluxer-uploads/` in `my-bucket`. +An endpoint with the bucket in its hostname, such as `https://my-bucket.s3.example.com` with `FLUXER_S3_FORCE_PATH_STYLE=true`, turns each bucket name into a key prefix the same way. The store cannot resolve a Fluxer bucket name as the source of a server-side copy there. The API downloads the object to its temporary directory and uploads the copy from there. + +The API cannot list objects through either kind of endpoint, because the store reads the bucket name in a listing request as a key. [Report deletion](/admin-api/reports/#report-retention) lists a report's profile snapshot images first. On a store that answers the listing with an error, the deletion fails. Separate buckets do not have this limit. + #### `FLUXER_S3_PUBLIC_ENDPOINT` No default. The host substituted into presigned URLs, which are the only place it appears. Compose forwards it from `.env`, and uses `FLUXER_S3_ENDPOINT` when it is unset, except on `app-proxy`, which gets only a value `.env` sets and adds it to its Content Security Policy. `admin` reads it to find the origin that report downloads are presigned against. See `FLUXER_S3_BUCKET_REPORTS`.