diff --git a/.changeset/poor-lizards-juggle.md b/.changeset/poor-lizards-juggle.md new file mode 100644 index 0000000000..8d03cd5bdc --- /dev/null +++ b/.changeset/poor-lizards-juggle.md @@ -0,0 +1,5 @@ +--- +"@comet/cms-api": patch +--- + +Prevent the API from crashing because of stream errors when delivering a file diff --git a/.changeset/spotty-ladybugs-travel.md b/.changeset/spotty-ladybugs-travel.md new file mode 100644 index 0000000000..195456352b --- /dev/null +++ b/.changeset/spotty-ladybugs-travel.md @@ -0,0 +1,15 @@ +--- +"@comet/cms-api": patch +--- + +Prevent socket exhaustion in `BlobStorageS3Storage` + +By default, the S3 client allows a maximum of 50 open sockets. +A socket is only released once a file is streamed completely. +Meaning, it can remain open forever if a file stream is interrupted (e.g., when the user leaves the site). +This could lead to socket exhaustion, preventing further file delivery. + +To resolve this, the following changes were made: + +1. Add a close handler to destroy the stream when the client disconnects +2. Set a 60-second `requestTimeout` to close unused connections diff --git a/demo/api/src/config/environment-variables.ts b/demo/api/src/config/environment-variables.ts index 33ca9988cd..118a20f6d7 100644 --- a/demo/api/src/config/environment-variables.ts +++ b/demo/api/src/config/environment-variables.ts @@ -76,23 +76,23 @@ export class EnvironmentVariables { @IsString() BLOB_STORAGE_DIRECTORY_PREFIX: string; - @ValidateIf((v) => v.DAM_STORAGE_DRIVER === "s3") + @ValidateIf((v) => v.BLOB_STORAGE_DRIVER === "s3") @IsString() S3_REGION: string; - @ValidateIf((v) => v.DAM_STORAGE_DRIVER === "s3") + @ValidateIf((v) => v.BLOB_STORAGE_DRIVER === "s3") @IsString() S3_ENDPOINT: string; - @ValidateIf((v) => v.DAM_STORAGE_DRIVER === "s3") + @ValidateIf((v) => v.BLOB_STORAGE_DRIVER === "s3") @IsString() S3_ACCESS_KEY_ID: string; - @ValidateIf((v) => v.DAM_STORAGE_DRIVER === "s3") + @ValidateIf((v) => v.BLOB_STORAGE_DRIVER === "s3") @IsString() S3_SECRET_ACCESS_KEY: string; - @ValidateIf((v) => v.DAM_STORAGE_DRIVER === "s3") + @ValidateIf((v) => v.BLOB_STORAGE_DRIVER === "s3") @IsString() S3_BUCKET: string; diff --git a/packages/api/cms-api/src/blob-storage/backends/azure/blob-storage-azure.storage.ts b/packages/api/cms-api/src/blob-storage/backends/azure/blob-storage-azure.storage.ts index 79cbe7470c..2555e362ff 100644 --- a/packages/api/cms-api/src/blob-storage/backends/azure/blob-storage-azure.storage.ts +++ b/packages/api/cms-api/src/blob-storage/backends/azure/blob-storage-azure.storage.ts @@ -75,16 +75,16 @@ export class BlobStorageAzureStorage implements BlobStorageBackendInterface { } } - async getFile(folderName: string, fileName: string): Promise { + async getFile(folderName: string, fileName: string): Promise { const response = await this.client.getContainerClient(folderName).getBlobClient(fileName).download(); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return response.readableStreamBody!; // is defined in node.js but not for browsers + return Readable.from(response.readableStreamBody!); // is defined in node.js but not for browsers } - async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { + async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { const response = await this.client.getContainerClient(folderName).getBlobClient(fileName).download(offset, length); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return response.readableStreamBody!; // is defined in node.js but not for browsers + return Readable.from(response.readableStreamBody!); // is defined in node.js but not for browsers } async removeFile(folderName: string, fileName: string): Promise { diff --git a/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.interface.ts b/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.interface.ts index 9ea8d6ff1a..9dbc530d61 100644 --- a/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.interface.ts +++ b/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.interface.ts @@ -1,3 +1,5 @@ +import { Readable } from "stream"; + export type StorageMetaData = { size: number; etag?: string; @@ -16,8 +18,8 @@ export interface BlobStorageBackendInterface { removeFolder(folderName: string): Promise; fileExists(folderName: string, fileName: string): Promise; createFile(folderName: string, fileName: string, data: NodeJS.ReadableStream | Buffer | string, options: CreateFileOptions): Promise; - getFile(folderName: string, fileName: string): Promise; - getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise; + getFile(folderName: string, fileName: string): Promise; + getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise; getFileMetaData(folderName: string, fileName: string): Promise; removeFile(folderName: string, fileName: string): Promise; getBackendFilePathPrefix(): string; diff --git a/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.service.ts b/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.service.ts index 4973b6ee16..51bc22c631 100644 --- a/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.service.ts +++ b/packages/api/cms-api/src/blob-storage/backends/blob-storage-backend.service.ts @@ -1,4 +1,5 @@ import { Inject, Injectable } from "@nestjs/common"; +import { Readable } from "stream"; import { createHashedPath } from "../../dam/files/files.utils"; import { BlobStorageConfig } from "../blob-storage.config"; @@ -47,11 +48,11 @@ export class BlobStorageBackendService implements BlobStorageBackendInterface { return this.backend.createFile(folderName, fileName, data, { ...options, headers: normalizeHeaders(headers) }); } - async getFile(folderName: string, fileName: string): Promise { + async getFile(folderName: string, fileName: string): Promise { return this.backend.getFile(folderName, fileName); } - async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { + async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { return this.backend.getPartialFile(folderName, fileName, offset, length); } diff --git a/packages/api/cms-api/src/blob-storage/backends/file/blob-storage-file.storage.ts b/packages/api/cms-api/src/blob-storage/backends/file/blob-storage-file.storage.ts index a035c901e6..9bf892e48c 100644 --- a/packages/api/cms-api/src/blob-storage/backends/file/blob-storage-file.storage.ts +++ b/packages/api/cms-api/src/blob-storage/backends/file/blob-storage-file.storage.ts @@ -1,6 +1,6 @@ import * as fs from "fs"; import * as path from "path"; -import { Stream } from "stream"; +import { Readable, Stream } from "stream"; import { BlobStorageBackendInterface, CreateFileOptions, StorageMetaData } from "../blob-storage-backend.interface"; import { BlobStorageFileConfig } from "./blob-storage-file.config"; @@ -76,11 +76,11 @@ export class BlobStorageFileStorage implements BlobStorageBackendInterface { ]); } - async getFile(folderName: string, fileName: string): Promise { + async getFile(folderName: string, fileName: string): Promise { return fs.createReadStream(`${this.path}/${folderName}/${fileName}`); } - async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { + async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { return fs.createReadStream(`${this.path}/${folderName}/${fileName}`, { start: offset, end: offset + length - 1, diff --git a/packages/api/cms-api/src/blob-storage/backends/s3/blob-storage-s3.storage.ts b/packages/api/cms-api/src/blob-storage/backends/s3/blob-storage-s3.storage.ts index ad60319861..ca754d9b7f 100644 --- a/packages/api/cms-api/src/blob-storage/backends/s3/blob-storage-s3.storage.ts +++ b/packages/api/cms-api/src/blob-storage/backends/s3/blob-storage-s3.storage.ts @@ -12,6 +12,13 @@ export class BlobStorageS3Storage implements BlobStorageBackendInterface { constructor(config: BlobStorageS3Config["s3"]) { this.client = new AWS.S3({ + requestHandler: { + // https://github.com/aws/aws-sdk-js-v3/blob/main/supplemental-docs/CLIENTS.md#request-handler-requesthandler + // Workaround to prevent socket exhaustion caused by dangling streams (e.g., when the user leaves the site). + // Close the connection when no request/response was sent for 60 seconds, indicating that the file stream was terminated. + requestTimeout: 60000, + connectionTimeout: 6000, // fail faster if there are no available connections + }, credentials: { accessKeyId: config.accessKeyId, secretAccessKey: config.secretAccessKey, @@ -95,14 +102,14 @@ export class BlobStorageS3Storage implements BlobStorageBackendInterface { await this.client.send(new AWS.PutObjectCommand(input)); } - async getFile(folderName: string, fileName: string): Promise { + async getFile(folderName: string, fileName: string): Promise { const response = await this.client.send(new AWS.GetObjectCommand(this.getCommandInput(folderName, fileName))); // Blob is not supported and used in node return Readable.from(response.Body as Readable | NodeJS.ReadableStream); } - async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { + async getPartialFile(folderName: string, fileName: string, offset: number, length: number): Promise { const input: AWS.GetObjectCommandInput = { ...this.getCommandInput(folderName, fileName), Range: `bytes=${offset}-${offset + length - 1}`, diff --git a/packages/api/cms-api/src/dam/files/files.controller.ts b/packages/api/cms-api/src/dam/files/files.controller.ts index 0f402212df..b33aeb8ee8 100644 --- a/packages/api/cms-api/src/dam/files/files.controller.ts +++ b/packages/api/cms-api/src/dam/files/files.controller.ts @@ -6,6 +6,7 @@ import { Get, Headers, Inject, + Logger, NotFoundException, Param, Post, @@ -18,6 +19,7 @@ import { plainToInstance } from "class-transformer"; import { validate } from "class-validator"; import { Response } from "express"; import { OutgoingHttpHeaders } from "http"; +import { Readable } from "stream"; import { GetCurrentUser } from "../../auth/decorators/get-current-user.decorator"; import { DisableGlobalGuard } from "../../auth/decorators/global-guard-disable.decorator"; @@ -54,6 +56,7 @@ export function createFilesController({ Scope: PassedScope }: { Scope?: Type { + this.logger.error("Stream error:", error); + res.end(); + }); + + res.on("close", () => { + stream.destroy(); + }); + res.writeHead(200, { ...headers, ...options?.overrideHeaders, }); } - response.pipe(res); + stream.pipe(res); } }