Skip to content

Commit 9bd5078

Browse files
authored
fix(custom-resource): custom resources fail with data containing multi-byte utf8 chars (#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 #24491 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 15cb919 commit 9bd5078

File tree

47 files changed

+1782
-1238
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+1782
-1238
lines changed

packages/@aws-cdk/aws-logs/lib/log-retention-provider/index.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,10 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
188188
hostname: parsedUrl.hostname,
189189
path: parsedUrl.path,
190190
method: 'PUT',
191-
headers: { 'content-type': '', 'content-length': responseBody.length },
191+
headers: {
192+
'content-type': '',
193+
'content-length': Buffer.byteLength(responseBody, 'utf8'),
194+
},
192195
};
193196

194197
return new Promise((resolve, reject) => {

packages/@aws-cdk/core/lib/custom-resource-provider/nodejs-entrypoint.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ async function submitResponse(status: 'SUCCESS' | 'FAILED', event: Response) {
115115
hostname: parsedUrl.hostname,
116116
path: parsedUrl.path,
117117
method: 'PUT',
118-
headers: { 'content-type': '', 'content-length': responseBody.length },
118+
headers: {
119+
'content-type': '',
120+
'content-length': Buffer.byteLength(responseBody, 'utf8'),
121+
},
119122
};
120123

121124
const retryOptions = {

packages/@aws-cdk/core/test/custom-resource-provider/nodejs-entrypoint.test.ts

+35-17
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,19 @@ describe('nodejs entrypoint', () => {
1414
const createEvent = makeEvent({ RequestType: 'Create' });
1515

1616
// WHEN
17-
const response = await invokeHandler(createEvent, async _ => ({ PhysicalResourceId: 'returned-from-handler' }));
17+
const { response } = await invokeHandler(createEvent, async _ => ({ PhysicalResourceId: 'returned-from-handler' }));
1818

1919
// THEN
2020
expect(response.Status).toEqual('SUCCESS');
2121
expect(response.PhysicalResourceId).toEqual('returned-from-handler');
22-
2322
});
2423

2524
test('data (attributes)', async () => {
2625
// GIVEN
2726
const createEvent = makeEvent({ RequestType: 'Create' });
2827

2928
// WHEN
30-
const response = await invokeHandler(createEvent, async _ => {
29+
const { response } = await invokeHandler(createEvent, async _ => {
3130
return {
3231
Data: {
3332
Attribute1: 'hello',
@@ -47,33 +46,52 @@ describe('nodejs entrypoint', () => {
4746
Foo: 1111,
4847
},
4948
});
50-
5149
});
5250

5351
test('no echo', async () => {
5452
// GIVEN
5553
const createEvent = makeEvent({ RequestType: 'Create' });
5654

5755
// WHEN
58-
const response = await invokeHandler(createEvent, async _ => ({ NoEcho: true }));
56+
const { response } = await invokeHandler(createEvent, async _ => ({ NoEcho: true }));
5957

6058
// THEN
6159
expect(response.Status).toEqual('SUCCESS');
6260
expect(response.NoEcho).toEqual(true);
63-
6461
});
6562

6663
test('reason', async () => {
6764
// GIVEN
6865
const createEvent = makeEvent({ RequestType: 'Create' });
6966

7067
// WHEN
71-
const response = await invokeHandler(createEvent, async _ => ({ Reason: 'hello, reason' }));
68+
const { response } = await invokeHandler(createEvent, async _ => ({ Reason: 'hello, reason' }));
7269

7370
// THEN
7471
expect(response.Status).toEqual('SUCCESS');
7572
expect(response.Reason).toEqual('hello, reason');
73+
});
74+
75+
test('utf8 is supported', async () => {
76+
// GIVEN
77+
const createEvent = makeEvent({ RequestType: 'Create' });
78+
const { request: emptyDataRequest } = await invokeHandler(createEvent, async _ => ({
79+
Data: {
80+
Attribute: '', // 0 bytes
81+
},
82+
}));
7683

84+
// WHEN
85+
const { request: utf8DataRequest } = await invokeHandler(createEvent, async _ => ({
86+
Data: {
87+
Attribute: 'ÅÄÖ', // 6 bytes
88+
},
89+
}));
90+
91+
// THEN
92+
const emptyLength = emptyDataRequest.headers?.['content-length'] as number;
93+
const utf8Length = utf8DataRequest.headers?.['content-length'] as number;
94+
expect(utf8Length - emptyLength).toEqual(6);
7795
});
7896
});
7997

@@ -82,7 +100,7 @@ describe('nodejs entrypoint', () => {
82100
const createEvent = makeEvent({ RequestType: 'Create' });
83101

84102
// WHEN
85-
const response = await invokeHandler(createEvent, async _ => {
103+
const { response } = await invokeHandler(createEvent, async _ => {
86104
throw new Error('this is an error');
87105
});
88106

@@ -95,16 +113,14 @@ describe('nodejs entrypoint', () => {
95113
PhysicalResourceId: 'AWSCDK::CustomResourceProviderFramework::CREATE_FAILED',
96114
LogicalResourceId: '<LogicalResourceId>',
97115
});
98-
99-
100116
});
101117

102118
test('physical resource id cannot be changed in DELETE', async () => {
103119
// GIVEN
104120
const event = makeEvent({ RequestType: 'Delete' });
105121

106122
// WHEN
107-
const response = await invokeHandler(event, async _ => ({
123+
const { response } = await invokeHandler(event, async _ => ({
108124
PhysicalResourceId: 'Changed',
109125
}));
110126

@@ -117,8 +133,6 @@ describe('nodejs entrypoint', () => {
117133
PhysicalResourceId: 'AWSCDK::CustomResourceProviderFramework::MISSING_PHYSICAL_ID',
118134
LogicalResourceId: '<LogicalResourceId>',
119135
});
120-
121-
122136
});
123137

124138
test('DELETE after CREATE is ignored with success', async () => {
@@ -129,7 +143,7 @@ describe('nodejs entrypoint', () => {
129143
});
130144

131145
// WHEN
132-
const response = await invokeHandler(event, async _ => {
146+
const { response } = await invokeHandler(event, async _ => {
133147
throw new Error('handler should not be called');
134148
});
135149

@@ -142,7 +156,6 @@ describe('nodejs entrypoint', () => {
142156
PhysicalResourceId: 'AWSCDK::CustomResourceProviderFramework::CREATE_FAILED',
143157
LogicalResourceId: '<LogicalResourceId>',
144158
});
145-
146159
});
147160
});
148161

@@ -179,17 +192,22 @@ async function invokeHandler(req: AWSLambda.CloudFormationCustomResourceEvent, u
179192
};
180193

181194
let actualResponse;
195+
let actualRequest;
182196
entrypoint.external.sendHttpRequest = async (options: https.RequestOptions, responseBody: string): Promise<void> => {
183197
assert(options.hostname === parsedResponseUrl.hostname, 'request hostname expected to be based on response URL');
184198
assert(options.path === parsedResponseUrl.path, 'request path expected to be based on response URL');
185199
assert(options.method === 'PUT', 'request method is expected to be PUT');
186200
actualResponse = responseBody;
201+
actualRequest = options;
187202
};
188203

189204
await entrypoint.handler(req, {} as AWSLambda.Context);
190-
if (!actualResponse) {
205+
if (!actualRequest || !actualResponse) {
191206
throw new Error('no response sent to cloudformation');
192207
}
193208

194-
return JSON.parse(actualResponse) as AWSLambda.CloudFormationCustomResourceResponse;
209+
return {
210+
response: JSON.parse(actualResponse) as AWSLambda.CloudFormationCustomResourceResponse,
211+
request: actualRequest as https.RequestOptions,
212+
};
195213
}
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,4 @@
11
const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
22
baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
33

4-
baseConfig.overrides = [{
5-
// @TODO Fixing the import order will cause custom resources to updated due to a hash change
6-
// We should apply the fix the next time `framework.ts` is meaningfully changed to avoid unnecessary resource updates
7-
"files": ["lib/provider-framework/runtime/framework.ts"],
8-
"rules": {
9-
'import/order': 'warn', // this should be 'error'
10-
}
11-
}]
12-
134
module.exports = baseConfig;

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/runtime/index.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,10 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
242242
hostname: parsedUrl.hostname,
243243
path: parsedUrl.path,
244244
method: 'PUT',
245-
headers: { 'content-type': '', 'content-length': responseBody.length },
245+
headers: {
246+
'content-type': '',
247+
'content-length': Buffer.byteLength(responseBody, 'utf8'),
248+
},
246249
};
247250

248251
return new Promise((resolve, reject) => {

packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/cfn-response.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export async function submitResponse(status: 'SUCCESS' | 'FAILED', event: CloudF
4949
method: 'PUT',
5050
headers: {
5151
'content-type': '',
52-
'content-length': responseBody.length,
52+
'content-length': Buffer.byteLength(responseBody, 'utf8'),
5353
},
5454
}, responseBody);
5555
}

packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/* eslint-disable max-len */
22
/* eslint-disable no-console */
3-
import { IsCompleteResponse, OnEventResponse } from '../types';
43
import * as cfnResponse from './cfn-response';
54
import * as consts from './consts';
65
import { invokeFunction, startExecution } from './outbound';
76
import { getEnv, log } from './util';
7+
import { IsCompleteResponse, OnEventResponse } from '../types';
88

99
// use consts for handler names to compiler-enforce the coupling with construction code.
1010
export = {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"version": "30.1.0",
3+
"files": {
4+
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
5+
"source": {
6+
"path": "AwsCustomResourceTestDefaultTestDeployAssert289A7DC5.template.json",
7+
"packaging": "file"
8+
},
9+
"destinations": {
10+
"current_account-current_region": {
11+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12+
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
13+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
14+
}
15+
}
16+
}
17+
},
18+
"dockerImages": {}
19+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"Parameters": {
3+
"BootstrapVersion": {
4+
"Type": "AWS::SSM::Parameter::Value<String>",
5+
"Default": "/cdk-bootstrap/hnb659fds/version",
6+
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
7+
}
8+
},
9+
"Rules": {
10+
"CheckBootstrapVersion": {
11+
"Assertions": [
12+
{
13+
"Assert": {
14+
"Fn::Not": [
15+
{
16+
"Fn::Contains": [
17+
[
18+
"1",
19+
"2",
20+
"3",
21+
"4",
22+
"5"
23+
],
24+
{
25+
"Ref": "BootstrapVersion"
26+
}
27+
]
28+
}
29+
]
30+
},
31+
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
32+
}
33+
]
34+
}
35+
}
36+
}
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/custom-resources/test/aws-custom-resource/integ.aws-custom-resource.js.snapshot/aws-cdk-sdk-js.assets.json

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
{
2-
"version": "22.0.0",
2+
"version": "30.1.0",
33
"files": {
4-
"a268caa53756f51bda8ad5f499be4ed8484a81b314811806fbb66f874837c476": {
4+
"8c980c60f4c1c0edebd987e6043e356b8d439b2d731c5af3329df082ca5a6a79": {
55
"source": {
6-
"path": "asset.a268caa53756f51bda8ad5f499be4ed8484a81b314811806fbb66f874837c476",
6+
"path": "asset.8c980c60f4c1c0edebd987e6043e356b8d439b2d731c5af3329df082ca5a6a79",
77
"packaging": "zip"
88
},
99
"destinations": {
1010
"current_account-current_region": {
1111
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12-
"objectKey": "a268caa53756f51bda8ad5f499be4ed8484a81b314811806fbb66f874837c476.zip",
12+
"objectKey": "8c980c60f4c1c0edebd987e6043e356b8d439b2d731c5af3329df082ca5a6a79.zip",
1313
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
1414
}
1515
}
1616
},
17-
"fcfb1dd36ebebdb91b2be028c7fedf6fec075728ba6c5165e04b3d0be259c783": {
17+
"eaaca485f9a98a657308ad4e676057416abc8024d04826b9c08aa25d37293c94": {
1818
"source": {
1919
"path": "aws-cdk-sdk-js.template.json",
2020
"packaging": "file"
2121
},
2222
"destinations": {
2323
"current_account-current_region": {
2424
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
25-
"objectKey": "fcfb1dd36ebebdb91b2be028c7fedf6fec075728ba6c5165e04b3d0be259c783.json",
25+
"objectKey": "eaaca485f9a98a657308ad4e676057416abc8024d04826b9c08aa25d37293c94.json",
2626
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
2727
}
2828
}

0 commit comments

Comments
 (0)