Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3:PutObject doesn't work when key has any non alphanumeric character and getSignerURL doesn't work #866

Closed
Amplifiyer opened this issue Feb 6, 2020 · 15 comments · Fixed by smithy-lang/smithy-typescript#121
Assignees
Labels
bug This issue is a bug. rc-blocker

Comments

@Amplifiyer
Copy link
Contributor

Describe the bug
S3:PutObject doesn't work when key has any non alphanumeric character such as space and getSignerURL doesn't work.

Both return a SignatureDoesNotMatch exception.

SDK version number

    "@aws-sdk/client-s3": "^1.0.0-alpha.10",
    "@aws-sdk/s3-request-presigner": "^1.0.0-alpha.10",
    "@aws-sdk/util-create-request": "^1.0.0-alpha.9",
    "@aws-sdk/util-format-url": "^1.0.0-alpha.4",

Is the issue in the browser/Node.js?
Browser

Details of the browser/Node.js version
Browsers:
Chrome: 79.0.3945.117
Safari: 13.0.4

To Reproduce (observed behavior)

  • Clone and run the repo app https://github.com/Amplifiyer/aws-sdk-js-tests/tree/s3client
  • The first three results are from V2 and last three are from V3
  • When key of file being uploaded has spaces, V3 throws exception SignatureDoesNotMatch while V2 doesn't
  • Copy paste the signed URL generated by V2 and V3 and open in new tab and see that V2 file is downloaded while V3 fails.

Expected behavior
No errors...

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@Amplifiyer
Copy link
Contributor Author

This fix has caused regression, now the full path of the key is encoded so /folder1/folder2/file.jpg becomes /folder1%2ffolder2%2ffile.jpg

@AllanZhengYP
Copy link
Contributor

AllanZhengYP commented Feb 8, 2020

@Amplifiyer Sorry for the regression. smithy-lang/smithy-typescript#123 is ready to fix the problem.

@trivikr trivikr reopened this Feb 9, 2020
@trivikr
Copy link
Member

trivikr commented Feb 9, 2020

Reopening for verification, new fix is in #894

@trivikr
Copy link
Member

trivikr commented Feb 9, 2020

Verified that the issue is fixed with @aws-sdk/client-s3@1.0.0-alpha.13

Code (verified in Node.js):

const AWS = require("aws-sdk");
const { S3 } = require("@aws-sdk/client-s3");
const { REGION } = require("./config");

(async () => {
  let response;

  const bucketName =
    "test-bucket-" +
    Math.random()
      .toString(36)
      .substring(2);
  const params = {
    Bucket: bucketName
  };
  const objectNames = [
    "NameWithoutSpaces.jpg",
    "Name With Spaces.jpg",
    "folder1/folder2/file.jpg"
  ];

  const v2Client = new AWS.S3({ region: REGION });
  const v3Client = new S3({ region: REGION });

  console.log(`Creating bucket ${bucketName}`);
  await v3Client.createBucket(params);

  // Using v2 waiter, as v3 one is not available
  console.log(`\nWaiting for "${bucketName}" bucket creation\n`);
  await v2Client.waitFor("bucketExists", params).promise();

  for (let i = 0; i < objectNames.length; i++) {
    console.log(`Putting "${objectNames[i]}" in ${bucketName}`);
    await v3Client.putObject({
      Body: "000000",
      Bucket: bucketName,
      Key: objectNames[i]
    });
  }

  response = await v3Client.listObjects(params);
  console.log("\nlistObjects:");
  console.log(JSON.stringify(response, null, 2));
  console.log("\n");

  for (let i = 0; i < objectNames.length; i++) {
    console.log(`Deleting "${objectNames[i]}" from ${bucketName}`);
    await v3Client.deleteObject({
      Bucket: bucketName,
      Key: objectNames[i]
    });
  }

  console.log(`\nDeleting bucket ${bucketName}`);
  await v3Client.deleteBucket(params);
})();

Output:

Creating bucket test-bucket-rg1g50xtd6

Waiting for "test-bucket-rg1g50xtd6" bucket creation

Putting "NameWithoutSpaces.jpg" in test-bucket-rg1g50xtd6
Putting "Name With Spaces.jpg" in test-bucket-rg1g50xtd6
Putting "folder1/folder2/file.jpg" in test-bucket-rg1g50xtd6

