Skip to content

Commit 7bebf40

Browse files
authored
fix(s3): add support for uppercase characters in legacy bucket names (#31813)
### Issue # (if applicable) Closes #31731 ### Reason for this change As @GavinZZ [commented](#31731 (comment)) in issue #31731, the official AWS S3 documentation stated that uppercase characters were permitted in bucket names prior to March 1, 2018. ### Description of changes - add support for uppercase characters in legacy bucket names ### Description of how you validated changes - added new unit test cases and updated the integration test cases with legacy bucket names ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 318eae6 commit 7bebf40

File tree

10 files changed

+65
-34
lines changed

10 files changed

+65
-34
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.js.snapshot/aws-cdk-s3-legacy-name-integ.assets.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.js.snapshot/aws-cdk-s3-legacy-name-integ.template.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"Action": "s3:*",
2727
"Effect": "Allow",
2828
"Resource": [
29-
"arn:aws:s3:::my_legacy_bucket2/*",
29+
"arn:aws:s3:::My_legacy_bucket2/*",
3030
{
3131
"Fn::Join": [
3232
"",
@@ -35,7 +35,7 @@
3535
{
3636
"Ref": "AWS::Partition"
3737
},
38-
":s3:::my_legacy_bucket1/*"
38+
":s3:::My_legacy_bucket1/*"
3939
]
4040
]
4141
},
@@ -47,7 +47,7 @@
4747
{
4848
"Ref": "AWS::Partition"
4949
},
50-
":s3:::my_legacy_bucket3/*"
50+
":s3:::My_legacy_bucket3/*"
5151
]
5252
]
5353
}

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.js.snapshot/awscdks3integtestDefaultTestDeployAssert9DECDFBF.assets.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.js.snapshot/cdk.out

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.js.snapshot/integ.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.js.snapshot/manifest.json

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.js.snapshot/tree.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-legacy-naming.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import * as iam from 'aws-cdk-lib/aws-iam';
77
const app = new cdk.App();
88
const stack = new cdk.Stack(app, 'aws-cdk-s3-legacy-name-integ');
99

10-
const legacyBucketFromName = s3.Bucket.fromBucketName(stack, 'LegacyBucketFromName', 'my_legacy_bucket1');
10+
const legacyBucketFromName = s3.Bucket.fromBucketName(stack, 'LegacyBucketFromName', 'My_legacy_bucket1');
1111

12-
const legacyBucketFromArn = s3.Bucket.fromBucketArn(stack, 'LegacyBucketFromArn', 'arn:aws:s3:::my_legacy_bucket2');
12+
const legacyBucketFromArn = s3.Bucket.fromBucketArn(stack, 'LegacyBucketFromArn', 'arn:aws:s3:::My_legacy_bucket2');
1313

1414
const legacyBucketFromAttributes = s3.Bucket.fromBucketAttributes(stack, 'LegacyBucketFromAttributes', {
15-
bucketName: 'my_legacy_bucket3',
15+
bucketName: 'My_legacy_bucket3',
1616
});
1717

1818
const role = new iam.Role(stack, 'LegacyBucketRole', {
@@ -37,4 +37,3 @@ new IntegTest(app, 'aws-cdk-s3-integ-test', {
3737
// In the synthesized template, verify:
3838
// 1. The bucket names imported using different methods (Bucket.fromBucketName, Bucket.fromBucketArn, Bucket.fromBucketAttributes) are not modified and contain underscores.
3939
// 2. The policy attached to the role includes the correct bucket ARNs with underscores for all three imported buckets.
40-

packages/aws-cdk-lib/aws-s3/lib/bucket.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,24 +1897,37 @@ export class Bucket extends BucketBase {
18971897
if (bucketName.length < 3 || bucketName.length > 63) {
18981898
errors.push('Bucket name must be at least 3 and no more than 63 characters');
18991899
}
1900-
const charsetRegex = allowLegacyBucketNaming ? /[^a-z0-9._-]/ : /[^a-z0-9.-]/;
1901-
const charsetMatch = bucketName.match(charsetRegex);
1902-
if (charsetMatch) {
1903-
errors.push(`Bucket name must only contain lowercase characters and the symbols, period (.)${allowLegacyBucketNaming ? ', underscore (_), ' : ' '}and dash (-) `
1904-
+ `(offset: ${charsetMatch.index})`);
1900+
1901+
const illegalCharsetRegEx = allowLegacyBucketNaming ? /[^A-Za-z0-9._-]/ : /[^a-z0-9.-]/;
1902+
const allowedEdgeCharsetRegEx = allowLegacyBucketNaming ? /[A-Za-z0-9]/ : /[a-z0-9]/;
1903+
1904+
const illegalCharMatch = bucketName.match(illegalCharsetRegEx);
1905+
if (illegalCharMatch) {
1906+
errors.push(allowLegacyBucketNaming
1907+
? 'Bucket name must only contain uppercase or lowercase characters and the symbols, period (.), underscore (_), and dash (-)'
1908+
: 'Bucket name must only contain lowercase characters and the symbols, period (.) and dash (-)'
1909+
+ ` (offset: ${illegalCharMatch.index})`,
1910+
);
19051911
}
1906-
if (!/[a-z0-9]/.test(bucketName.charAt(0))) {
1907-
errors.push('Bucket name must start and end with a lowercase character or number '
1908-
+ '(offset: 0)');
1912+
if (!allowedEdgeCharsetRegEx.test(bucketName.charAt(0))) {
1913+
errors.push(allowLegacyBucketNaming
1914+
? 'Bucket name must start with an uppercase, lowercase character or number'
1915+
: 'Bucket name must start with a lowercase character or number'
1916+
+ ' (offset: 0)',
1917+
);
19091918
}
1910-
if (!/[a-z0-9]/.test(bucketName.charAt(bucketName.length - 1))) {
1911-
errors.push('Bucket name must start and end with a lowercase character or number '
1912-
+ `(offset: ${bucketName.length - 1})`);
1919+
if (!allowedEdgeCharsetRegEx.test(bucketName.charAt(bucketName.length - 1))) {
1920+
errors.push(allowLegacyBucketNaming
1921+
? 'Bucket name must end with an uppercase, lowercase character or number'
1922+
: 'Bucket name must end with a lowercase character or number'
1923+
+ ` (offset: ${bucketName.length - 1})`,
1924+
);
19131925
}
1926+
19141927
const consecSymbolMatch = bucketName.match(/\.-|-\.|\.\./);
19151928
if (consecSymbolMatch) {
1916-
errors.push('Bucket name must not have dash next to period, or period next to dash, or consecutive periods '
1917-
+ `(offset: ${consecSymbolMatch.index})`);
1929+
errors.push('Bucket name must not have dash next to period, or period next to dash, or consecutive periods'
1930+
+ ` (offset: ${consecSymbolMatch.index})`);
19181931
}
19191932
if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(bucketName)) {
19201933
errors.push('Bucket name must not resemble an IP address');

packages/aws-cdk-lib/aws-s3/test/bucket.test.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,8 @@ describe('bucket', () => {
193193
`Invalid S3 bucket name (value: ${bucket})`,
194194
'Bucket name must be at least 3 and no more than 63 characters',
195195
'Bucket name must only contain lowercase characters and the symbols, period (.) and dash (-) (offset: 5)',
196-
'Bucket name must start and end with a lowercase character or number (offset: 0)',
197-
`Bucket name must start and end with a lowercase character or number (offset: ${bucket.length - 1})`,
196+
'Bucket name must start with a lowercase character or number (offset: 0)',
197+
`Bucket name must end with a lowercase character or number (offset: ${bucket.length - 1})`,
198198
'Bucket name must not have dash next to period, or period next to dash, or consecutive periods (offset: 7)',
199199
].join(EOL);
200200

@@ -215,12 +215,30 @@ describe('bucket', () => {
215215
}).toThrow(/Bucket name must only contain lowercase characters and the symbols, period \(\.\) and dash \(-\)/);
216216
});
217217

218+
test('validateBucketName allows uppercase characters when allowLegacyBucketNaming=true', () => {
219+
expect(() => {
220+
s3.Bucket.validateBucketName('Test_Bucket_Name', true);
221+
}).not.toThrow();
222+
});
223+
224+
test('validateBucketName does not allow uppercase characters when allowLegacyBucketNaming=false', () => {
225+
expect(() => {
226+
s3.Bucket.validateBucketName('Test_Bucket_Name', false);
227+
}).toThrow(/Bucket name must only contain lowercase characters and the symbols, period \(\.\) and dash \(-\)/);
228+
});
229+
218230
test('validateBucketName does not allow underscore by default', () => {
219231
expect(() => {
220232
s3.Bucket.validateBucketName('test_bucket_name');
221233
}).toThrow(/Bucket name must only contain lowercase characters and the symbols, period \(\.\) and dash \(-\)/);
222234
});
223235

236+
test('validateBucketName does not allow uppercase characters by default', () => {
237+
expect(() => {
238+
s3.Bucket.validateBucketName('TestBucketName');
239+
}).toThrow(/Bucket name must only contain lowercase characters and the symbols, period \(\.\) and dash \(-\)/);
240+
});
241+
224242
test('fails if bucket name has less than 3 or more than 63 characters', () => {
225243
const stack = new cdk.Stack();
226244

@@ -3877,4 +3895,3 @@ describe('bucket', () => {
38773895
});
38783896
});
38793897
});
3880-

0 commit comments

Comments
 (0)