fix(custom-resource): custom resources fail with data containing mult…

…i-byte utf8 chars (aws#24501) Custom Resources need to write their response into a S3 object. This is implemented as a PUT request to a pre-signed URL and has to specify the `content-length` of the response object. Previously the CustomResource code would use `responseBody.length`. However this returns the number of graphemes, not bytes. If any utf8 characters with `graphemes != bytes` are part of the response, CloudFormation would fail the deployment with a `Response is not valid JSON` error. Also updates the `log-retention-provider` code, although the data should only contain 1-byte characters. Due to this limitation it can't be tested. Closes aws#24491 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk · Mar 28, 2023 · b4df5ec · b4df5ec
1 parent a8c7fe3
commit b4df5ec
Show file tree

Hide file tree

Showing 47 changed files with 1,782 additions and 1,238 deletions.
diff --git a/packages/@aws-cdk/aws-logs/lib/log-retention-provider/index.ts b/packages/@aws-cdk/aws-logs/lib/log-retention-provider/index.ts
@@ -188,7 +188,10 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
       hostname: parsedUrl.hostname,
       path: parsedUrl.path,
       method: 'PUT',
-      headers: { 'content-type': '', 'content-length': responseBody.length },
+      headers: {
+        'content-type': '',
+        'content-length': Buffer.byteLength(responseBody, 'utf8'),
+      },
     };
 
     return new Promise((resolve, reject) => {

diff --git a/packages/@aws-cdk/core/lib/custom-resource-provider/nodejs-entrypoint.ts b/packages/@aws-cdk/core/lib/custom-resource-provider/nodejs-entrypoint.ts
@@ -115,7 +115,10 @@ async function submitResponse(status: 'SUCCESS' | 'FAILED', event: Response) {
     hostname: parsedUrl.hostname,
     path: parsedUrl.path,
     method: 'PUT',
-    headers: { 'content-type': '', 'content-length': responseBody.length },
+    headers: {
+      'content-type': '',
+      'content-length': Buffer.byteLength(responseBody, 'utf8'),
+    },
   };
 
   const retryOptions = {

diff --git a/packages/@aws-cdk/core/test/custom-resource-provider/nodejs-entrypoint.test.ts b/packages/@aws-cdk/core/test/custom-resource-provider/nodejs-entrypoint.test.ts
@@ -14,20 +14,19 @@ describe('nodejs entrypoint', () => {
       const createEvent = makeEvent({ RequestType: 'Create' });
 
       // WHEN
-      const response = await invokeHandler(createEvent, async _ => ({ PhysicalResourceId: 'returned-from-handler' }));
+      const { response } = await invokeHandler(createEvent, async _ => ({ PhysicalResourceId: 'returned-from-handler' }));
 
       // THEN
       expect(response.Status).toEqual('SUCCESS');
       expect(response.PhysicalResourceId).toEqual('returned-from-handler');
-
     });
 
     test('data (attributes)', async () => {
       // GIVEN
       const createEvent = makeEvent({ RequestType: 'Create' });
 
       // WHEN
-      const response = await invokeHandler(createEvent, async _ => {
+      const { response } = await invokeHandler(createEvent, async _ => {
         return {
           Data: {
             Attribute1: 'hello',
@@ -47,33 +46,52 @@ describe('nodejs entrypoint', () => {
           Foo: 1111,
         },
       });
-
     });
 
     test('no echo', async () => {
       // GIVEN
       const createEvent = makeEvent({ RequestType: 'Create' });
 
       // WHEN
-      const response = await invokeHandler(createEvent, async _ => ({ NoEcho: true }));
+      const { response } = await invokeHandler(createEvent, async _ => ({ NoEcho: true }));
 
       // THEN
       expect(response.Status).toEqual('SUCCESS');
       expect(response.NoEcho).toEqual(true);
-
     });
 
     test('reason', async () => {
       // GIVEN
       const createEvent = makeEvent({ RequestType: 'Create' });
 
       // WHEN
-      const response = await invokeHandler(createEvent, async _ => ({ Reason: 'hello, reason' }));
+      const { response } = await invokeHandler(createEvent, async _ => ({ Reason: 'hello, reason' }));
 
       // THEN
       expect(response.Status).toEqual('SUCCESS');
       expect(response.Reason).toEqual('hello, reason');
+    });
+
+    test('utf8 is supported', async () => {
+      // GIVEN
+      const createEvent = makeEvent({ RequestType: 'Create' });
+      const { request: emptyDataRequest } = await invokeHandler(createEvent, async _ => ({
+        Data: {
+          Attribute: '', // 0 bytes
+        },
+      }));
 
+      // WHEN
+      const { request: utf8DataRequest } = await invokeHandler(createEvent, async _ => ({
+        Data: {
+          Attribute: 'ÅÄÖ', // 6 bytes
+        },
+      }));
+
+      // THEN
+      const emptyLength = emptyDataRequest.headers?.['content-length'] as number;
+      const utf8Length = utf8DataRequest.headers?.['content-length'] as number;
+      expect(utf8Length - emptyLength).toEqual(6);
     });
   });
 
@@ -82,7 +100,7 @@ describe('nodejs entrypoint', () => {
     const createEvent = makeEvent({ RequestType: 'Create' });
 
     // WHEN
-    const response = await invokeHandler(createEvent, async _ => {
+    const { response } = await invokeHandler(createEvent, async _ => {
       throw new Error('this is an error');
     });
 
@@ -95,16 +113,14 @@ describe('nodejs entrypoint', () => {
       PhysicalResourceId: 'AWSCDK::CustomResourceProviderFramework::CREATE_FAILED',
       LogicalResourceId: '<LogicalResourceId>',
     });
-
-
   });
 
   test('physical resource id cannot be changed in DELETE', async () => {
     // GIVEN
     const event = makeEvent({ RequestType: 'Delete' });
 
     // WHEN
-    const response = await invokeHandler(event, async _ => ({
+    const { response } = await invokeHandler(event, async _ => ({
       PhysicalResourceId: 'Changed',
     }));
 
@@ -117,8 +133,6 @@ describe('nodejs entrypoint', () => {
       PhysicalResourceId: 'AWSCDK::CustomResourceProviderFramework::MISSING_PHYSICAL_ID',
       LogicalResourceId: '<LogicalResourceId>',
     });
-
-
   });
 
   test('DELETE after CREATE is ignored with success', async () => {
@@ -129,7 +143,7 @@ describe('nodejs entrypoint', () => {
     });
 
     // WHEN
-    const response = await invokeHandler(event, async _ => {
+    const { response } = await invokeHandler(event, async _ => {
       throw new Error('handler should not be called');
     });
 
@@ -142,7 +156,6 @@ describe('nodejs entrypoint', () => {
       PhysicalResourceId: 'AWSCDK::CustomResourceProviderFramework::CREATE_FAILED',
       LogicalResourceId: '<LogicalResourceId>',
     });
-
   });
 });
 
@@ -179,17 +192,22 @@ async function invokeHandler(req: AWSLambda.CloudFormationCustomResourceEvent, u
   };
 
   let actualResponse;
+  let actualRequest;
   entrypoint.external.sendHttpRequest = async (options: https.RequestOptions, responseBody: string): Promise<void> => {
     assert(options.hostname === parsedResponseUrl.hostname, 'request hostname expected to be based on response URL');
     assert(options.path === parsedResponseUrl.path, 'request path expected to be based on response URL');
     assert(options.method === 'PUT', 'request method is expected to be PUT');
     actualResponse = responseBody;
+    actualRequest = options;
   };
 
   await entrypoint.handler(req, {} as AWSLambda.Context);
-  if (!actualResponse) {
+  if (!actualRequest || !actualResponse) {
     throw new Error('no response sent to cloudformation');
   }
 
-  return JSON.parse(actualResponse) as AWSLambda.CloudFormationCustomResourceResponse;
+  return {
+    response: JSON.parse(actualResponse) as AWSLambda.CloudFormationCustomResourceResponse,
+    request: actualRequest as https.RequestOptions,
+  };
 }
diff --git a/packages/@aws-cdk/custom-resources/.eslintrc.js b/packages/@aws-cdk/custom-resources/.eslintrc.js
@@ -1,13 +1,4 @@
 const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
 baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
 
-baseConfig.overrides = [{
-  // @TODO Fixing the import order will cause custom resources to updated due to a hash change
-  // We should apply the fix the next time `framework.ts` is meaningfully changed to avoid unnecessary resource updates 
-  "files": ["lib/provider-framework/runtime/framework.ts"],
-  "rules": {
-    'import/order': 'warn', // this should be 'error'
-  }
-}]
-
 module.exports = baseConfig;
diff --git a/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/runtime/index.ts b/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/runtime/index.ts
@@ -242,7 +242,10 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
       hostname: parsedUrl.hostname,
       path: parsedUrl.path,
       method: 'PUT',
-      headers: { 'content-type': '', 'content-length': responseBody.length },
+      headers: {
+        'content-type': '',
+        'content-length': Buffer.byteLength(responseBody, 'utf8'),
+      },
     };
 
     return new Promise((resolve, reject) => {

diff --git a/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/cfn-response.ts b/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/cfn-response.ts
@@ -49,7 +49,7 @@ export async function submitResponse(status: 'SUCCESS' | 'FAILED', event: CloudF
     method: 'PUT',
     headers: {
       'content-type': '',
-      'content-length': responseBody.length,
+      'content-length': Buffer.byteLength(responseBody, 'utf8'),
     },
   }, responseBody);
 }

diff --git a/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts b/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts
@@ -1,10 +1,10 @@
 /* eslint-disable max-len */
 /* eslint-disable no-console */
-import { IsCompleteResponse, OnEventResponse } from '../types';
 import * as cfnResponse from './cfn-response';
 import * as consts from './consts';
 import { invokeFunction, startExecution } from './outbound';
 import { getEnv, log } from './util';
+import { IsCompleteResponse, OnEventResponse } from '../types';
 
 // use consts for handler names to compiler-enforce the coupling with construction code.
 export = {

diff --git a/...tom-resource.js.snapshot/AwsCustomResourceTestDefaultTestDeployAssert289A7DC5.assets.json b/...tom-resource.js.snapshot/AwsCustomResourceTestDefaultTestDeployAssert289A7DC5.assets.json
@@ -0,0 +1,19 @@
+{
+  "version": "30.1.0",
+  "files": {
+    "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
+      "source": {
+        "path": "AwsCustomResourceTestDefaultTestDeployAssert289A7DC5.template.json",
+        "packaging": "file"
+      },
+      "destinations": {
+        "current_account-current_region": {
+          "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
+          "objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
+          "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
+        }
+      }
+    }
+  },
+  "dockerImages": {}
+}
diff --git a/...m-resource.js.snapshot/AwsCustomResourceTestDefaultTestDeployAssert289A7DC5.template.json b/...m-resource.js.snapshot/AwsCustomResourceTestDefaultTestDeployAssert289A7DC5.template.json
@@ -0,0 +1,36 @@
+{
+ "Parameters": {
+  "BootstrapVersion": {
+   "Type": "AWS::SSM::Parameter::Value<String>",
+   "Default": "/cdk-bootstrap/hnb659fds/version",
+   "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
+  }
+ },
+ "Rules": {
+  "CheckBootstrapVersion": {
+   "Assertions": [
+    {
+     "Assert": {
+      "Fn::Not": [
+       {
+        "Fn::Contains": [
+         [
+          "1",
+          "2",
+          "3",
+          "4",
+          "5"
+         ],
+         {
+          "Ref": "BootstrapVersion"
+         }
+        ]
+       }
+      ]
+     },
+     "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
+    }
+   ]
+  }
+ }
+}
diff --git a/...484a81b314811806fbb66f874837c476/index.js → ...8d439b2d731c5af3329df082ca5a6a79/index.js b/...484a81b314811806fbb66f874837c476/index.js → ...8d439b2d731c5af3329df082ca5a6a79/index.js
diff --git a/...test/aws-custom-resource/integ.aws-custom-resource.js.snapshot/aws-cdk-sdk-js.assets.json b/...test/aws-custom-resource/integ.aws-custom-resource.js.snapshot/aws-cdk-sdk-js.assets.json
@@ -1,28 +1,28 @@
 {
-  "version": "22.0.0",
+  "version": "30.1.0",
   "files": {
-    "a268caa53756f51bda8ad5f499be4ed8484a81b314811806fbb66f874837c476": {
+    "8c980c60f4c1c0edebd987e6043e356b8d439b2d731c5af3329df082ca5a6a79": {
       "source": {
-        "path": "asset.a268caa53756f51bda8ad5f499be4ed8484a81b314811806fbb66f874837c476",
+        "path": "asset.8c980c60f4c1c0edebd987e6043e356b8d439b2d731c5af3329df082ca5a6a79",
         "packaging": "zip"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "a268caa53756f51bda8ad5f499be4ed8484a81b314811806fbb66f874837c476.zip",
+          "objectKey": "8c980c60f4c1c0edebd987e6043e356b8d439b2d731c5af3329df082ca5a6a79.zip",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }
     },
-    "fcfb1dd36ebebdb91b2be028c7fedf6fec075728ba6c5165e04b3d0be259c783": {
+    "eaaca485f9a98a657308ad4e676057416abc8024d04826b9c08aa25d37293c94": {
       "source": {
         "path": "aws-cdk-sdk-js.template.json",
         "packaging": "file"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "fcfb1dd36ebebdb91b2be028c7fedf6fec075728ba6c5165e04b3d0be259c783.json",
+          "objectKey": "eaaca485f9a98a657308ad4e676057416abc8024d04826b9c08aa25d37293c94.json",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }