Upgrade aws-sdk to v3 (re-attempt) (#28410)

* Upgrade `aws-sdk` to v3

Also vendored nodemailer-ses-transport

* Moved default region to CE settings

* Ensure timeout is added to `requestHandler` and `region` is populated

* fix unsigned header in signed URL request

The x-amz-acl header is not needed when using the signed request,
as ACL are already defined when creating the signed URL in
PutObjectCommand constructor.

* Add default AWS region for latexqc service

* remove unnecessary region in compose files

* Use AWS_REGION=us-west-2 for history-v1

* prevent retries uploading streams with PutObjectCommand

* Remove AWS SDK JS message suppression

GitOrigin-RevId: 6fda6f02160023ffed76143397bbd965f86a9509

libraries/object-persistor/README.md

@@ -264,7 +264,8 @@ For the `FS` persistor, the `bucketName` should be the full path to the folder o
 - `s3.key` (required): The AWS access key ID
 - `s3.secret` (required): The AWS secret access key
 - `s3.partSize`: The part size for S3 uploads. Defaults to 100 megabytes.
-- `s3.httpOptions`: HTTP options passed directly to the [S3 constructor](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#constructor-property).
+- `s3.httpOptions`: HTTP Options passed to the [`NodeHttpHandler` constructor](https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-smithy-node-http-handler/Class/NodeHttpHandler/)
+  - For backwards compatibility reasons, the `timeout` property that was passed to the [S3 constructor](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#constructor-property) before migrating to AWS SDK v3 is now passed to the `NodeHttpHandler` constructor as `connectionTimeout`.
 - `s3.maxRetries`: The number of times the S3 client will retry in case of an error
 - `s3.endpoint`: For testing - overrides the S3 endpoint to use a different service (e.g. a fake S3 server)
 - `s3.pathStyle`: For testing - use old path-style URLs, for services that do not support subdomain-based access

libraries/object-persistor/package.json

@@ -20,12 +20,15 @@
   "author": "Overleaf (https://www.overleaf.com/)",
   "license": "AGPL-3.0",
   "dependencies": {
+    "@aws-sdk/client-s3": "^3.864.0",
+    "@aws-sdk/lib-storage": "^3.864.0",
+    "@aws-sdk/node-http-handler": "^3.370.0",
+    "@aws-sdk/s3-request-presigner": "^3.864.0",
     "@google-cloud/storage": "^6.10.1",
     "@overleaf/logger": "*",
     "@overleaf/metrics": "*",
     "@overleaf/o-error": "*",
     "@overleaf/stream-utils": "*",
-    "aws-sdk": "^2.1691.0",
     "fast-crc32c": "overleaf/node-fast-crc32c#aae6b2a4c7a7a159395df9cc6c38dfde702d6f51",
     "glob": "^7.1.6",
     "range-parser": "^1.2.1",

libraries/object-persistor/src/PerProjectEncryptedS3Persistor.js

@@ -20,10 +20,6 @@ const hkdf = promisify(Crypto.hkdf)
 
 const AES256_KEY_LENGTH = 32
 
-/**
- * @typedef {import('aws-sdk').AWSError} AWSError
- */
-
 /**
  * @typedef {Object} Settings
  * @property {boolean} automaticallyRotateDEKEncryption
@@ -37,16 +33,6 @@ const AES256_KEY_LENGTH = 32
  * @typedef {import('./types').ListDirectoryResult} ListDirectoryResult
  */
 
-/**
- * Helper function to make TS happy when accessing error properties
- * AWSError is not an actual class, so we cannot use instanceof.
- * @param {any} err
- * @return {err is AWSError}
- */
-function isAWSError(err) {
-  return !!err
-}
-
 /**
  * @param {any} err
  * @return {boolean}
@@ -55,9 +41,8 @@ function isForbiddenError(err) {
   if (!err || !(err instanceof ReadError || err instanceof NotFoundError)) {
     return false
   }
-  const cause = err.cause
+  // @ts-ignore
-  if (!isAWSError(cause)) return false
+  return err?.cause.statusCode === 403 || err?.cause.Code === 'AccessDenied'
-  return cause.statusCode === 403
 }
 
 class RootKeyEncryptionKey {

libraries/object-persistor/src/PersistorHelper.js

@@ -144,17 +144,22 @@ function wrapError(error, message, params, ErrorType) {
     ...params,
     cause: error,
   }
+
+  // aws-sdk v3 renames `code` to `Code`, but it's not always present, so we
+  // add a fallback to `name` for compatibility.
+  const errorCode = error.code || error.Code || error.name
+
   if (
     error instanceof NotFoundError ||
     ['NoSuchKey', 'NotFound', 404, 'AccessDenied', 'ENOENT'].includes(
-      error.code
+      errorCode
     ) ||
     (error.response && error.response.statusCode === 404)
   ) {
     return new NotFoundError('no such file', params, error)
   } else if (
     params.ifNoneMatch === '*' &&
-    (error.code === 'PreconditionFailed' ||
+    (errorCode === 'PreconditionFailed' ||
       error.response?.statusCode === 412 ||
       error instanceof AlreadyWrittenError)
   ) {

libraries/object-persistor/src/S3Md5Middleware.js

@@ -0,0 +1,49 @@
+const Crypto = require('node:crypto')
+
+/**
+ * Creates an S3 client that uses MD5 checksums for DeleteObjects operations
+ * https://github.com/aws/aws-sdk-js-v3/blob/main/supplemental-docs/MD5_FALLBACK.md
+ */
+const md5Middleware = (next, context) => async args => {
+  if (context.commandName !== 'DeleteObjectsCommand') {
+    return next(args)
+  }
+
+  const headers = args.request.headers
+
+  // Remove any checksum headers added by default middleware
+  // This ensures our Content-MD5 is the primary integrity check
+  Object.keys(headers).forEach(header => {
+    const lowerHeader = header.toLowerCase()
+    if (
+      lowerHeader.startsWith('x-amz-checksum-') ||
+      lowerHeader.startsWith('x-amz-sdk-checksum-')
+    ) {
+      delete headers[header]
+    }
+  })
+
+  if (args.request.body) {
+    const bodyContent = Buffer.from(args.request.body)
+    headers['Content-MD5'] = Crypto.createHash('md5')
+      .update(bodyContent)
+      .digest('base64')
+  }
+
+  return await next(args)
+}
+
+function addMd5Middleware(client) {
+  // Add the middleware relative to the flexible checksums middleware
+  // This ensures it runs after default checksums might be added, but before signing
+  client.middlewareStack.add(md5Middleware, {
+    step: 'build',
+    toMiddleware: 'flexibleChecksumsMiddleware',
+    name: 'addMD5ChecksumForDeleteObjects',
+    tags: ['MD5_FALLBACK'],
+  })
+}
+
+module.exports = {
+  addMd5Middleware,
+}

libraries/object-persistor/src/S3Persistor.js

@@ -15,18 +15,23 @@ const PersistorHelper = require('./PersistorHelper')
 
 const { pipeline, PassThrough } = require('node:stream')
 const fs = require('node:fs')
-const S3 = require('aws-sdk/clients/s3')
+const {
-const { URL } = require('node:url')
+  S3Client,
+  CreateBucketCommand,
+  GetObjectCommand,
+  PutObjectCommand,
+  HeadObjectCommand,
+  DeleteObjectCommand,
+  ListObjectsV2Command,
+  DeleteObjectsCommand,
+  CopyObjectCommand,
+} = require('@aws-sdk/client-s3')
+const { Upload } = require('@aws-sdk/lib-storage')
+const { getSignedUrl } = require('@aws-sdk/s3-request-presigner')
+const { NodeHttpHandler } = require('@aws-sdk/node-http-handler')
 const { WriteError, ReadError, NotFoundError } = require('./Errors')
 const zlib = require('node:zlib')
-
+const { addMd5Middleware } = require('./S3Md5Middleware')
-/**
- * @typedef {import('aws-sdk/clients/s3').ListObjectsV2Output} ListObjectsV2Output
- */
-
-/**
- * @typedef {import('aws-sdk/clients/s3').Object} S3Object
- */
 
 /**
  * @typedef {import('./types').ListDirectoryResult} ListDirectoryResult
@@ -73,9 +78,12 @@ class SSECOptions {
 }
 
 class S3Persistor extends AbstractPersistor {
-  /** @type {Map<string, S3>} */
+  /** @type {Map<string, S3Client>} */
   #clients = new Map()
 
+  /** @type {Map<string, S3Client>} */
+  #noRetryClients = new Map()
+
   constructor(settings = {}) {
     super()
 
@@ -117,7 +125,7 @@ class S3Persistor extends AbstractPersistor {
       // observer will catch errors, clean up and log a warning
       pipeline(readStream, observer, () => {})
 
-      /** @type {S3.PutObjectRequest} */
+      /** @type {import('@aws-sdk/client-s3').PutObjectCommandInput} */
       const uploadOptions = {
         Bucket: bucketName,
         Key: key,
@@ -154,13 +162,21 @@ class S3Persistor extends AbstractPersistor {
       }
 
       if (this.settings.disableMultiPartUpload) {
-        await this._getClientForBucket(bucketName, computeChecksums)
+        // retries are disabled for `PutObjectCommand` when using streams
-          .putObject(uploadOptions)
+        // https://github.com/aws/aws-sdk-js-v3/issues/6770
-          .promise()
+        const noRetry = true
+        await this._getClientForBucket(
+          bucketName,
+          computeChecksums,
+          noRetry
+        ).send(new PutObjectCommand(uploadOptions))
       } else {
-        await this._getClientForBucket(bucketName, computeChecksums)
+        const upload = new Upload({
-          .upload(uploadOptions, { partSize: this.settings.partSize })
+          client: this._getClientForBucket(bucketName, computeChecksums),
-          .promise()
+          params: uploadOptions,
+          partSize: this.settings.partSize,
+        })
+        await upload.done()
       }
     } catch (err) {
       throw PersistorHelper.wrapError(
@@ -185,6 +201,7 @@ class S3Persistor extends AbstractPersistor {
   async getObjectStream(bucketName, key, opts) {
     opts = opts || {}
 
+    /** @type {import('@aws-sdk/client-s3').GetObjectCommandInput} */
     const params = {
       Bucket: bucketName,
       Key: key,
@@ -200,32 +217,18 @@ class S3Persistor extends AbstractPersistor {
       bucket: bucketName,
     })
 
-    const req = this._getClientForBucket(bucketName).getObject(params)
+    const abortController = new AbortController()
-    const stream = req.createReadStream()
+    let stream, contentEncoding
-
-    let contentEncoding
     try {
-      await new Promise((resolve, reject) => {
+      const { Body, ContentEncoding } = await this._getClientForBucket(
-        req.on('httpHeaders', (statusCode, headers) => {
+        bucketName
-          switch (statusCode) {
+      ).send(new GetObjectCommand(params), {
-            case 200: // full response
+        abortSignal: abortController.signal,
-            case 206: // partial response
-              contentEncoding = headers['content-encoding']
-              return resolve(undefined)
-            case 403: // AccessDenied
-              return // handled by stream.on('error') handler below
-            case 404: // NoSuchKey
-              return reject(new NotFoundError('not found'))
-            default:
-            // handled by stream.on('error') handler below
-          }
-        })
-        // The AWS SDK is forwarding any errors from the request to the stream.
-        // The AWS SDK is emitting additional errors on the stream ahead of starting to stream.
-        stream.on('error', reject)
-        // The AWS SDK is kicking off the request in the next event loop cycle.
       })
+      stream = Body
+      contentEncoding = ContentEncoding
     } catch (err) {
+      abortController.abort()
       throw PersistorHelper.wrapError(
         err,
         'error reading file from S3',
@@ -239,8 +242,11 @@ class S3Persistor extends AbstractPersistor {
     if (contentEncoding === 'gzip' && opts.autoGunzip) {
       transformer.push(zlib.createGunzip())
     }
+    // @ts-ignore stream (Body) can be undefined in GetObjectCommand
     pipeline(stream, observer, ...transformer, pass, err => {
-      if (err) req.abort()
+      if (err) {
+        abortController.abort()
+      }
     })
     return pass
   }
@@ -253,13 +259,13 @@ class S3Persistor extends AbstractPersistor {
   async getRedirectUrl(bucketName, key) {
     const expiresSeconds = Math.round(this.settings.signedUrlExpiryInMs / 1000)
     try {
-      return await this._getClientForBucket(bucketName).getSignedUrlPromise(
+      return await getSignedUrl(
-        'getObject',
+        this._getClientForBucket(bucketName),
-        {
+        new GetObjectCommand({
           Bucket: bucketName,
           Key: key,
-          Expires: expiresSeconds,
+        }),
-        }
+        { expiresIn: expiresSeconds }
       )
     } catch (err) {
       throw PersistorHelper.wrapError(
@@ -286,15 +292,15 @@ class S3Persistor extends AbstractPersistor {
     const objects = contents.map(item => ({ Key: item.Key || '' }))
     if (objects?.length) {
       try {
-        await this._getClientForBucket(bucketName)
+        await this._getClientForBucket(bucketName).send(
-          .deleteObjects({
+          new DeleteObjectsCommand({
             Bucket: bucketName,
             Delete: {
               Objects: objects,
               Quiet: true,
             },
           })
-          .promise()
+        )
       } catch (err) {
         throw PersistorHelper.wrapError(
           err,
@@ -329,9 +335,9 @@ class S3Persistor extends AbstractPersistor {
     }
 
     try {
-      response = await this._getClientForBucket(bucketName)
+      response = await this._getClientForBucket(bucketName).send(
-        .listObjectsV2(options)
+        new ListObjectsV2Command(options)
-        .promise()
+      )
     } catch (err) {
       throw PersistorHelper.wrapError(
         err,
@@ -349,7 +355,7 @@ class S3Persistor extends AbstractPersistor {
    * @param {string} key
    * @param {Object} opts
    * @param {SSECOptions} [opts.ssecOptions]
-   * @return {Promise<S3.HeadObjectOutput>}
+   * @return {Promise<import('@aws-sdk/client-s3').HeadObjectOutput>}
    */
   async #headObject(bucketName, key, opts = {}) {
     const params = { Bucket: bucketName, Key: key }
@@ -357,9 +363,8 @@ class S3Persistor extends AbstractPersistor {
       Object.assign(params, opts.ssecOptions.getGetOptions())
     }
     try {
-      return await this._getClientForBucket(bucketName)
+      const client = await this._getClientForBucket(bucketName)
-        .headObject(params)
+      return await client.send(new HeadObjectCommand(params))
-        .promise()
     } catch (err) {
       throw PersistorHelper.wrapError(
         err,
@@ -433,9 +438,9 @@ class S3Persistor extends AbstractPersistor {
    */
   async deleteObject(bucketName, key) {
     try {
-      await this._getClientForBucket(bucketName)
+      await this._getClientForBucket(bucketName).send(
-        .deleteObject({ Bucket: bucketName, Key: key })
+        new DeleteObjectCommand({ Bucket: bucketName, Key: key })
-        .promise()
+      )
     } catch (err) {
       // s3 does not give us a NotFoundError here
       throw PersistorHelper.wrapError(
@@ -460,7 +465,7 @@ class S3Persistor extends AbstractPersistor {
     const params = {
       Bucket: bucketName,
       Key: destKey,
-      CopySource: `${bucketName}/${sourceKey}`,
+      CopySource: `/${bucketName}/${sourceKey}`,
     }
     if (opts.ssecSrcOptions) {
       Object.assign(params, opts.ssecSrcOptions.getCopyOptions())
@@ -469,7 +474,9 @@ class S3Persistor extends AbstractPersistor {
       Object.assign(params, opts.ssecOptions.getPutOptions())
     }
     try {
-      await this._getClientForBucket(bucketName).copyObject(params).promise()
+      await this._getClientForBucket(bucketName).send(
+        new CopyObjectCommand(params)
+      )
     } catch (err) {
       throw PersistorHelper.wrapError(
         err,
@@ -519,9 +526,9 @@ class S3Persistor extends AbstractPersistor {
       if (continuationToken) {
         options.ContinuationToken = continuationToken
       }
-      const response = await this._getClientForBucket(bucketName)
+      const response = await this._getClientForBucket(bucketName).send(
-        .listObjectsV2(options)
+        new ListObjectsV2Command(options)
-        .promise()
+      )
 
       const size =
         response.Contents?.reduce((acc, item) => (item.Size || 0) + acc, 0) || 0
@@ -549,36 +556,46 @@ class S3Persistor extends AbstractPersistor {
   /**
    * @param {string} bucket
    * @param {boolean} computeChecksums
-   * @return {S3}
+   * @param {boolean} noRetries
+   * @return {S3Client}
    * @private
    */
-  _getClientForBucket(bucket, computeChecksums = false) {
+  _getClientForBucket(bucket, computeChecksums = false, noRetries = false) {
-    /** @type {S3.Types.ClientConfiguration} */
+    /** @type {import('@aws-sdk/client-s3').S3ClientConfig} */
     const clientOptions = {}
     const cacheKey = `${bucket}:${computeChecksums}`
     if (computeChecksums) {
-      clientOptions.computeChecksums = true
+      clientOptions.requestChecksumCalculation = 'WHEN_SUPPORTED'
+      clientOptions.responseChecksumValidation = 'WHEN_SUPPORTED'
     }
-    let client = this.#clients.get(cacheKey)
+
+    const clientMap = noRetries ? this.#noRetryClients : this.#clients
+    let client = clientMap.get(cacheKey)
     if (!client) {
-      client = new S3(
+      client = new S3Client(
         this._buildClientOptions(
           this.settings.bucketCreds?.[bucket],
-          clientOptions
+          clientOptions,
+          noRetries
         )
       )
-      this.#clients.set(cacheKey, client)
+      clientMap.set(cacheKey, client)
+
+      // https://github.com/aws/aws-sdk-js-v3/blob/main/supplemental-docs/MD5_FALLBACK.md
+      addMd5Middleware(client)
     }
+
     return client
   }
 
   /**
    * @param {Object} bucketCredentials
-   * @param {S3.Types.ClientConfiguration} clientOptions
+   * @param {import('@aws-sdk/client-s3').S3ClientConfig} clientOptions
-   * @return {S3.Types.ClientConfiguration}
+   * @param {boolean} noRetries
+   * @return {import('@aws-sdk/client-s3').S3ClientConfig}
    * @private
    */
-  _buildClientOptions(bucketCredentials, clientOptions) {
+  _buildClientOptions(bucketCredentials, clientOptions, noRetries = false) {
     const options = clientOptions || {}
 
     if (bucketCredentials) {
@@ -593,39 +610,89 @@ class S3Persistor extends AbstractPersistor {
       }
     } else {
       // Use the default credentials provider (process.env -> SSP -> ini -> IAM)
-      // Docs: https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/CredentialProviderChain.html#defaultProviders-property
+      // Docs: https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/setting-credentials-node.html
     }
 
+    // region is required since aws sdk v3
+    options.region =
+      this.settings.region || process.env.AWS_REGION || 'us-east-1'
+
+    let sslEnabled = false
     if (this.settings.endpoint) {
       const endpoint = new URL(this.settings.endpoint)
       options.endpoint = this.settings.endpoint
-      options.sslEnabled = endpoint.protocol === 'https:'
+      sslEnabled = endpoint.protocol === 'https:'
     }
 
     // path-style access is only used for acceptance tests
     if (this.settings.pathStyle) {
-      options.s3ForcePathStyle = true
+      options.forcePathStyle = true
     }
 
-    for (const opt of ['httpOptions', 'maxRetries', 'region']) {
+    // maxRetries has been moved to maxAttempts in aws-sdk v3,
-      if (this.settings[opt]) {
+    // we're keeping the existing setting for backwards compatibility
-        options[opt] = this.settings[opt]
+    if (!noRetries && this.settings.maxRetries) {
-      }
+      options.maxAttempts = this.settings.maxRetries + 1
     }
 
-    if (options.sslEnabled && this.settings.ca && !options.httpOptions?.agent) {
+    const requestHandlerParams = this.settings.httpOptions || {}
-      options.httpOptions = options.httpOptions || {}
+
-      options.httpOptions.agent = new https.Agent({
+    if (sslEnabled && this.settings.ca) {
+      const agent = new https.Agent({
         rejectUnauthorized: true,
         ca: this.settings.ca,
       })
+      Object.assign(requestHandlerParams, {
+        httpAgent: agent,
+        httpsAgent: agent,
+      })
+      options.requestHandler = new NodeHttpHandler({
+        httpAgent: agent,
+        httpsAgent: agent,
+      })
+    }
+
+    const requestHandler = this._buildNodeHttpHandler(requestHandlerParams)
+    if (requestHandler) {
+      options.requestHandler = requestHandler
     }
 
     return options
   }
 
   /**
-   * @param {S3.HeadObjectOutput} response
+   * @param {import('@aws-sdk/node-http-handler').NodeHttpHandlerOptions & { timeout?: number }} params
+   * @private
+   */
+  _buildNodeHttpHandler(params) {
+    const isEmpty = Object.keys(params).length === 0
+    if (isEmpty) {
+      return
+    }
+    // ensures backwards compatibility with aws-sdk v2 httpOptions
+    if (params.timeout) {
+      params.connectionTimeout = params.timeout
+      delete params.timeout
+    }
+    return new NodeHttpHandler(params)
+  }
+
+  // test-only
+  _createBucket(bucketName) {
+    return this._getClientForBucket(bucketName).send(
+      new CreateBucketCommand({ Bucket: bucketName })
+    )
+  }
+
+  // test-only
+  _upload(bucketName, uploadOptions) {
+    return this._getClientForBucket(bucketName).send(
+      new PutObjectCommand(uploadOptions)
+    )
+  }
+
+  /**
+   * @param {import('@aws-sdk/client-s3').HeadObjectOutput} response
    * @return {string|null}
    * @private
    */

libraries/object-persistor/src/types.d.ts

@@ -1,6 +1,6 @@
-import type { ListObjectsV2Output, Object } from 'aws-sdk/clients/s3'
+import type { ListObjectsV2Output, _Object } from '@aws-sdk/client-s3'
 
 export type ListDirectoryResult = {
-  contents: Array<Object>
+  contents: Array<_Object>
   response: ListObjectsV2Output
 }

libraries/object-persistor/test/unit/S3ClientMock.js

@@ -0,0 +1,112 @@
+/* eslint-disable new-cap */
+const sinon = require('sinon')
+const chai = require('chai')
+
+const { expect, assert } = chai
+
+module.exports = function () {
+  const s3ClientStub = {
+    send: sinon.stub(),
+    middlewareStack: new Set(),
+  }
+
+  const assertSendCalledWith = (s3Command, payload) => {
+    for (let i = 0; i < s3ClientStub.send.callCount; i++) {
+      const call = s3ClientStub.send.getCall(i)
+      const callArg = call.args[0]
+      if (callArg?.name === new s3Command().name) {
+        if (payload) {
+          expect(callArg.payload).to.deep.equal(payload)
+        }
+        return
+      }
+    }
+    assert.fail(
+      `Expected S3Client to be called with '${new s3Command().name}' command but it was not called`
+    )
+  }
+
+  const assertSendCallCount = (s3Command, expectedCount) => {
+    const callCount = s3ClientStub.send.callCount
+    let counter = 0
+    for (let i = 0; i < callCount; i++) {
+      const call = s3ClientStub.send.getCall(i)
+      const callArg = call.args[0]
+      if (callArg?.name === new s3Command().name) {
+        counter++
+      }
+    }
+    expect(counter).to.equal(expectedCount)
+  }
+
+  const assertSendNotCalledWith = s3Command => {
+    for (let i = 0; i < s3ClientStub.send.callCount; i++) {
+      const call = s3ClientStub.send.getCall(i)
+      const callArg = call.args[0]
+      if (callArg?.name === new s3Command().name) {
+        assert.fail(
+          `Expected S3Client not to be called with '${new s3Command().name}' command but it was called`
+        )
+      }
+    }
+  }
+
+  const mockSend = (s3Command, response, options = {}) => {
+    const mock = s3ClientStub.send.withArgs(sinon.match.instanceOf(s3Command))
+    const fn = options.rejects ? 'rejects' : 'resolves'
+
+    if (options.nextResponses?.length) {
+      mock.onCall(0)[fn](response)
+      for (let i = 0; i < options.nextResponses.length; i++) {
+        mock.onCall(i + 1)[fn](options.nextResponses[i])
+      }
+    } else {
+      mock[fn](response)
+    }
+  }
+
+  return {
+    s3ClientStub,
+    assertSendCalledWith,
+    assertSendCallCount,
+    assertSendNotCalledWith,
+    mockSend,
+    S3Client: sinon.stub().returns(s3ClientStub),
+    CopyObjectCommand: class DeleteObjectCommand {
+      constructor(payload) {
+        this.name = 'CopyObjectCommand'
+        this.payload = payload
+      }
+    },
+    DeleteObjectCommand: class DeleteObjectCommand {
+      constructor(payload) {
+        this.name = 'DeleteObjectCommand'
+        this.payload = payload
+      }
+    },
+    DeleteObjectsCommand: class DeleteObjectCommand {
+      constructor(payload) {
+        this.name = 'DeleteObjectsCommand'
+        this.payload = payload
+      }
+    },
+    GetObjectCommand: class DeleteObjectCommand {
+      constructor(payload) {
+        this.name = 'GetObjectCommand'
+        this.payload = payload
+      }
+    },
+    HeadObjectCommand: class DeleteObjectCommand {
+      constructor(payload) {
+        this.name = 'HeadObjectCommand'
+        this.payload = payload
+      }
+    },
+    ListObjectsV2Command: class DeleteObjectCommand {
+      constructor(payload) {
+        this.name = 'ListObjectsV2Command'
+        this.payload = payload
+      }
+    },
+  }
+}

libraries/object-persistor/test/unit/S3PersistorTests.js

@@ -4,6 +4,8 @@ const { expect } = chai
 const SandboxedModule = require('sandboxed-module')
 const Errors = require('../../src/Errors')
 const { EventEmitter } = require('node:events')
+const { Readable } = require('node:stream')
+const mockS3 = require('./S3ClientMock')
 
 const MODULE_PATH = '../../src/S3Persistor.js'
 
@@ -15,6 +17,7 @@ describe('S3PersistorTests', function () {
       accessKeyId: defaultS3Key,
       secretAccessKey: defaultS3Secret,
     },
+    region: 'us-east-1',
   }
   const filename = '/wombat/potato.tex'
   const bucket = 'womBucket'
@@ -34,17 +37,19 @@ describe('S3PersistorTests', function () {
     Transform,
     PassThrough,
     S3,
+    awsRequestPresigner,
+    nodeHttpHandler,
+    awsLibStorage,
+    awsLibStorageUpload,
+    abortSignal,
     Fs,
     ReadStream,
     Stream,
     StreamPromises,
-    S3GetObjectRequest,
     S3Persistor,
-    S3Client,
     S3NotFoundError,
     S3AccessDeniedError,
     FileNotFoundError,
-    EmptyPromise,
     settings,
     Hash,
     crypto
@@ -72,46 +77,7 @@ describe('S3PersistorTests', function () {
       pipeline: sinon.stub().resolves(),
     }
 
-    EmptyPromise = {
-      promise: sinon.stub().resolves(),
-    }
-
     ReadStream = new EventEmitter()
-    class FakeS3GetObjectRequest extends EventEmitter {
-      constructor() {
-        super()
-        this.statusCode = 200
-        this.err = null
-        this.aborted = false
-      }
-
-      abort() {
-        this.aborted = true
-      }
-
-      createReadStream() {
-        setTimeout(() => {
-          if (this.notFoundSSEC) {
-            // special case for AWS S3: 404 NoSuchKey wrapped in a 400. A single request received a single response, and multiple httpHeaders events are triggered. Don't ask.
-            this.emit('httpHeaders', 400, {})
-            this.emit('httpHeaders', 404, {})
-            ReadStream.emit('error', S3NotFoundError)
-            return
-          }
-
-          if (this.err) return ReadStream.emit('error', this.err)
-          this.emit('httpHeaders', this.statusCode, {})
-          if (this.statusCode === 403) {
-            ReadStream.emit('error', S3AccessDeniedError)
-          }
-          if (this.statusCode === 404) {
-            ReadStream.emit('error', S3NotFoundError)
-          }
-        })
-        return ReadStream
-      }
-    }
-    S3GetObjectRequest = new FakeS3GetObjectRequest()
 
     FileNotFoundError = new Error('File not found')
     FileNotFoundError.code = 'ENOENT'
@@ -121,33 +87,27 @@ describe('S3PersistorTests', function () {
     }
 
     S3NotFoundError = new Error('not found')
-    S3NotFoundError.code = 'NoSuchKey'
+    S3NotFoundError.name = 'NoSuchKey'
 
     S3AccessDeniedError = new Error('access denied')
     S3AccessDeniedError.code = 'AccessDenied'
 
-    S3Client = {
+    S3 = mockS3()
-      getObject: sinon.stub().returns(S3GetObjectRequest),
+
-      headObject: sinon.stub().returns({
+    awsLibStorageUpload = sinon.stub().returns({
-        promise: sinon.stub().resolves({
+      done: sinon.stub().resolves(),
-          ContentLength: objectSize,
+    })
-          ETag: md5,
+
-        }),
+    awsLibStorage = {
-      }),
+      Upload: awsLibStorageUpload,
-      listObjectsV2: sinon.stub().returns({
+    }
-        promise: sinon.stub().resolves({
+    awsRequestPresigner = {
-          Contents: files,
+      getSignedUrl: sinon.stub().resolves(redirectUrl),
-        }),
+    }
-      }),
+
-      upload: sinon
+    nodeHttpHandler = {
-        .stub()
+      NodeHttpHandler: sinon.stub(),
-        .returns({ promise: sinon.stub().resolves({ ETag: `"${md5}"` }) }),
-      copyObject: sinon.stub().returns(EmptyPromise),
-      deleteObject: sinon.stub().returns(EmptyPromise),
-      deleteObjects: sinon.stub().returns(EmptyPromise),
-      getSignedUrlPromise: sinon.stub().resolves(redirectUrl),
     }
-    S3 = sinon.stub().callsFake(() => Object.assign({}, S3Client))
 
     Hash = {
       end: sinon.stub(),
@@ -162,17 +122,27 @@ describe('S3PersistorTests', function () {
       warn: sinon.stub(),
     }
 
+    abortSignal = sinon.stub()
+
+    const AbortCtrl = sinon.stub().returns({
+      signal: {},
+      abort: abortSignal,
+    })
+
     S3Persistor = new (SandboxedModule.require(MODULE_PATH, {
       requires: {
-        'aws-sdk/clients/s3': S3,
+        '@aws-sdk/client-s3': S3,
+        '@aws-sdk/lib-storage': awsLibStorage,
+        '@aws-sdk/s3-request-presigner': awsRequestPresigner,
         '@overleaf/logger': Logger,
+        '@aws-sdk/node-http-handler': nodeHttpHandler,
         './Errors': Errors,
         fs: Fs,
         stream: Stream,
         'stream/promises': StreamPromises,
         crypto,
       },
-      globals: { console, Buffer },
+      globals: { console, Buffer, AbortController: AbortCtrl },
     }).S3Persistor)(settings)
   })
 
@@ -181,6 +151,10 @@ describe('S3PersistorTests', function () {
       let stream
 
       beforeEach(async function () {
+        S3.mockSend(S3.GetObjectCommand, {
+          Body: Readable.from('content'),
+          ContentEncoding: 'gzip',
+        })
         stream = await S3Persistor.getObjectStream(bucket, key)
       })
 
@@ -188,12 +162,8 @@ describe('S3PersistorTests', function () {
         expect(stream).to.be.instanceOf(PassThrough)
       })
 
-      it('sets the AWS client up with credentials from settings', function () {
-        expect(S3).to.have.been.calledWith(defaultS3Credentials)
-      })
-
       it('fetches the right key from the right bucket', function () {
-        expect(S3Client.getObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.GetObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
@@ -201,14 +171,14 @@ describe('S3PersistorTests', function () {
 
       it('pipes the stream through the meter', async function () {
         expect(Stream.pipeline).to.have.been.calledWith(
-          ReadStream,
+          sinon.match.instanceOf(Readable),
           sinon.match.instanceOf(Transform),
           sinon.match.instanceOf(PassThrough)
         )
       })
 
       it('does not abort the request', function () {
-        expect(S3GetObjectRequest.aborted).to.equal(false)
+        expect(abortSignal).not.to.have.been.called
       })
     })
 
@@ -216,6 +186,10 @@ describe('S3PersistorTests', function () {
       let stream
 
       beforeEach(async function () {
+        S3.mockSend(S3.GetObjectCommand, {
+          Body: Readable.from('this is a longer content'),
+          ContentEncoding: 'gzip',
+        })
         stream = await S3Persistor.getObjectStream(bucket, key, {
           start: 5,
           end: 10,
@@ -227,7 +201,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('passes the byte range on to S3', function () {
-        expect(S3Client.getObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.GetObjectCommand, {
           Bucket: bucket,
           Key: key,
           Range: 'bytes=5-10',
@@ -239,6 +213,10 @@ describe('S3PersistorTests', function () {
       let stream
 
       beforeEach(async function () {
+        S3.mockSend(S3.GetObjectCommand, {
+          Body: Readable.from('content'),
+          ContentEncoding: 'gzip',
+        })
         Stream.pipeline.yields(new Error())
         stream = await S3Persistor.getObjectStream(bucket, key)
       })
@@ -248,7 +226,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('aborts the request', function () {
-        expect(S3GetObjectRequest.aborted).to.equal(true)
+        expect(abortSignal).to.have.been.calledOnce
       })
     })
 
@@ -261,9 +239,14 @@ describe('S3PersistorTests', function () {
           accessKeyId: alternativeKey,
           secretAccessKey: alternativeSecret,
         },
+        region: 'us-east-1',
       }
 
       beforeEach(async function () {
+        S3.mockSend(S3.GetObjectCommand, {
+          Body: Readable.from('content'),
+          ContentEncoding: 'gzip',
+        })
         settings.bucketCreds = {}
         settings.bucketCreds[bucket] = {
           auth_key: alternativeKey,
@@ -278,11 +261,11 @@ describe('S3PersistorTests', function () {
       })
 
       it('sets the AWS client up with the alternative credentials', function () {
-        expect(S3).to.have.been.calledWith(alternativeS3Credentials)
+        expect(S3.S3Client).to.have.been.calledWith(alternativeS3Credentials)
       })
 
       it('fetches the right key from the right bucket', function () {
-        expect(S3Client.getObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.GetObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
@@ -291,9 +274,13 @@ describe('S3PersistorTests', function () {
       it('uses the default credentials for an unknown bucket', async function () {
         stream = await S3Persistor.getObjectStream('anotherBucket', key)
 
-        expect(S3).to.have.been.calledTwice
+        expect(S3.S3Client).to.have.been.calledTwice
-        expect(S3.firstCall).to.have.been.calledWith(alternativeS3Credentials)
+        expect(S3.S3Client.firstCall).to.have.been.calledWith(
-        expect(S3.secondCall).to.have.been.calledWith(defaultS3Credentials)
+          alternativeS3Credentials
+        )
+        expect(S3.S3Client.secondCall).to.have.been.calledWith(
+          defaultS3Credentials
+        )
       })
     })
 
@@ -302,24 +289,41 @@ describe('S3PersistorTests', function () {
         delete settings.key
         delete settings.secret
 
+        S3.mockSend(S3.GetObjectCommand, {
+          Body: Readable.from('content'),
+          ContentEncoding: 'gzip',
+        })
+
         await S3Persistor.getObjectStream(bucket, key)
-        expect(S3).to.have.been.calledOnce
+        expect(S3.S3Client).to.have.been.calledOnce
-        expect(S3.args[0].credentials).to.not.exist
+        expect(S3.S3Client.args[0].credentials).to.not.exist
       })
     })
 
     describe('when given S3 options', function () {
-      const httpOptions = { timeout: 2000 }
+      const httpOptions = { connectionTimeout: 2000 }
       const maxRetries = 2
 
       beforeEach(async function () {
         settings.httpOptions = httpOptions
         settings.maxRetries = maxRetries
+
+        S3.mockSend(S3.GetObjectCommand, {
+          Body: Readable.from('content'),
+          ContentEncoding: 'gzip',
+        })
+
         await S3Persistor.getObjectStream(bucket, key)
       })
 
-      it('configures the S3 client appropriately', function () {
+      it('configures the options and the requestHandler with NodeHttpHandler', function () {
-        expect(S3).to.have.been.calledWithMatch({ httpOptions, maxRetries })
+        expect(S3.S3Client).to.have.been.calledWithMatch({
+          requestHandler: sinon.match.any,
+          maxAttempts: maxRetries + 1,
+        })
+        expect(nodeHttpHandler.NodeHttpHandler).to.have.been.calledWithMatch(
+          httpOptions
+        )
       })
     })
 
@@ -327,7 +331,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3GetObjectRequest.statusCode = 404
+        S3.mockSend(S3.GetObjectCommand, S3NotFoundError, { rejects: true })
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -356,7 +360,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3GetObjectRequest.notFoundSSEC = 404
+        S3.mockSend(S3.GetObjectCommand, S3NotFoundError, { rejects: true })
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -384,7 +388,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3GetObjectRequest.statusCode = 403
+        S3.mockSend(S3.GetObjectCommand, S3AccessDeniedError, { rejects: true })
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -413,7 +417,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3GetObjectRequest.err = genericError
+        S3.mockSend(S3.GetObjectCommand, genericError, { rejects: true })
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -447,7 +451,7 @@ describe('S3PersistorTests', function () {
     })
 
     it('should request a signed URL', function () {
-      expect(S3Client.getSignedUrlPromise).to.have.been.called
+      expect(awsRequestPresigner.getSignedUrl).to.have.been.called
     })
 
     it('should return the url', function () {
@@ -460,6 +464,7 @@ describe('S3PersistorTests', function () {
       let size
 
       beforeEach(async function () {
+        S3.mockSend(S3.HeadObjectCommand, { ContentLength: objectSize })
         size = await S3Persistor.getObjectSize(bucket, key)
       })
 
@@ -468,7 +473,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('should pass the bucket and key to S3', function () {
-        expect(S3Client.headObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.HeadObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
@@ -479,9 +484,7 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3Client.headObject = sinon.stub().returns({
+        S3.mockSend(S3.HeadObjectCommand, S3NotFoundError, { rejects: true })
-          promise: sinon.stub().rejects(S3NotFoundError),
-        })
         try {
           await S3Persistor.getObjectSize(bucket, key)
         } catch (err) {
@@ -502,9 +505,7 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3Client.headObject = sinon.stub().returns({
+        S3.mockSend(S3.HeadObjectCommand, genericError, { rejects: true })
-          promise: sinon.stub().rejects(genericError),
-        })
         try {
           await S3Persistor.getObjectSize(bucket, key)
         } catch (err) {
@@ -525,19 +526,17 @@ describe('S3PersistorTests', function () {
   describe('sendStream', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        return S3Persistor.sendStream(bucket, key, ReadStream)
+        await S3Persistor.sendStream(bucket, key, ReadStream)
       })
 
-      it('should upload the stream', function () {
+      it('should upload the stream in a single part', function () {
-        expect(S3Client.upload).to.have.been.calledWith({
+        expect(awsLibStorageUpload).to.have.been.calledWith({
-          Bucket: bucket,
+          client: S3.s3ClientStub,
-          Key: key,
+          params: {
-          Body: sinon.match.instanceOf(Stream.Transform),
+            Bucket: bucket,
-        })
+            Key: key,
-      })
+            Body: sinon.match.instanceOf(Stream.Transform),
-
+          },
-      it('should upload files in a single part', function () {
-        expect(S3Client.upload).to.have.been.calledWith(sinon.match.any, {
           partSize: 100 * 1024 * 1024,
         })
       })
@@ -552,17 +551,21 @@ describe('S3PersistorTests', function () {
 
     describe('when a hash is supplied', function () {
       beforeEach(async function () {
-        return S3Persistor.sendStream(bucket, key, ReadStream, {
+        await S3Persistor.sendStream(bucket, key, ReadStream, {
           sourceMd5: 'aaaaaaaabbbbbbbbaaaaaaaabbbbbbbb',
         })
       })
 
       it('sends the hash in base64', function () {
-        expect(S3Client.upload).to.have.been.calledWith({
+        expect(awsLibStorageUpload).to.have.been.calledWith({
-          Bucket: bucket,
+          client: S3.s3ClientStub,
-          Key: key,
+          params: {
-          Body: sinon.match.instanceOf(Transform),
+            Bucket: bucket,
-          ContentMD5: 'qqqqqru7u7uqqqqqu7u7uw==',
+            Key: key,
+            Body: sinon.match.instanceOf(Transform),
+            ContentMD5: 'qqqqqru7u7uqqqqqu7u7uw==',
+          },
+          partSize: 100 * 1024 * 1024,
         })
       })
     })
@@ -572,19 +575,29 @@ describe('S3PersistorTests', function () {
       const contentEncoding = 'gzip'
 
       beforeEach(async function () {
-        return S3Persistor.sendStream(bucket, key, ReadStream, {
+        await S3Persistor.sendStream(bucket, key, ReadStream, {
           contentType,
           contentEncoding,
         })
       })
 
       it('sends the metadata to S3', function () {
-        expect(S3Client.upload).to.have.been.calledWith({
+        expect(awsLibStorageUpload).to.have.been.calledWith({
-          Bucket: bucket,
+          client: S3.s3ClientStub,
-          Key: key,
+          params: {
-          Body: sinon.match.instanceOf(Transform),
+            Bucket: bucket,
-          ContentType: contentType,
+            Key: key,
-          ContentEncoding: contentEncoding,
+            Body: sinon.match.instanceOf(Transform),
+            ContentType: contentType,
+            ContentEncoding: contentEncoding,
+          },
+          partSize: 100 * 1024 * 1024,
+        })
+      })
+
+      it('configures the options to not to retry requests', function () {
+        expect(S3.S3Client).not.to.have.been.calledWithMatch({
+          maxAttempts: sinon.match.number,
         })
       })
     })
@@ -592,9 +605,7 @@ describe('S3PersistorTests', function () {
     describe('when the upload fails', function () {
       let error
       beforeEach(async function () {
-        S3Client.upload = sinon.stub().returns({
+        awsLibStorageUpload.rejects(genericError)
-          promise: sinon.stub().rejects(genericError),
-        })
         try {
           await S3Persistor.sendStream(bucket, key, ReadStream)
         } catch (err) {
@@ -611,7 +622,8 @@ describe('S3PersistorTests', function () {
   describe('sendFile', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        return S3Persistor.sendFile(bucket, key, filename)
+        S3.s3ClientStub.send.resolves()
+        await S3Persistor.sendFile(bucket, key, filename)
       })
 
       it('should create a read stream for the file', function () {
@@ -619,10 +631,14 @@ describe('S3PersistorTests', function () {
       })
 
       it('should upload the stream', function () {
-        expect(S3Client.upload).to.have.been.calledWith({
+        expect(awsLibStorageUpload).to.have.been.calledWith({
-          Bucket: bucket,
+          client: S3.s3ClientStub,
-          Key: key,
+          params: {
-          Body: sinon.match.instanceOf(Transform),
+            Bucket: bucket,
+            Key: key,
+            Body: sinon.match.instanceOf(Transform),
+          },
+          partSize: settings.partSize,
         })
       })
     })
@@ -632,6 +648,10 @@ describe('S3PersistorTests', function () {
     describe('when the etag is a valid md5 hash', function () {
       let hash
       beforeEach(async function () {
+        S3.mockSend(S3.HeadObjectCommand, {
+          ContentLength: objectSize,
+          ETag: md5,
+        })
         hash = await S3Persistor.getObjectMd5Hash(bucket, key)
       })
 
@@ -640,33 +660,34 @@ describe('S3PersistorTests', function () {
       })
 
       it('should get the hash from the object metadata', function () {
-        expect(S3Client.headObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.HeadObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
       })
 
       it('should not download the object', function () {
-        expect(S3Client.getObject).not.to.have.been.called
+        S3.assertSendNotCalledWith(S3.GetObjectCommand)
       })
     })
 
     describe("when the etag isn't a valid md5 hash", function () {
       let hash
       beforeEach(async function () {
-        S3Client.headObject = sinon.stub().returns({
+        S3.mockSend(S3.GetObjectCommand, {
-          promise: sinon.stub().resolves({
+          ContentLength: objectSize,
-            ETag: 'somethingthatisntanmd5',
+          ETag: md5,
-            Bucket: bucket,
+        })
-            Key: key,
+        S3.mockSend(S3.HeadObjectCommand, {
-          }),
+          ETag: 'somethingthatisntanmd5',
+          Bucket: bucket,
+          Key: key,
         })
-
         hash = await S3Persistor.getObjectMd5Hash(bucket, key)
       })
 
       it('should re-fetch the file to verify it', function () {
-        expect(S3Client.getObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.GetObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
@@ -685,14 +706,15 @@ describe('S3PersistorTests', function () {
   describe('copyObject', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        return S3Persistor.copyObject(bucket, key, destKey)
+        S3.mockSend(S3.CopyObjectCommand)
+        await S3Persistor.copyObject(bucket, key, destKey)
       })
 
       it('should copy the object', function () {
-        expect(S3Client.copyObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.CopyObjectCommand, {
           Bucket: bucket,
           Key: destKey,
-          CopySource: `${bucket}/${key}`,
+          CopySource: `/${bucket}/${key}`,
         })
       })
     })
@@ -701,9 +723,7 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3Client.copyObject = sinon.stub().returns({
+        S3.mockSend(S3.CopyObjectCommand, S3NotFoundError, { rejects: true })
-          promise: sinon.stub().rejects(S3NotFoundError),
-        })
         try {
           await S3Persistor.copyObject(bucket, key, destKey)
         } catch (err) {
@@ -720,11 +740,12 @@ describe('S3PersistorTests', function () {
   describe('deleteObject', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        return S3Persistor.deleteObject(bucket, key)
+        S3.mockSend(S3.DeleteObjectCommand)
+        await S3Persistor.deleteObject(bucket, key)
       })
 
       it('should delete the object', function () {
-        expect(S3Client.deleteObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.DeleteObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
@@ -735,68 +756,76 @@ describe('S3PersistorTests', function () {
   describe('deleteDirectory', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        return S3Persistor.deleteDirectory(bucket, key)
+        S3.mockSend(S3.ListObjectsV2Command, { Contents: files })
+        await S3Persistor.deleteDirectory(bucket, key)
       })
 
       it('should list the objects in the directory', function () {
-        expect(S3Client.listObjectsV2).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
           Bucket: bucket,
           Prefix: key,
         })
       })
 
       it('should delete the objects using their keys', function () {
-        expect(S3Client.deleteObjects).to.have.been.calledWith({
+        S3.s3ClientStub.send.withArgs(new S3.DeleteObjectsCommand()).resolves()
-          Bucket: bucket,
+        S3.assertSendCalledWith(
-          Delete: {
+          S3.DeleteObjectsCommand,
-            Objects: [{ Key: 'llama' }, { Key: 'hippo' }],
+          {
-            Quiet: true,
+            Bucket: bucket,
+            Delete: {
+              Objects: [{ Key: 'llama' }, { Key: 'hippo' }],
+              Quiet: true,
+            },
           },
-        })
+          1
+        )
       })
     })
 
     describe('when there are no files', function () {
       beforeEach(async function () {
-        S3Client.listObjectsV2 = sinon
+        S3.mockSend(S3.ListObjectsV2Command, { Contents: [] })
-          .stub()
+        await S3Persistor.deleteDirectory(bucket, key)
-          .returns({ promise: sinon.stub().resolves({ Contents: [] }) })
-        return S3Persistor.deleteDirectory(bucket, key)
       })
 
       it('should list the objects in the directory', function () {
-        expect(S3Client.listObjectsV2).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
           Bucket: bucket,
           Prefix: key,
         })
       })
 
       it('should not try to delete any objects', function () {
-        expect(S3Client.deleteObjects).not.to.have.been.called
+        S3.assertSendNotCalledWith(S3.DeleteObjectsCommand)
       })
     })
 
     describe('when there are more files available', function () {
       const continuationToken = 'wombat'
       beforeEach(async function () {
-        S3Client.listObjectsV2.onCall(0).returns({
+        S3.mockSend(
-          promise: sinon.stub().resolves({
+          S3.ListObjectsV2Command,
+          {
             Contents: files,
             IsTruncated: true,
             NextContinuationToken: continuationToken,
-          }),
+          },
-        })
+          {
-
+            nextResponses: [{ Contents: [{ Key: 'last-file', Size: 33 }] }],
+          }
+        )
+        S3.mockSend(S3.DeleteObjectsCommand)
         return S3Persistor.deleteDirectory(bucket, key)
       })
 
       it('should list the objects a second time, with a continuation token', function () {
-        expect(S3Client.listObjectsV2).to.be.calledTwice
+        S3.assertSendCallCount(S3.ListObjectsV2Command, 2)
-        expect(S3Client.listObjectsV2).to.be.calledWith({
+        expect(S3.s3ClientStub.send.firstCall.args[0].payload).to.deep.equal({
           Bucket: bucket,
           Prefix: key,
         })
-        expect(S3Client.listObjectsV2).to.be.calledWith({
+        expect(S3.s3ClientStub.send.thirdCall.args[0].payload).to.deep.equal({
           Bucket: bucket,
           Prefix: key,
           ContinuationToken: continuationToken,
@@ -804,7 +833,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('should delete both sets of files', function () {
-        expect(S3Client.deleteObjects).to.have.been.calledTwice
+        S3.assertSendCallCount(S3.DeleteObjectsCommand, 2)
       })
     })
 
@@ -812,9 +841,7 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3Client.listObjectsV2 = sinon
+        S3.mockSend(S3.ListObjectsV2Command, genericError, { rejects: true })
-          .stub()
-          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.deleteDirectory(bucket, key)
         } catch (err) {
@@ -831,7 +858,8 @@ describe('S3PersistorTests', function () {
       })
 
       it('should not try to delete any objects', function () {
-        expect(S3Client.deleteObjects).not.to.have.been.called
+        // call count should be 1, only the ListObjectsV2Command tested above
+        expect(S3.s3ClientStub.send.callCount).to.equal(1)
       })
     })
 
@@ -839,9 +867,8 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3Client.deleteObjects = sinon
+        S3.mockSend(S3.ListObjectsV2Command, { Contents: files })
-          .stub()
+        S3.mockSend(S3.DeleteObjectsCommand, genericError, { rejects: true })
-          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.deleteDirectory(bucket, key)
         } catch (err) {
@@ -864,11 +891,12 @@ describe('S3PersistorTests', function () {
       let size
 
       beforeEach(async function () {
+        S3.mockSend(S3.ListObjectsV2Command, { Contents: files })
         size = await S3Persistor.directorySize(bucket, key)
       })
 
       it('should list the objects in the directory', function () {
-        expect(S3Client.listObjectsV2).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
           Bucket: bucket,
           Prefix: key,
         })
@@ -883,14 +911,12 @@ describe('S3PersistorTests', function () {
       let size
 
       beforeEach(async function () {
-        S3Client.listObjectsV2 = sinon
+        S3.mockSend(S3.ListObjectsV2Command, { Contents: [] })
-          .stub()
-          .returns({ promise: sinon.stub().resolves({ Contents: [] }) })
         size = await S3Persistor.directorySize(bucket, key)
       })
 
       it('should list the objects in the directory', function () {
-        expect(S3Client.listObjectsV2).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
           Bucket: bucket,
           Prefix: key,
         })
@@ -905,24 +931,28 @@ describe('S3PersistorTests', function () {
       const continuationToken = 'wombat'
       let size
       beforeEach(async function () {
-        S3Client.listObjectsV2.onCall(0).returns({
+        S3.mockSend(
-          promise: sinon.stub().resolves({
+          S3.ListObjectsV2Command,
+          {
             Contents: files,
             IsTruncated: true,
             NextContinuationToken: continuationToken,
-          }),
+          },
-        })
+          {
+            nextResponses: [{ Contents: [{ Key: 'last-file', Size: 33 }] }],
+          }
+        )
 
         size = await S3Persistor.directorySize(bucket, key)
       })
 
       it('should list the objects a second time, with a continuation token', function () {
-        expect(S3Client.listObjectsV2).to.be.calledTwice
+        S3.assertSendCallCount(S3.ListObjectsV2Command, 2)
-        expect(S3Client.listObjectsV2).to.be.calledWith({
+        expect(S3.s3ClientStub.send.firstCall.args[0].payload).to.deep.equal({
           Bucket: bucket,
           Prefix: key,
         })
-        expect(S3Client.listObjectsV2).to.be.calledWith({
+        expect(S3.s3ClientStub.send.secondCall.args[0].payload).to.deep.equal({
           Bucket: bucket,
           Prefix: key,
           ContinuationToken: continuationToken,
@@ -938,9 +968,7 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3Client.listObjectsV2 = sinon
+        S3.mockSend(S3.ListObjectsV2Command, genericError, { rejects: true })
-          .stub()
-          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.directorySize(bucket, key)
         } catch (err) {
@@ -963,11 +991,12 @@ describe('S3PersistorTests', function () {
       let exists
 
       beforeEach(async function () {
+        S3.mockSend(S3.HeadObjectCommand, { ContentLength: objectSize })
         exists = await S3Persistor.checkIfObjectExists(bucket, key)
       })
 
       it('should get the object header', function () {
-        expect(S3Client.headObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.HeadObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
@@ -982,14 +1011,13 @@ describe('S3PersistorTests', function () {
       let exists
 
       beforeEach(async function () {
-        S3Client.headObject = sinon
+        S3.mockSend(S3.HeadObjectCommand, S3NotFoundError, { rejects: true })
-          .stub()
+        S3.s3ClientStub.send.rejects(S3NotFoundError)
-          .returns({ promise: sinon.stub().rejects(S3NotFoundError) })
         exists = await S3Persistor.checkIfObjectExists(bucket, key)
       })
 
       it('should get the object header', function () {
-        expect(S3Client.headObject).to.have.been.calledWith({
+        S3.assertSendCalledWith(S3.HeadObjectCommand, {
           Bucket: bucket,
           Key: key,
         })
@@ -1004,9 +1032,7 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3Client.headObject = sinon
+        S3.mockSend(S3.HeadObjectCommand, genericError, { rejects: true })
-          .stub()
-          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.checkIfObjectExists(bucket, key)
         } catch (err) {
@@ -1027,22 +1053,4 @@ describe('S3PersistorTests', function () {
       })
     })
   })
-
-  describe('_getClientForBucket', function () {
-    it('should return same instance for same bucket', function () {
-      const a = S3Persistor._getClientForBucket('foo')
-      const b = S3Persistor._getClientForBucket('foo')
-      expect(a).to.equal(b)
-    })
-    it('should return different instance for different bucket', function () {
-      const a = S3Persistor._getClientForBucket('foo')
-      const b = S3Persistor._getClientForBucket('bar')
-      expect(a).to.not.equal(b)
-    })
-    it('should return different instance for same bucket different computeChecksums', function () {
-      const a = S3Persistor._getClientForBucket('foo', false)
-      const b = S3Persistor._getClientForBucket('foo', true)
-      expect(a).to.not.equal(b)
-    })
-  })
 })

server-ce/config/settings.js

@@ -367,6 +367,7 @@ if (process.env.OVERLEAF_EMAIL_FROM_ADDRESS != null) {
       // AWS Creds
       AWSAccessKeyID: process.env.OVERLEAF_EMAIL_AWS_SES_ACCESS_KEY_ID,
       AWSSecretKey: process.env.OVERLEAF_EMAIL_AWS_SES_SECRET_KEY,
+      region: process.env.OVERLEAF_EMAIL_AWS_SES_REGION || 'us-east-1',
 
       // SMTP Creds
       host: process.env.OVERLEAF_EMAIL_SMTP_HOST,
@@ -383,10 +384,6 @@ if (process.env.OVERLEAF_EMAIL_FROM_ADDRESS != null) {
     },
   }
 
-  if (process.env.OVERLEAF_EMAIL_AWS_SES_REGION != null) {
-    settings.email.parameters.region = process.env.OVERLEAF_EMAIL_AWS_SES_REGION
-  }
-
   if (
     process.env.OVERLEAF_EMAIL_SMTP_USER != null ||
     process.env.OVERLEAF_EMAIL_SMTP_PASS != null

services/filestore/package.json

@@ -19,6 +19,7 @@
     "types:check": "tsc --noEmit"
   },
   "dependencies": {
+    "@aws-sdk/client-s3": "^3.864.0",
     "@overleaf/logger": "*",
     "@overleaf/metrics": "*",
     "@overleaf/o-error": "*",

services/filestore/test/acceptance/js/FilestoreTests.js

@@ -13,6 +13,7 @@ const streamifier = require('streamifier')
 chai.use(require('chai-as-promised'))
 const { ObjectId } = require('mongodb')
 const ChildProcess = require('node:child_process')
+const { ListObjectsV2Command } = require('@aws-sdk/client-s3')
 
 const fsWriteFile = promisify(fs.writeFile)
 const fsStat = promisify(fs.stat)
@@ -517,19 +518,13 @@ describe('Filestore', function () {
               ...s3Config(),
               key: process.env.MINIO_ROOT_USER,
               secret: process.env.MINIO_ROOT_PASSWORD,
-            })._getClientForBucket(bucketName)
+            })
-            await s3
+            await s3._createBucket(bucketName)
-              .createBucket({
+            await s3._upload(bucketName, {
-                Bucket: bucketName,
+              Bucket: bucketName,
-              })
+              Key: fileId,
-              .promise()
+              Body: constantFileContent,
-            await s3
+            })
-              .upload({
-                Bucket: bucketName,
-                Key: fileId,
-                Body: constantFileContent,
-              })
-              .promise()
           })
 
           it('should get the file from the specified bucket', async function () {
@@ -1250,22 +1245,22 @@ describe('Filestore', function () {
         }) {
           await createRandomContent(fileUrl1)
 
-          const { Contents: dekEntries } = await s3Client
+          const { Contents: dekEntries } = await s3Client.send(
-            .listObjectsV2({
+            new ListObjectsV2Command({
               Bucket: process.env.AWS_S3_USER_FILES_DEK_BUCKET_NAME,
               Prefix: `${templateId}/`,
             })
-            .promise()
+          )
           expect(dekEntries).to.have.length(dekBucketKeys.length)
           // Order is not predictable, use members
           expect(dekEntries.map(o => o.Key)).to.have.members(dekBucketKeys)
 
-          const { Contents: userFilesEntries } = await s3Client
+          const { Contents: userFilesEntries } = await s3Client.send(
-            .listObjectsV2({
+            new ListObjectsV2Command({
               Bucket: backendSettings.stores.template_files,
               Prefix: `${templateId}/`,
             })
-            .promise()
+          )
           expect(userFilesEntries).to.have.length(userFilesBucketKeys.length)
           // Order is not predictable, use members
           expect(userFilesEntries.map(o => o.Key)).to.have.members(

services/history-v1/test/acceptance/js/api/backupDeletion.test.mjs

@@ -15,6 +15,7 @@ import { makeProjectKey } from '../../../../storage/lib/blob_store/index.js'
 import config from 'config'
 import Stream from 'stream'
 import projectKey from '../../../../storage/lib/project_key.js'
+import { ListObjectsV2Command } from '@aws-sdk/client-s3'
 
 /**
  * @typedef {import("node-fetch").Response} Response
@@ -32,9 +33,11 @@ const deletedProjectsCollection = db.collection('deletedProjects')
 async function listS3Bucket(bucket, prefix) {
   // @ts-ignore access to internal library helper
   const client = backupPersistor._getClientForBucket(bucket)
-  const response = await client
+
-    .listObjectsV2({ Bucket: bucket, Prefix: prefix })
+  const response = await client.send(
-    .promise()
+    new ListObjectsV2Command({ Bucket: bucket, Prefix: prefix })
+  )
+
   return (response.Contents || []).map(item => item.Key || '')
 }
 

services/history-v1/test/acceptance/js/storage/backup.test.mjs

@@ -35,6 +35,7 @@ import {
   chunksBucket,
 } from '../../../../storage/lib/backupPersistor.mjs'
 import { Readable } from 'node:stream'
+import { ListObjectsV2Command } from '@aws-sdk/client-s3'
 
 const projectsCollection = client.db().collection('projects')
 
@@ -583,11 +584,13 @@ describe('backup script', function () {
       // Get all chunks and verify they were backed up
       const listing = await backupPersistor
         ._getClientForBucket(chunksBucket)
-        .listObjectsV2({
+        .send(
-          Bucket: chunksBucket,
+          new ListObjectsV2Command({
-          Prefix: projectKey.format(historyId) + '/',
+            Bucket: chunksBucket,
-        })
+            Prefix: projectKey.format(historyId) + '/',
-        .promise()
+          })
+        )
+
       const chunkKeys = listing.Contents.map(item => item.Key)
       expect(chunkKeys.length).to.equal(6) // Should have multiple chunks
 

services/history-v1/test/acceptance/js/storage/backupBlob.test.mjs

@@ -25,10 +25,11 @@ import {
 } from '../../../../storage/lib/backupPersistor.mjs'
 import { WritableBuffer } from '@overleaf/stream-utils'
 import cleanup from './support/cleanup.js'
+import { ListObjectsV2Command } from '@aws-sdk/client-s3'
 
 async function listS3BucketRaw(bucket) {
   const client = backupPersistor._getClientForBucket(bucket)
-  return await client.listObjectsV2({ Bucket: bucket }).promise()
+  return await client.send(new ListObjectsV2Command({ Bucket: bucket }))
 }
 
 async function listS3Bucket(bucket, wantStorageClass) {

services/project-history/package.json

@@ -28,7 +28,6 @@
     "@overleaf/settings": "*",
     "@overleaf/stream-utils": "*",
     "async": "^3.2.5",
-    "aws-sdk": "^2.650.0",
     "body-parser": "^1.20.3",
     "bunyan": "^1.8.15",
     "celebrate": "^15.0.3",

services/web/app/src/Features/Email/EmailSender.js

@@ -3,7 +3,7 @@ const logger = require('@overleaf/logger')
 const metrics = require('@overleaf/metrics')
 const Settings = require('@overleaf/settings')
 const nodemailer = require('nodemailer')
-const sesTransport = require('nodemailer-ses-transport')
+const aws = require('@aws-sdk/client-ses')
 const OError = require('@overleaf/o-error')
 const { RateLimiter } = require('../../infrastructure/RateLimiter')
 const _ = require('lodash')
@@ -30,7 +30,15 @@ function getClient() {
     const emailParameters = EMAIL_SETTINGS.parameters
     if (emailParameters.AWSAccessKeyID || EMAIL_SETTINGS.driver === 'ses') {
       logger.debug('using aws ses for email')
-      client = nodemailer.createTransport(sesTransport(emailParameters))
+      const ses = new aws.SESClient({
+        apiVersion: '2010-12-01',
+        region: emailParameters.region,
+        credentials: {
+          accessKeyId: emailParameters.AWSAccessKeyID,
+          secretAccessKey: emailParameters.AWSSecretKey,
+        },
+      })
+      client = nodemailer.createTransport({ SES: { ses, aws } })
     } else if (emailParameters.sendgridApiKey) {
       throw new OError(
         'sendgridApiKey configuration option is deprecated, use SMTP instead'

services/web/package.json

@@ -76,6 +76,7 @@
     "safari > 14"
   ],
   "dependencies": {
+    "@aws-sdk/client-ses": "^3.864.0",
     "@contentful/rich-text-html-renderer": "^16.0.2",
     "@contentful/rich-text-types": "^16.0.2",
     "@google-cloud/bigquery": "^6.0.1",
@@ -151,7 +152,6 @@
     "nocache": "^2.1.0",
     "node-fetch": "^2.7.0",
     "nodemailer": "^6.7.0",
-    "nodemailer-ses-transport": "^1.5.1",
     "on-headers": "^1.0.2",
     "otplib": "^12.0.1",
     "p-limit": "^2.3.0",

services/web/test/unit/src/Email/EmailSenderTests.js

@@ -33,10 +33,12 @@ describe('EmailSender', function () {
 
     this.ses = { createTransport: () => this.sesClient }
 
+    this.SESClient = sinon.stub()
+
     this.EmailSender = SandboxedModule.require(MODULE_PATH, {
       requires: {
         nodemailer: this.ses,
-        'nodemailer-ses-transport': sinon.stub(),
+        '@aws-sdk/client-ses': { SESClient: this.SESClient },
         '@overleaf/settings': this.Settings,
         '../../infrastructure/RateLimiter': this.RateLimiter,
         '@overleaf/metrics': {