Browse Source

Revert "Merge pull request #27679 from overleaf/msm-aws-sdk-upgrade" (#28151)

This reverts commit 4989ae920d8b7fd9e79623947b7c40bcc2e56d92.

GitOrigin-RevId: 541d95bfeaa7ce820e8af67f646f013fe4fe5d21
Miguel Serrano 1 year ago
parent
commit
8948e40b22

+ 1 - 4
libraries/object-persistor/package.json

@@ -20,15 +20,12 @@
   "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",

+ 17 - 2
libraries/object-persistor/src/PerProjectEncryptedS3Persistor.js

@@ -20,6 +20,10 @@ const hkdf = promisify(Crypto.hkdf)
 
 const AES256_KEY_LENGTH = 32
 
+/**
+ * @typedef {import('aws-sdk').AWSError} AWSError
+ */
+
 /**
  * @typedef {Object} Settings
  * @property {boolean} automaticallyRotateDEKEncryption
@@ -33,6 +37,16 @@ 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}
@@ -41,8 +55,9 @@ function isForbiddenError(err) {
   if (!err || !(err instanceof ReadError || err instanceof NotFoundError)) {
     return false
   }
-  // @ts-ignore
-  return err?.cause.statusCode === 403 || err?.cause.Code === 'AccessDenied'
+  const cause = err.cause
+  if (!isAWSError(cause)) return false
+  return cause.statusCode === 403
 }
 
 class RootKeyEncryptionKey {

+ 2 - 7
libraries/object-persistor/src/PersistorHelper.js

@@ -144,22 +144,17 @@ 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(
-      errorCode
+      error.code
     ) ||
     (error.response && error.response.statusCode === 404)
   ) {
     return new NotFoundError('no such file', params, error)
   } else if (
     params.ifNoneMatch === '*' &&
-    (errorCode === 'PreconditionFailed' ||
+    (error.code === 'PreconditionFailed' ||
       error.response?.statusCode === 412 ||
       error instanceof AlreadyWrittenError)
   ) {

+ 0 - 49
libraries/object-persistor/src/S3Md5Middleware.js

@@ -1,49 +0,0 @@
-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,
-}

+ 79 - 107
libraries/object-persistor/src/S3Persistor.js

@@ -15,23 +15,18 @@ const PersistorHelper = require('./PersistorHelper')
 
 const { pipeline, PassThrough } = require('node:stream')
 const fs = require('node:fs')
-const {
-  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 S3 = require('aws-sdk/clients/s3')
+const { URL } = require('node:url')
 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
@@ -78,7 +73,7 @@ class SSECOptions {
 }
 
 class S3Persistor extends AbstractPersistor {
-  /** @type {Map<string, S3Client>} */
+  /** @type {Map<string, S3>} */
   #clients = new Map()
 
   constructor(settings = {}) {
@@ -122,7 +117,7 @@ class S3Persistor extends AbstractPersistor {
       // observer will catch errors, clean up and log a warning
       pipeline(readStream, observer, () => {})
 
-      /** @type {import('@aws-sdk/client-s3').PutObjectCommandInput} */
+      /** @type {S3.PutObjectRequest} */
       const uploadOptions = {
         Bucket: bucketName,
         Key: key,
@@ -159,16 +154,13 @@ class S3Persistor extends AbstractPersistor {
       }
 
       if (this.settings.disableMultiPartUpload) {
-        await this._getClientForBucket(bucketName, computeChecksums).send(
-          new PutObjectCommand(uploadOptions)
-        )
+        await this._getClientForBucket(bucketName, computeChecksums)
+          .putObject(uploadOptions)
+          .promise()
       } else {
-        const upload = new Upload({
-          client: this._getClientForBucket(bucketName, computeChecksums),
-          params: uploadOptions,
-          partSize: this.settings.partSize,
-        })
-        await upload.done()
+        await this._getClientForBucket(bucketName, computeChecksums)
+          .upload(uploadOptions, { partSize: this.settings.partSize })
+          .promise()
       }
     } catch (err) {
       throw PersistorHelper.wrapError(
@@ -193,7 +185,6 @@ class S3Persistor extends AbstractPersistor {
   async getObjectStream(bucketName, key, opts) {
     opts = opts || {}
 
-    /** @type {import('@aws-sdk/client-s3').GetObjectCommandInput} */
     const params = {
       Bucket: bucketName,
       Key: key,
@@ -209,18 +200,32 @@ class S3Persistor extends AbstractPersistor {
       bucket: bucketName,
     })
 
-    const abortController = new AbortController()
-    let stream, contentEncoding
+    const req = this._getClientForBucket(bucketName).getObject(params)
+    const stream = req.createReadStream()
+
+    let contentEncoding
     try {
-      const { Body, ContentEncoding } = await this._getClientForBucket(
-        bucketName
-      ).send(new GetObjectCommand(params), {
-        abortSignal: abortController.signal,
+      await new Promise((resolve, reject) => {
+        req.on('httpHeaders', (statusCode, headers) => {
+          switch (statusCode) {
+            case 200: // full response
+            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',
@@ -234,11 +239,8 @@ 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) {
-        abortController.abort()
-      }
+      if (err) req.abort()
     })
     return pass
   }
@@ -251,13 +253,13 @@ class S3Persistor extends AbstractPersistor {
   async getRedirectUrl(bucketName, key) {
     const expiresSeconds = Math.round(this.settings.signedUrlExpiryInMs / 1000)
     try {
-      return await getSignedUrl(
-        this._getClientForBucket(bucketName),
-        new GetObjectCommand({
+      return await this._getClientForBucket(bucketName).getSignedUrlPromise(
+        'getObject',
+        {
           Bucket: bucketName,
           Key: key,
-        }),
-        { expiresIn: expiresSeconds }
+          Expires: expiresSeconds,
+        }
       )
     } catch (err) {
       throw PersistorHelper.wrapError(
@@ -284,15 +286,15 @@ class S3Persistor extends AbstractPersistor {
     const objects = contents.map(item => ({ Key: item.Key || '' }))
     if (objects?.length) {
       try {
-        await this._getClientForBucket(bucketName).send(
-          new DeleteObjectsCommand({
+        await this._getClientForBucket(bucketName)
+          .deleteObjects({
             Bucket: bucketName,
             Delete: {
               Objects: objects,
               Quiet: true,
             },
           })
-        )
+          .promise()
       } catch (err) {
         throw PersistorHelper.wrapError(
           err,
@@ -327,9 +329,9 @@ class S3Persistor extends AbstractPersistor {
     }
 
     try {
-      response = await this._getClientForBucket(bucketName).send(
-        new ListObjectsV2Command(options)
-      )
+      response = await this._getClientForBucket(bucketName)
+        .listObjectsV2(options)
+        .promise()
     } catch (err) {
       throw PersistorHelper.wrapError(
         err,
@@ -347,7 +349,7 @@ class S3Persistor extends AbstractPersistor {
    * @param {string} key
    * @param {Object} opts
    * @param {SSECOptions} [opts.ssecOptions]
-   * @return {Promise<import('@aws-sdk/client-s3').HeadObjectOutput>}
+   * @return {Promise<S3.HeadObjectOutput>}
    */
   async #headObject(bucketName, key, opts = {}) {
     const params = { Bucket: bucketName, Key: key }
@@ -355,8 +357,9 @@ class S3Persistor extends AbstractPersistor {
       Object.assign(params, opts.ssecOptions.getGetOptions())
     }
     try {
-      const client = await this._getClientForBucket(bucketName)
-      return await client.send(new HeadObjectCommand(params))
+      return await this._getClientForBucket(bucketName)
+        .headObject(params)
+        .promise()
     } catch (err) {
       throw PersistorHelper.wrapError(
         err,
@@ -430,9 +433,9 @@ class S3Persistor extends AbstractPersistor {
    */
   async deleteObject(bucketName, key) {
     try {
-      await this._getClientForBucket(bucketName).send(
-        new DeleteObjectCommand({ Bucket: bucketName, Key: key })
-      )
+      await this._getClientForBucket(bucketName)
+        .deleteObject({ Bucket: bucketName, Key: key })
+        .promise()
     } catch (err) {
       // s3 does not give us a NotFoundError here
       throw PersistorHelper.wrapError(
@@ -457,7 +460,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())
@@ -466,9 +469,7 @@ class S3Persistor extends AbstractPersistor {
       Object.assign(params, opts.ssecOptions.getPutOptions())
     }
     try {
-      await this._getClientForBucket(bucketName).send(
-        new CopyObjectCommand(params)
-      )
+      await this._getClientForBucket(bucketName).copyObject(params).promise()
     } catch (err) {
       throw PersistorHelper.wrapError(
         err,
@@ -518,9 +519,9 @@ class S3Persistor extends AbstractPersistor {
       if (continuationToken) {
         options.ContinuationToken = continuationToken
       }
-      const response = await this._getClientForBucket(bucketName).send(
-        new ListObjectsV2Command(options)
-      )
+      const response = await this._getClientForBucket(bucketName)
+        .listObjectsV2(options)
+        .promise()
 
       const size =
         response.Contents?.reduce((acc, item) => (item.Size || 0) + acc, 0) || 0
@@ -548,38 +549,33 @@ class S3Persistor extends AbstractPersistor {
   /**
    * @param {string} bucket
    * @param {boolean} computeChecksums
-   * @return {S3Client}
+   * @return {S3}
    * @private
    */
   _getClientForBucket(bucket, computeChecksums = false) {
-    /** @type {import('@aws-sdk/client-s3').S3ClientConfig} */
+    /** @type {S3.Types.ClientConfiguration} */
     const clientOptions = {}
     const cacheKey = `${bucket}:${computeChecksums}`
     if (computeChecksums) {
-      clientOptions.requestChecksumCalculation = 'WHEN_SUPPORTED'
-      clientOptions.responseChecksumValidation = 'WHEN_SUPPORTED'
+      clientOptions.computeChecksums = true
     }
     let client = this.#clients.get(cacheKey)
     if (!client) {
-      client = new S3Client(
+      client = new S3(
         this._buildClientOptions(
           this.settings.bucketCreds?.[bucket],
           clientOptions
         )
       )
       this.#clients.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 {import('@aws-sdk/client-s3').S3ClientConfig} clientOptions
-   * @return {import('@aws-sdk/client-s3').S3ClientConfig}
+   * @param {S3.Types.ClientConfiguration} clientOptions
+   * @return {S3.Types.ClientConfiguration}
    * @private
    */
   _buildClientOptions(bucketCredentials, clientOptions) {
@@ -597,63 +593,39 @@ class S3Persistor extends AbstractPersistor {
       }
     } else {
       // Use the default credentials provider (process.env -> SSP -> ini -> IAM)
-      // Docs: https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/setting-credentials-node.html
+      // Docs: https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/CredentialProviderChain.html#defaultProviders-property
     }
 
-    let sslEnabled = false
     if (this.settings.endpoint) {
       const endpoint = new URL(this.settings.endpoint)
       options.endpoint = this.settings.endpoint
-      sslEnabled = endpoint.protocol === 'https:'
+      options.sslEnabled = endpoint.protocol === 'https:'
     }
 
     // path-style access is only used for acceptance tests
     if (this.settings.pathStyle) {
-      options.forcePathStyle = true
+      options.s3ForcePathStyle = true
     }
 
-    for (const opt of ['httpOptions', 'region']) {
+    for (const opt of ['httpOptions', 'maxRetries', 'region']) {
       if (this.settings[opt]) {
         options[opt] = this.settings[opt]
       }
     }
 
-    // maxRetries has been moved to maxAttempts in aws-sdk v3,
-    // we're keeping the existing setting
-    if (this.settings.maxRetries) {
-      options.maxAttempts = this.settings.maxRetries + 1
-    }
-
-    if (sslEnabled && this.settings.ca) {
-      const agent = new https.Agent({
+    if (options.sslEnabled && this.settings.ca && !options.httpOptions?.agent) {
+      options.httpOptions = options.httpOptions || {}
+      options.httpOptions.agent = new https.Agent({
         rejectUnauthorized: true,
         ca: this.settings.ca,
       })
-      options.requestHandler = new NodeHttpHandler({
-        httpAgent: agent,
-        httpsAgent: agent,
-      })
     }
 
     return options
   }
 
-  // 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
+   * @param {S3.HeadObjectOutput} response
    * @return {string|null}
    * @private
    */

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

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

+ 0 - 112
libraries/object-persistor/test/unit/S3ClientMock.js

@@ -1,112 +0,0 @@
-/* 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
-      }
-    },
-  }
-}

+ 216 - 210
libraries/object-persistor/test/unit/S3PersistorTests.js

@@ -4,8 +4,6 @@ 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'
 
@@ -36,18 +34,17 @@ describe('S3PersistorTests', function () {
     Transform,
     PassThrough,
     S3,
-    awsRequestPresigner,
-    awsLibStorage,
-    awsLibStorageUpload,
-    abortSignal,
     Fs,
     ReadStream,
     Stream,
     StreamPromises,
+    S3GetObjectRequest,
     S3Persistor,
+    S3Client,
     S3NotFoundError,
     S3AccessDeniedError,
     FileNotFoundError,
+    EmptyPromise,
     settings,
     Hash,
     crypto
@@ -75,7 +72,46 @@ 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'
@@ -85,23 +121,33 @@ describe('S3PersistorTests', function () {
     }
 
     S3NotFoundError = new Error('not found')
-    S3NotFoundError.name = 'NoSuchKey'
+    S3NotFoundError.code = 'NoSuchKey'
 
     S3AccessDeniedError = new Error('access denied')
     S3AccessDeniedError.code = 'AccessDenied'
 
-    S3 = mockS3()
-
-    awsLibStorageUpload = sinon.stub().returns({
-      done: sinon.stub().resolves(),
-    })
-
-    awsLibStorage = {
-      Upload: awsLibStorageUpload,
-    }
-    awsRequestPresigner = {
-      getSignedUrl: sinon.stub().resolves(redirectUrl),
+    S3Client = {
+      getObject: sinon.stub().returns(S3GetObjectRequest),
+      headObject: sinon.stub().returns({
+        promise: sinon.stub().resolves({
+          ContentLength: objectSize,
+          ETag: md5,
+        }),
+      }),
+      listObjectsV2: sinon.stub().returns({
+        promise: sinon.stub().resolves({
+          Contents: files,
+        }),
+      }),
+      upload: 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(),
@@ -116,29 +162,17 @@ 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/client-s3': S3,
-        '@aws-sdk/lib-storage': awsLibStorage,
-        '@aws-sdk/s3-request-presigner': awsRequestPresigner,
+        'aws-sdk/clients/s3': S3,
         '@overleaf/logger': Logger,
-        '@aws-sdk/node-http-handler': {
-          NodeHttpHandler: sinon.stub(),
-        },
         './Errors': Errors,
         fs: Fs,
         stream: Stream,
         'stream/promises': StreamPromises,
         crypto,
       },
-      globals: { console, Buffer, AbortController: AbortCtrl },
+      globals: { console, Buffer },
     }).S3Persistor)(settings)
   })
 
@@ -147,10 +181,6 @@ describe('S3PersistorTests', function () {
       let stream
 
       beforeEach(async function () {
-        S3.mockSend(S3.GetObjectCommand, {
-          Body: Readable.from('content'),
-          ContentEncoding: 'gzip',
-        })
         stream = await S3Persistor.getObjectStream(bucket, key)
       })
 
@@ -158,8 +188,12 @@ 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 () {
-        S3.assertSendCalledWith(S3.GetObjectCommand, {
+        expect(S3Client.getObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
@@ -167,14 +201,14 @@ describe('S3PersistorTests', function () {
 
       it('pipes the stream through the meter', async function () {
         expect(Stream.pipeline).to.have.been.calledWith(
-          sinon.match.instanceOf(Readable),
+          ReadStream,
           sinon.match.instanceOf(Transform),
           sinon.match.instanceOf(PassThrough)
         )
       })
 
       it('does not abort the request', function () {
-        expect(abortSignal).not.to.have.been.called
+        expect(S3GetObjectRequest.aborted).to.equal(false)
       })
     })
 
@@ -182,10 +216,6 @@ 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,
@@ -197,7 +227,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('passes the byte range on to S3', function () {
-        S3.assertSendCalledWith(S3.GetObjectCommand, {
+        expect(S3Client.getObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
           Range: 'bytes=5-10',
@@ -209,10 +239,6 @@ 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)
       })
@@ -222,7 +248,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('aborts the request', function () {
-        expect(abortSignal).to.have.been.calledOnce
+        expect(S3GetObjectRequest.aborted).to.equal(true)
       })
     })
 
@@ -238,10 +264,6 @@ describe('S3PersistorTests', function () {
       }
 
       beforeEach(async function () {
-        S3.mockSend(S3.GetObjectCommand, {
-          Body: Readable.from('content'),
-          ContentEncoding: 'gzip',
-        })
         settings.bucketCreds = {}
         settings.bucketCreds[bucket] = {
           auth_key: alternativeKey,
@@ -256,11 +278,11 @@ describe('S3PersistorTests', function () {
       })
 
       it('sets the AWS client up with the alternative credentials', function () {
-        expect(S3.S3Client).to.have.been.calledWith(alternativeS3Credentials)
+        expect(S3).to.have.been.calledWith(alternativeS3Credentials)
       })
 
       it('fetches the right key from the right bucket', function () {
-        S3.assertSendCalledWith(S3.GetObjectCommand, {
+        expect(S3Client.getObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
@@ -269,13 +291,9 @@ describe('S3PersistorTests', function () {
       it('uses the default credentials for an unknown bucket', async function () {
         stream = await S3Persistor.getObjectStream('anotherBucket', key)
 
-        expect(S3.S3Client).to.have.been.calledTwice
-        expect(S3.S3Client.firstCall).to.have.been.calledWith(
-          alternativeS3Credentials
-        )
-        expect(S3.S3Client.secondCall).to.have.been.calledWith(
-          defaultS3Credentials
-        )
+        expect(S3).to.have.been.calledTwice
+        expect(S3.firstCall).to.have.been.calledWith(alternativeS3Credentials)
+        expect(S3.secondCall).to.have.been.calledWith(defaultS3Credentials)
       })
     })
 
@@ -284,14 +302,9 @@ 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.S3Client).to.have.been.calledOnce
-        expect(S3.S3Client.args[0].credentials).to.not.exist
+        expect(S3).to.have.been.calledOnce
+        expect(S3.args[0].credentials).to.not.exist
       })
     })
 
@@ -302,20 +315,11 @@ describe('S3PersistorTests', function () {
       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 () {
-        expect(S3.S3Client).to.have.been.calledWithMatch({
-          httpOptions,
-          maxAttempts: maxRetries + 1,
-        })
+        expect(S3).to.have.been.calledWithMatch({ httpOptions, maxRetries })
       })
     })
 
@@ -323,7 +327,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3.mockSend(S3.GetObjectCommand, S3NotFoundError, { rejects: true })
+        S3GetObjectRequest.statusCode = 404
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -352,7 +356,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3.mockSend(S3.GetObjectCommand, S3NotFoundError, { rejects: true })
+        S3GetObjectRequest.notFoundSSEC = 404
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -380,7 +384,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3.mockSend(S3.GetObjectCommand, S3AccessDeniedError, { rejects: true })
+        S3GetObjectRequest.statusCode = 403
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -409,7 +413,7 @@ describe('S3PersistorTests', function () {
       let error, stream
 
       beforeEach(async function () {
-        S3.mockSend(S3.GetObjectCommand, genericError, { rejects: true })
+        S3GetObjectRequest.err = genericError
         try {
           stream = await S3Persistor.getObjectStream(bucket, key)
         } catch (err) {
@@ -443,7 +447,7 @@ describe('S3PersistorTests', function () {
     })
 
     it('should request a signed URL', function () {
-      expect(awsRequestPresigner.getSignedUrl).to.have.been.called
+      expect(S3Client.getSignedUrlPromise).to.have.been.called
     })
 
     it('should return the url', function () {
@@ -456,7 +460,6 @@ describe('S3PersistorTests', function () {
       let size
 
       beforeEach(async function () {
-        S3.mockSend(S3.HeadObjectCommand, { ContentLength: objectSize })
         size = await S3Persistor.getObjectSize(bucket, key)
       })
 
@@ -465,7 +468,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('should pass the bucket and key to S3', function () {
-        S3.assertSendCalledWith(S3.HeadObjectCommand, {
+        expect(S3Client.headObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
@@ -476,7 +479,9 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3.mockSend(S3.HeadObjectCommand, S3NotFoundError, { rejects: true })
+        S3Client.headObject = sinon.stub().returns({
+          promise: sinon.stub().rejects(S3NotFoundError),
+        })
         try {
           await S3Persistor.getObjectSize(bucket, key)
         } catch (err) {
@@ -497,7 +502,9 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3.mockSend(S3.HeadObjectCommand, genericError, { rejects: true })
+        S3Client.headObject = sinon.stub().returns({
+          promise: sinon.stub().rejects(genericError),
+        })
         try {
           await S3Persistor.getObjectSize(bucket, key)
         } catch (err) {
@@ -518,17 +525,19 @@ describe('S3PersistorTests', function () {
   describe('sendStream', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        await S3Persistor.sendStream(bucket, key, ReadStream)
+        return S3Persistor.sendStream(bucket, key, ReadStream)
       })
 
-      it('should upload the stream in a single part', function () {
-        expect(awsLibStorageUpload).to.have.been.calledWith({
-          client: S3.s3ClientStub,
-          params: {
-            Bucket: bucket,
-            Key: key,
-            Body: sinon.match.instanceOf(Stream.Transform),
-          },
+      it('should upload the stream', function () {
+        expect(S3Client.upload).to.have.been.calledWith({
+          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,
         })
       })
@@ -543,21 +552,17 @@ describe('S3PersistorTests', function () {
 
     describe('when a hash is supplied', function () {
       beforeEach(async function () {
-        await S3Persistor.sendStream(bucket, key, ReadStream, {
+        return S3Persistor.sendStream(bucket, key, ReadStream, {
           sourceMd5: 'aaaaaaaabbbbbbbbaaaaaaaabbbbbbbb',
         })
       })
 
       it('sends the hash in base64', function () {
-        expect(awsLibStorageUpload).to.have.been.calledWith({
-          client: S3.s3ClientStub,
-          params: {
-            Bucket: bucket,
-            Key: key,
-            Body: sinon.match.instanceOf(Transform),
-            ContentMD5: 'qqqqqru7u7uqqqqqu7u7uw==',
-          },
-          partSize: 100 * 1024 * 1024,
+        expect(S3Client.upload).to.have.been.calledWith({
+          Bucket: bucket,
+          Key: key,
+          Body: sinon.match.instanceOf(Transform),
+          ContentMD5: 'qqqqqru7u7uqqqqqu7u7uw==',
         })
       })
     })
@@ -567,23 +572,19 @@ describe('S3PersistorTests', function () {
       const contentEncoding = 'gzip'
 
       beforeEach(async function () {
-        await S3Persistor.sendStream(bucket, key, ReadStream, {
+        return S3Persistor.sendStream(bucket, key, ReadStream, {
           contentType,
           contentEncoding,
         })
       })
 
       it('sends the metadata to S3', function () {
-        expect(awsLibStorageUpload).to.have.been.calledWith({
-          client: S3.s3ClientStub,
-          params: {
-            Bucket: bucket,
-            Key: key,
-            Body: sinon.match.instanceOf(Transform),
-            ContentType: contentType,
-            ContentEncoding: contentEncoding,
-          },
-          partSize: 100 * 1024 * 1024,
+        expect(S3Client.upload).to.have.been.calledWith({
+          Bucket: bucket,
+          Key: key,
+          Body: sinon.match.instanceOf(Transform),
+          ContentType: contentType,
+          ContentEncoding: contentEncoding,
         })
       })
     })
@@ -591,7 +592,9 @@ describe('S3PersistorTests', function () {
     describe('when the upload fails', function () {
       let error
       beforeEach(async function () {
-        awsLibStorageUpload.rejects(genericError)
+        S3Client.upload = sinon.stub().returns({
+          promise: sinon.stub().rejects(genericError),
+        })
         try {
           await S3Persistor.sendStream(bucket, key, ReadStream)
         } catch (err) {
@@ -608,8 +611,7 @@ describe('S3PersistorTests', function () {
   describe('sendFile', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        S3.s3ClientStub.send.resolves()
-        await S3Persistor.sendFile(bucket, key, filename)
+        return S3Persistor.sendFile(bucket, key, filename)
       })
 
       it('should create a read stream for the file', function () {
@@ -617,14 +619,10 @@ describe('S3PersistorTests', function () {
       })
 
       it('should upload the stream', function () {
-        expect(awsLibStorageUpload).to.have.been.calledWith({
-          client: S3.s3ClientStub,
-          params: {
-            Bucket: bucket,
-            Key: key,
-            Body: sinon.match.instanceOf(Transform),
-          },
-          partSize: settings.partSize,
+        expect(S3Client.upload).to.have.been.calledWith({
+          Bucket: bucket,
+          Key: key,
+          Body: sinon.match.instanceOf(Transform),
         })
       })
     })
@@ -634,10 +632,6 @@ 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)
       })
 
@@ -646,34 +640,33 @@ describe('S3PersistorTests', function () {
       })
 
       it('should get the hash from the object metadata', function () {
-        S3.assertSendCalledWith(S3.HeadObjectCommand, {
+        expect(S3Client.headObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
       })
 
       it('should not download the object', function () {
-        S3.assertSendNotCalledWith(S3.GetObjectCommand)
+        expect(S3Client.getObject).not.to.have.been.called
       })
     })
 
     describe("when the etag isn't a valid md5 hash", function () {
       let hash
       beforeEach(async function () {
-        S3.mockSend(S3.GetObjectCommand, {
-          ContentLength: objectSize,
-          ETag: md5,
-        })
-        S3.mockSend(S3.HeadObjectCommand, {
-          ETag: 'somethingthatisntanmd5',
-          Bucket: bucket,
-          Key: key,
+        S3Client.headObject = sinon.stub().returns({
+          promise: sinon.stub().resolves({
+            ETag: 'somethingthatisntanmd5',
+            Bucket: bucket,
+            Key: key,
+          }),
         })
+
         hash = await S3Persistor.getObjectMd5Hash(bucket, key)
       })
 
       it('should re-fetch the file to verify it', function () {
-        S3.assertSendCalledWith(S3.GetObjectCommand, {
+        expect(S3Client.getObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
@@ -692,15 +685,14 @@ describe('S3PersistorTests', function () {
   describe('copyObject', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        S3.mockSend(S3.CopyObjectCommand)
-        await S3Persistor.copyObject(bucket, key, destKey)
+        return S3Persistor.copyObject(bucket, key, destKey)
       })
 
       it('should copy the object', function () {
-        S3.assertSendCalledWith(S3.CopyObjectCommand, {
+        expect(S3Client.copyObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: destKey,
-          CopySource: `/${bucket}/${key}`,
+          CopySource: `${bucket}/${key}`,
         })
       })
     })
@@ -709,7 +701,9 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3.mockSend(S3.CopyObjectCommand, S3NotFoundError, { rejects: true })
+        S3Client.copyObject = sinon.stub().returns({
+          promise: sinon.stub().rejects(S3NotFoundError),
+        })
         try {
           await S3Persistor.copyObject(bucket, key, destKey)
         } catch (err) {
@@ -726,12 +720,11 @@ describe('S3PersistorTests', function () {
   describe('deleteObject', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        S3.mockSend(S3.DeleteObjectCommand)
-        await S3Persistor.deleteObject(bucket, key)
+        return S3Persistor.deleteObject(bucket, key)
       })
 
       it('should delete the object', function () {
-        S3.assertSendCalledWith(S3.DeleteObjectCommand, {
+        expect(S3Client.deleteObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
@@ -742,76 +735,68 @@ describe('S3PersistorTests', function () {
   describe('deleteDirectory', function () {
     describe('with valid parameters', function () {
       beforeEach(async function () {
-        S3.mockSend(S3.ListObjectsV2Command, { Contents: files })
-        await S3Persistor.deleteDirectory(bucket, key)
+        return S3Persistor.deleteDirectory(bucket, key)
       })
 
       it('should list the objects in the directory', function () {
-        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
+        expect(S3Client.listObjectsV2).to.have.been.calledWith({
           Bucket: bucket,
           Prefix: key,
         })
       })
 
       it('should delete the objects using their keys', function () {
-        S3.s3ClientStub.send.withArgs(new S3.DeleteObjectsCommand()).resolves()
-        S3.assertSendCalledWith(
-          S3.DeleteObjectsCommand,
-          {
-            Bucket: bucket,
-            Delete: {
-              Objects: [{ Key: 'llama' }, { Key: 'hippo' }],
-              Quiet: true,
-            },
+        expect(S3Client.deleteObjects).to.have.been.calledWith({
+          Bucket: bucket,
+          Delete: {
+            Objects: [{ Key: 'llama' }, { Key: 'hippo' }],
+            Quiet: true,
           },
-          1
-        )
+        })
       })
     })
 
     describe('when there are no files', function () {
       beforeEach(async function () {
-        S3.mockSend(S3.ListObjectsV2Command, { Contents: [] })
-        await S3Persistor.deleteDirectory(bucket, key)
+        S3Client.listObjectsV2 = sinon
+          .stub()
+          .returns({ promise: sinon.stub().resolves({ Contents: [] }) })
+        return S3Persistor.deleteDirectory(bucket, key)
       })
 
       it('should list the objects in the directory', function () {
-        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
+        expect(S3Client.listObjectsV2).to.have.been.calledWith({
           Bucket: bucket,
           Prefix: key,
         })
       })
 
       it('should not try to delete any objects', function () {
-        S3.assertSendNotCalledWith(S3.DeleteObjectsCommand)
+        expect(S3Client.deleteObjects).not.to.have.been.called
       })
     })
 
     describe('when there are more files available', function () {
       const continuationToken = 'wombat'
       beforeEach(async function () {
-        S3.mockSend(
-          S3.ListObjectsV2Command,
-          {
+        S3Client.listObjectsV2.onCall(0).returns({
+          promise: sinon.stub().resolves({
             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 () {
-        S3.assertSendCallCount(S3.ListObjectsV2Command, 2)
-        expect(S3.s3ClientStub.send.firstCall.args[0].payload).to.deep.equal({
+        expect(S3Client.listObjectsV2).to.be.calledTwice
+        expect(S3Client.listObjectsV2).to.be.calledWith({
           Bucket: bucket,
           Prefix: key,
         })
-        expect(S3.s3ClientStub.send.thirdCall.args[0].payload).to.deep.equal({
+        expect(S3Client.listObjectsV2).to.be.calledWith({
           Bucket: bucket,
           Prefix: key,
           ContinuationToken: continuationToken,
@@ -819,7 +804,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('should delete both sets of files', function () {
-        S3.assertSendCallCount(S3.DeleteObjectsCommand, 2)
+        expect(S3Client.deleteObjects).to.have.been.calledTwice
       })
     })
 
@@ -827,7 +812,9 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3.mockSend(S3.ListObjectsV2Command, genericError, { rejects: true })
+        S3Client.listObjectsV2 = sinon
+          .stub()
+          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.deleteDirectory(bucket, key)
         } catch (err) {
@@ -844,8 +831,7 @@ describe('S3PersistorTests', function () {
       })
 
       it('should not try to delete any objects', function () {
-        // call count should be 1, only the ListObjectsV2Command tested above
-        expect(S3.s3ClientStub.send.callCount).to.equal(1)
+        expect(S3Client.deleteObjects).not.to.have.been.called
       })
     })
 
@@ -853,8 +839,9 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3.mockSend(S3.ListObjectsV2Command, { Contents: files })
-        S3.mockSend(S3.DeleteObjectsCommand, genericError, { rejects: true })
+        S3Client.deleteObjects = sinon
+          .stub()
+          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.deleteDirectory(bucket, key)
         } catch (err) {
@@ -877,12 +864,11 @@ 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 () {
-        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
+        expect(S3Client.listObjectsV2).to.have.been.calledWith({
           Bucket: bucket,
           Prefix: key,
         })
@@ -897,12 +883,14 @@ describe('S3PersistorTests', function () {
       let size
 
       beforeEach(async function () {
-        S3.mockSend(S3.ListObjectsV2Command, { Contents: [] })
+        S3Client.listObjectsV2 = sinon
+          .stub()
+          .returns({ promise: sinon.stub().resolves({ Contents: [] }) })
         size = await S3Persistor.directorySize(bucket, key)
       })
 
       it('should list the objects in the directory', function () {
-        S3.assertSendCalledWith(S3.ListObjectsV2Command, {
+        expect(S3Client.listObjectsV2).to.have.been.calledWith({
           Bucket: bucket,
           Prefix: key,
         })
@@ -917,28 +905,24 @@ describe('S3PersistorTests', function () {
       const continuationToken = 'wombat'
       let size
       beforeEach(async function () {
-        S3.mockSend(
-          S3.ListObjectsV2Command,
-          {
+        S3Client.listObjectsV2.onCall(0).returns({
+          promise: sinon.stub().resolves({
             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 () {
-        S3.assertSendCallCount(S3.ListObjectsV2Command, 2)
-        expect(S3.s3ClientStub.send.firstCall.args[0].payload).to.deep.equal({
+        expect(S3Client.listObjectsV2).to.be.calledTwice
+        expect(S3Client.listObjectsV2).to.be.calledWith({
           Bucket: bucket,
           Prefix: key,
         })
-        expect(S3.s3ClientStub.send.secondCall.args[0].payload).to.deep.equal({
+        expect(S3Client.listObjectsV2).to.be.calledWith({
           Bucket: bucket,
           Prefix: key,
           ContinuationToken: continuationToken,
@@ -954,7 +938,9 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3.mockSend(S3.ListObjectsV2Command, genericError, { rejects: true })
+        S3Client.listObjectsV2 = sinon
+          .stub()
+          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.directorySize(bucket, key)
         } catch (err) {
@@ -977,12 +963,11 @@ 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 () {
-        S3.assertSendCalledWith(S3.HeadObjectCommand, {
+        expect(S3Client.headObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
@@ -997,13 +982,14 @@ describe('S3PersistorTests', function () {
       let exists
 
       beforeEach(async function () {
-        S3.mockSend(S3.HeadObjectCommand, S3NotFoundError, { rejects: true })
-        S3.s3ClientStub.send.rejects(S3NotFoundError)
+        S3Client.headObject = sinon
+          .stub()
+          .returns({ promise: sinon.stub().rejects(S3NotFoundError) })
         exists = await S3Persistor.checkIfObjectExists(bucket, key)
       })
 
       it('should get the object header', function () {
-        S3.assertSendCalledWith(S3.HeadObjectCommand, {
+        expect(S3Client.headObject).to.have.been.calledWith({
           Bucket: bucket,
           Key: key,
         })
@@ -1018,7 +1004,9 @@ describe('S3PersistorTests', function () {
       let error
 
       beforeEach(async function () {
-        S3.mockSend(S3.HeadObjectCommand, genericError, { rejects: true })
+        S3Client.headObject = sinon
+          .stub()
+          .returns({ promise: sinon.stub().rejects(genericError) })
         try {
           await S3Persistor.checkIfObjectExists(bucket, key)
         } catch (err) {
@@ -1039,4 +1027,22 @@ 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)
+    })
+  })
 })

File diff suppressed because it is too large
+ 333 - 6147
package-lock.json


+ 4 - 1
server-ce/config/settings.js

@@ -367,7 +367,6 @@ 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,
@@ -384,6 +383,10 @@ 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

+ 0 - 1
services/filestore/docker-compose.ci.yml

@@ -27,7 +27,6 @@ services:
       AWS_S3_PATH_STYLE: 'true'
       AWS_ACCESS_KEY_ID: OVERLEAF_FILESTORE_S3_ACCESS_KEY_ID
       AWS_SECRET_ACCESS_KEY: OVERLEAF_FILESTORE_S3_SECRET_ACCESS_KEY
-      AWS_REGION: us-east-1
       MINIO_ROOT_USER: MINIO_ROOT_USER
       MINIO_ROOT_PASSWORD: MINIO_ROOT_PASSWORD
       GCS_API_ENDPOINT: http://gcs:9090

+ 0 - 1
services/filestore/docker-compose.yml

@@ -44,7 +44,6 @@ services:
       AWS_S3_PATH_STYLE: 'true'
       AWS_ACCESS_KEY_ID: OVERLEAF_FILESTORE_S3_ACCESS_KEY_ID
       AWS_SECRET_ACCESS_KEY: OVERLEAF_FILESTORE_S3_SECRET_ACCESS_KEY
-      AWS_REGION: us-east-1
       MINIO_ROOT_USER: MINIO_ROOT_USER
       MINIO_ROOT_PASSWORD: MINIO_ROOT_PASSWORD
       GCS_API_ENDPOINT: http://gcs:9090

+ 0 - 1
services/filestore/package.json

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

+ 19 - 14
services/filestore/test/acceptance/js/FilestoreTests.js

@@ -13,7 +13,6 @@ 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)
@@ -533,13 +532,19 @@ describe('Filestore', function () {
               ...s3Config(),
               key: process.env.MINIO_ROOT_USER,
               secret: process.env.MINIO_ROOT_PASSWORD,
-            })
-            await s3._createBucket(bucketName)
-            await s3._upload(bucketName, {
-              Bucket: bucketName,
-              Key: fileId,
-              Body: constantFileContent,
-            })
+            })._getClientForBucket(bucketName)
+            await s3
+              .createBucket({
+                Bucket: bucketName,
+              })
+              .promise()
+            await s3
+              .upload({
+                Bucket: bucketName,
+                Key: fileId,
+                Body: constantFileContent,
+              })
+              .promise()
           })
 
           it('should get the file from the specified bucket', async function () {
@@ -1269,22 +1274,22 @@ describe('Filestore', function () {
         }) {
           await createRandomContent(fileUrl1)
 
-          const { Contents: dekEntries } = await s3Client.send(
-            new ListObjectsV2Command({
+          const { Contents: dekEntries } = await s3Client
+            .listObjectsV2({
               Bucket: process.env.AWS_S3_USER_FILES_DEK_BUCKET_NAME,
               Prefix: `${projectId}/`,
             })
-          )
+            .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.send(
-            new ListObjectsV2Command({
+          const { Contents: userFilesEntries } = await s3Client
+            .listObjectsV2({
               Bucket: backendSettings.stores.user_files,
               Prefix: `${projectId}/`,
             })
-          )
+            .promise()
           expect(userFilesEntries).to.have.length(userFilesBucketKeys.length)
           // Order is not predictable, use members
           expect(userFilesEntries.map(o => o.Key)).to.have.members(

+ 0 - 1
services/history-v1/docker-compose.ci.yml

@@ -43,7 +43,6 @@ services:
       AWS_S3_PATH_STYLE: 'true'
       AWS_ACCESS_KEY_ID: OVERLEAF_HISTORY_S3_ACCESS_KEY_ID
       AWS_SECRET_ACCESS_KEY: OVERLEAF_HISTORY_S3_SECRET_ACCESS_KEY
-      AWS_REGION: us-east-1
       MINIO_ROOT_USER: MINIO_ROOT_USER
       MINIO_ROOT_PASSWORD: MINIO_ROOT_PASSWORD
       GCS_API_ENDPOINT: http://gcs:9090

+ 0 - 1
services/history-v1/docker-compose.yml

@@ -60,7 +60,6 @@ services:
       AWS_S3_PATH_STYLE: 'true'
       AWS_ACCESS_KEY_ID: OVERLEAF_HISTORY_S3_ACCESS_KEY_ID
       AWS_SECRET_ACCESS_KEY: OVERLEAF_HISTORY_S3_SECRET_ACCESS_KEY
-      AWS_REGION: us-east-1
       MINIO_ROOT_USER: MINIO_ROOT_USER
       MINIO_ROOT_PASSWORD: MINIO_ROOT_PASSWORD
       GCS_API_ENDPOINT: http://gcs:9090

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

@@ -15,7 +15,6 @@ 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
@@ -33,11 +32,9 @@ 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.send(
-    new ListObjectsV2Command({ Bucket: bucket, Prefix: prefix })
-  )
-
+  const response = await client
+    .listObjectsV2({ Bucket: bucket, Prefix: prefix })
+    .promise()
   return (response.Contents || []).map(item => item.Key || '')
 }
 

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

@@ -35,7 +35,6 @@ 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')
 
@@ -584,13 +583,11 @@ describe('backup script', function () {
       // Get all chunks and verify they were backed up
       const listing = await backupPersistor
         ._getClientForBucket(chunksBucket)
-        .send(
-          new ListObjectsV2Command({
-            Bucket: chunksBucket,
-            Prefix: projectKey.format(historyId) + '/',
-          })
-        )
-
+        .listObjectsV2({
+          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
 

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

@@ -25,11 +25,10 @@ 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.send(new ListObjectsV2Command({ Bucket: bucket }))
+  return await client.listObjectsV2({ Bucket: bucket }).promise()
 }
 
 async function listS3Bucket(bucket, wantStorageClass) {

+ 1 - 0
services/project-history/package.json

@@ -28,6 +28,7 @@
     "@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",

+ 2 - 10
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 aws = require('@aws-sdk/client-ses')
+const sesTransport = require('nodemailer-ses-transport')
 const OError = require('@overleaf/o-error')
 const { RateLimiter } = require('../../infrastructure/RateLimiter')
 const _ = require('lodash')
@@ -30,15 +30,7 @@ function getClient() {
     const emailParameters = EMAIL_SETTINGS.parameters
     if (emailParameters.AWSAccessKeyID || EMAIL_SETTINGS.driver === 'ses') {
       logger.debug('using aws ses for email')
-      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 } })
+      client = nodemailer.createTransport(sesTransport(emailParameters))
     } else if (emailParameters.sendgridApiKey) {
       throw new OError(
         'sendgridApiKey configuration option is deprecated, use SMTP instead'

+ 1 - 1
services/web/package.json

@@ -76,7 +76,6 @@
     "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,6 +150,7 @@
     "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",

+ 1 - 3
services/web/test/unit/src/Email/EmailSenderTests.js

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

Some files were not shown because too many files changed in this diff