listObjects:
{
  "$metadata": {
    "httpStatusCode": 200,
    "httpHeaders": {
      "x-amz-id-2": "zmdodovs2WdeQQ5ez4Sopo4wA00nYqdQKIAulLRWWkHdUIq8so3lp0LulZOEJ4zTgHCkZSpDf5I=",
      "x-amz-request-id": "9ECD6ACB1369BC6F",
      "date": "Sun, 09 Feb 2020 23:06:50 GMT",
      "x-amz-bucket-region": "us-west-2",
      "content-type": "application/xml",
      "transfer-encoding": "chunked",
      "server": "AmazonS3"
    },
    "retries": 0,
    "totalRetryDelay": 0
  },
  "__type": "ListObjectsOutput",
  "Contents": [
    {
      "__type": "Object",
      "ETag": "&quot;670b14728ad9902aecba32e22fa4f6bd&quot;",
      "Key": "Name With Spaces.jpg",
      "LastModified": "2020-02-09T23:06:50.000Z",
      "Owner": {
        "__type": "Owner",
        "DisplayName": "trivikr",
        "ID": "f36465be55e5435ccc8357d4857b23522cbf71100368115f086c017f5092617a"
      },
      "Size": 6,
      "StorageClass": "STANDARD"
    },
    {
      "__type": "Object",
      "ETag": "&quot;670b14728ad9902aecba32e22fa4f6bd&quot;",
      "Key": "NameWithoutSpaces.jpg",
      "LastModified": "2020-02-09T23:06:50.000Z",
      "Owner": {
        "__type": "Owner",
        "DisplayName": "trivikr",
        "ID": "f36465be55e5435ccc8357d4857b23522cbf71100368115f086c017f5092617a"
      },
      "Size": 6,
      "StorageClass": "STANDARD"
    },
    {
      "__type": "Object",
      "ETag": "&quot;670b14728ad9902aecba32e22fa4f6bd&quot;",
      "Key": "folder1/folder2/file.jpg",
      "LastModified": "2020-02-09T23:06:50.000Z",
      "Owner": {
        "__type": "Owner",
        "DisplayName": "trivikr",
        "ID": "f36465be55e5435ccc8357d4857b23522cbf71100368115f086c017f5092617a"
      },
      "Size": 6,
      "StorageClass": "STANDARD"
    }
  ],
  "IsTruncated": false,
  "Marker": "",
  "MaxKeys": 1000,
  "Name": "test-bucket-rg1g50xtd6",
  "Prefix": ""
}


Deleting "NameWithoutSpaces.jpg" from test-bucket-rg1g50xtd6
Deleting "Name With Spaces.jpg" from test-bucket-rg1g50xtd6
Deleting "folder1/folder2/file.jpg" from test-bucket-rg1g50xtd6

Deleting bucket test-bucket-rg1g50xtd6

@trivikr
Copy link
Member

trivikr commented Feb 9, 2020

@Amplifiyer Do close this issue if this is fixed in @aws-sdk/client-s3@1.0.0-alpha.13

@Amplifiyer
Copy link
Contributor Author

@trivikr I verified that the path and the space are properly preserved, however parenthesis are not and throwing the SignatureDoesNotMatch exception. Also the getSignedURL is still throwing the same exception and is not fixed in this issue.

The repro repo is still the same and I have pushed a commit to reproduce the parenthesis problem

@AllanZhengYP AllanZhengYP assigned trivikr and unassigned AllanZhengYP Feb 10, 2020
@kstich
Copy link
Contributor

kstich commented Feb 11, 2020

Looks like we'll need a more complete uri encoding to adhere to RFC 3986 if there's not one available by default.

@trivikr
Copy link
Member

trivikr commented Feb 11, 2020

Test code for characters in RFC3986 which is not covered:

Code:

const AWS = require("aws-sdk");
const { S3 } = require("@aws-sdk/client-s3");
const { REGION } = require("./config");

(async () => {
  let response;

  const bucketName =
    "test-bucket-" +
    Math.random()
      .toString(36)
      .substring(2);
  const params = {
    Bucket: bucketName
  };
  const prefix = "test-";
  const suffix = ".jpg";

  // Refs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent#Description
  const rfc3986ExtendedChars = "!'()*";

  const v2Client = new AWS.S3({ region: REGION });
  const v3Client = new S3({ region: REGION });

  console.log(`Creating bucket ${bucketName}`);
  await v3Client.createBucket(params);

  // Using v2 waiter, as v3 one is not available
  console.log(`\nWaiting for "${bucketName}" bucket creation\n`);
  await v2Client.waitFor("bucketExists", params).promise();

  for (let i = 0; i < rfc3986ExtendedChars.length; i++) {
    const Key = `${prefix}${rfc3986ExtendedChars.charAt(i)}${suffix}`;
    console.log(`Putting "${Key}" in ${bucketName}`);
    try {
      await v3Client.putObject({
        Body: "000000",
        Bucket: bucketName,
        Key
      });
    } catch (e) {
      console.log(`PutObject "${Key}" failed...`);
    }
  }

  response = await v3Client.listObjects(params);
  console.log("\nlistObjects:");
  console.log(JSON.stringify(response, null, 2));
  console.log("\n");

  if (response.Contents) {
    for (let i = 0; i < response.Contents.length; i++) {
      const { Key } = response.Contents[i];
      console.log(`Deleting "${Key}" from ${bucketName}`);
      await v3Client.deleteObject({
        Bucket: bucketName,
        Key
      });
    }
  }

  console.log(`\nDeleting bucket ${bucketName}`);
  await v3Client.deleteBucket(params);
})();

