fix(api): copy objects without server-side copy on one bucket (#3445)

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>;

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<void> {
 		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<void> {
 		await this.ensureDirectoryExists(path.dirname(filePath));
+		await this.downloadObject(bucket, key, filePath);
+	}
+
+	private async downloadObject(bucket: string, key: string, filePath: string): Promise<GetObjectCommandOutput> {
 		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<void> {
-		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);
 		}
 	}
 

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,

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<string, {body: Uint8Array; contentType: string}>();
 	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<typeof singleBucketStore>;
+
+		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);
+		});
+	},
+);

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,
 		};