Race condition in s3-request-presigner because client is mutated

[mayorandrew]

Describe the bug

s3-request-presigner package provides a function getSignedUrl that accepts client, command and options.

Based on the source code, it works like this:

1. Create middleware that intercepts HTTP request before authentication
2. Attach that middleware to client
3. Try to invoke the command on that client
4. Middleware intercepts the command and returns the signed URL instead of actually executing it
5. Detach that middleware

This happens asynchronously, so there is a time interval when this middleware is attached to client. If the client is used during this time interval elsewhere, that other request can break because of this middleware. This can even cause problems if we call getSignedUrl two times in parallel with the same client, say with Promise.all.

Expected Behavior

client should not be mutated inside getSignedUrl

In concrete example, if we follow the reproduction steps below, we should get the following array logged:

[
  '<some-signed-url>',
  {
    '$metadata': {
      httpStatusCode: 200,
      requestId: undefined,
      extendedRequestId: '<some-string>',
      cfId: undefined,
      attempts: 1,
      totalRetryDelay: 0
    },
    AcceptRanges: 'bytes',
    Body: IncomingMessage {
      _readableState: [ReadableState],
      _events: [Object: null prototype],
      _eventsCount: 1,
      _maxListeners: undefined,
      socket: [TLSSocket],
      httpVersionMajor: 1,
      httpVersionMinor: 1,
      httpVersion: '1.1',
      complete: false,
      rawHeaders: [Array],
      rawTrailers: [],
      aborted: false,
      upgrade: false,
      url: '',
      method: null,
      statusCode: 200,
      statusMessage: 'OK',
      client: [TLSSocket],
      _consuming: false,
      _dumped: false,
      req: [ClientRequest],
      [Symbol(kCapture)]: false,
      [Symbol(kHeaders)]: [Object],
      [Symbol(kHeadersCount)]: 18,
      [Symbol(kTrailers)]: null,
      [Symbol(kTrailersCount)]: 0,
      [Symbol(RequestTimeout)]: undefined
    },
    BucketKeyEnabled: undefined,
    CacheControl: undefined,
    ChecksumCRC32: undefined,
    ChecksumCRC32C: undefined,
    ChecksumSHA1: undefined,
    ChecksumSHA256: undefined,
    ContentDisposition: undefined,
    ContentEncoding: undefined,
    ContentLanguage: undefined,
    ContentLength: 318,
    ContentRange: undefined,
    ContentType: 'application/json',
    DeleteMarker: undefined,
    ETag: '"d01ab5f760b1202e3c9a17b5581a3ed9"',
    Expiration: undefined,
    Expires: undefined,
    LastModified: 2022-06-08T15:19:43.000Z,
    Metadata: {},
    MissingMeta: undefined,
    ObjectLockLegalHoldStatus: undefined,
    ObjectLockMode: undefined,
    ObjectLockRetainUntilDate: undefined,
    PartsCount: undefined,
    ReplicationStatus: undefined,
    RequestCharged: undefined,
    Restore: undefined,
    SSECustomerAlgorithm: undefined,
    SSECustomerKeyMD5: undefined,
    SSEKMSKeyId: undefined,
    ServerSideEncryption: undefined,
    StorageClass: undefined,
    TagCount: undefined,
    VersionId: undefined,
    WebsiteRedirectLocation: undefined
  }
]

Current Behavior

client is mutated and it breaks other requests running at the same time.

In concrete example, if we follow the reproduction steps below, we'll get the following array logged:

[
  '<some-signed-url>',
  {
    '$metadata': { httpStatusCode: 200, attempts: 1, totalRetryDelay: 0 },
    presigned: {
      method: 'GET',
      hostname: '<my-bucket-name>.s3.eu-west-1.amazonaws.com',
      port: undefined,
      body: undefined,
      protocol: 'https:',
      path: '/<my-file-name>',
      headers: [Object],
      query: [Object]
    }
  }
]

First element of the array is ok and as expected. The second element contains the raw response from the middleware attached by getSignedUrl.

Reproduction Steps

import { getSignedUrl } from "@aws-sdk/s3-request-presigner";
import { S3Client, GetObjectCommand } from "@aws-sdk/client-s3";

const client = new S3Client({ region: '<my-region>', credentials: { /* my credentials */ });
const command = new GetObjectCommand({ Bucket: '<my-bucket-name>', Key: '<my-file-name>' });

const results = await Promise.all([
  // generate signed url
  getSignedUrl(client, command, { expiresIn: 3600 }),
  // at the same time download the file
  client.send(command);
]);

console.log(results);

Possible Solution

No response

Additional Information/Context

No response

SDK version used

3.105.0

Environment details (OS name and version, etc.)

Windows 10 21H2, nodejs v16.14.2

[yenfryherrerafeliz]

Hi @mayorandrew thanks for opening an issue. I was able to reproduce what you reported. I will mark this issue to be reviewed so we can investigate this further.

Thanks.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.