Output:

Creating bucket test-bucket-15xrr6am05x

Waiting for "test-bucket-15xrr6am05x" bucket creation

Putting "test-!.jpg" in test-bucket-15xrr6am05x
PutObject "test-!.jpg" failed...
Putting "test-'.jpg" in test-bucket-15xrr6am05x
PutObject "test-'.jpg" failed...
Putting "test-(.jpg" in test-bucket-15xrr6am05x
PutObject "test-(.jpg" failed...
Putting "test-).jpg" in test-bucket-15xrr6am05x
PutObject "test-).jpg" failed...
Putting "test-*.jpg" in test-bucket-15xrr6am05x
PutObject "test-*.jpg" failed...

listObjects:
{
  "$metadata": {
    "httpStatusCode": 200,
    "httpHeaders": {
      "x-amz-id-2": "39mUhDIQPkQJJqHfK5CzPjc7rwOaiSqmfS2TsfNl0uh/n1sgnjToMhvxSDIkQe0MaZX4TS8eMDA=",
      "x-amz-request-id": "F9BE59A092CA0217",
      "date": "Tue, 11 Feb 2020 19:54:16 GMT",
      "x-amz-bucket-region": "us-west-2",
      "content-type": "application/xml",
      "transfer-encoding": "chunked",
      "server": "AmazonS3"
    },
    "retries": 0,
    "totalRetryDelay": 0
  },
  "__type": "ListObjectsOutput",
  "IsTruncated": false,
  "Marker": "",
  "MaxKeys": 1000,
  "Name": "test-bucket-15xrr6am05x",
  "Prefix": ""
}

Deleting bucket test-bucket-15xrr6am05x

MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent#Description

@srchase
Copy link
Contributor

srchase commented Feb 13, 2020

Additional fix for encoding in smithy-lang/smithy-typescript/pull/134

@srchase
Copy link
Contributor

srchase commented Feb 14, 2020

Confirmed fixed with client-s3@1.0.0-alpha.16

Code:

const { S3 } = require('@aws-sdk/client-s3');

(async () => {
    const client = new S3({region: "us-east-1"})
    await client.putObject({
        Bucket: "fooBucket",
        Key: "foo()",
        Body: "bodyText"
    }).then((res) => {
        console.log(res);
    }).catch((err) => {
        console.log(err);
    });
})();

Output:

{
  '$metadata': {
    httpStatusCode: 301,
    httpHeaders: {
      'x-amz-bucket-region': 'us-east-1',
      'x-amz-request-id': '...',
      'x-amz-id-2': '...',
      'content-type': 'application/xml',
      'transfer-encoding': 'chunked',
      date: 'Fri, 14 Feb 2020 17:03:09 GMT',
      connection: 'close',
      server: 'AmazonS3'
    },
    requestId: undefined,
    retries: 0,
    totalRetryDelay: 0
  },
  __type: 'PutObjectOutput',
  ...
}

@srchase srchase closed this as completed Feb 14, 2020
@Amplifiyer
Copy link
Contributor Author

I can see the putObject is fixed with parentheses, but it caused another regression. Now listObjects is failing due to encoding

Listing from public/ directory causes the request URL to be https://analytics3d5167b766084a44803d0ccb49a6eb66-devoa.s3.us-west-2.amazonaws.com/?prefix=public%252F
which then fails with AccessDenied Error

@Amplifiyer
Copy link
Contributor Author

the preSignedURL is still failing with the SignatureDoesNotMatch exception. Was this API tested after the fix? Just want to ensure that I'm not using the API incorrectly.

@AllanZhengYP
Copy link
Contributor

@Amplifiyer you can create the presign request like below:

  const { S3Client, GetObjectCommand } = require("@aws-sdk/client-s3");
  const {S3RequestPresigner} = require("@aws-sdk/s3-request-presigner");
  const {createRequest} = require("@aws-sdk/util-create-request");
  const {formatUrl} = require("@aws-sdk/util-format-url");
  const client = new S3Client({
    region: "us-west-2",
  });
  const command = new GetObjectCommand({
    Bucket: "bucket",
    Key: "key"
  });
  const presigner = new S3RequestPresigner({...client.config});
  const request = await createRequest(client, command);
  //expires in 15 mins
  const signed = await presigner.presignRequest(request, new Date(Date.now() + 900 * 1000));
  const url = formatUrl(signed);
  console.log(url);

@Amplifiyer
Copy link
Contributor Author

@AllanFly120 it's not working for me. See https://github.com/Amplifiyer/aws-sdk-js-tests/tree/s3client where I reproduced the same problem after updating new aws-sdk clients. Paranthesis problem fixed, but check the preSigned URL (copy paste and open in new tab)

@lock
Copy link

lock bot commented Feb 21, 2020

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.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. rc-blocker
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants