Skip to content

Commit

Permalink
fix(aws-cloudfront): distribution comment length not validated (aws#1…
Browse files Browse the repository at this point in the history
…4020) (aws#14094)

fixes aws#14020

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
minuz authored Apr 13, 2021
1 parent 9042bf7 commit 54fddc6
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 2 deletions.
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-cloudfront/lib/distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,12 @@ export class Distribution extends Resource implements IDistribution {
this.certificate = props.certificate;
this.errorResponses = props.errorResponses ?? [];

// Comments have an undocumented limit of 128 characters
const trimmedComment =
props.comment && props.comment.length > 128
? `${props.comment.substr(0, 128 - 3)}...`
: props.comment;

const distribution = new CfnDistribution(this, 'Resource', {
distributionConfig: {
enabled: props.enabled ?? true,
Expand All @@ -287,7 +293,7 @@ export class Distribution extends Resource implements IDistribution {
defaultCacheBehavior: this.defaultBehavior._renderBehavior(),
aliases: props.domainNames,
cacheBehaviors: Lazy.any({ produce: () => this.renderCacheBehaviors() }),
comment: props.comment,
comment: trimmedComment,
customErrorResponses: this.renderErrorResponses(),
defaultRootObject: props.defaultRootObject,
httpVersion: props.httpVersion ?? HttpVersion.HTTP2,
Expand Down
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -769,8 +769,14 @@ export class CloudFrontWebDistribution extends cdk.Resource implements IDistribu
constructor(scope: Construct, id: string, props: CloudFrontWebDistributionProps) {
super(scope, id);

// Comments have an undocumented limit of 128 characters
const trimmedComment =
props.comment && props.comment.length > 128
? `${props.comment.substr(0, 128 - 3)}...`
: props.comment;

let distributionConfig: CfnDistribution.DistributionConfigProperty = {
comment: props.comment,
comment: trimmedComment,
enabled: true,
defaultRootObject: props.defaultRootObject ?? 'index.html',
httpVersion: props.httpVersion || HttpVersion.HTTP2,
Expand Down
34 changes: 34 additions & 0 deletions packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,40 @@ test('exhaustive example of props renders correctly', () => {
});
});

test('ensure comment prop is not greater than max lenght', () => {
const origin = defaultOrigin();
new Distribution(stack, 'MyDist', {
defaultBehavior: { origin },
comment: `Adding a comment longer than 128 characters should be trimmed and added the
ellipsis so a user would know there was more to read and everything beyond this point should not show up`,
});

expect(stack).toHaveResource('AWS::CloudFront::Distribution', {
DistributionConfig: {
DefaultCacheBehavior: {
CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6',
Compress: true,
TargetOriginId: 'StackMyDistOrigin1D6D5E535',
ViewerProtocolPolicy: 'allow-all',
},
Comment: `Adding a comment longer than 128 characters should be trimmed and added the
ellipsis so a user would know there was more to ...`,
Enabled: true,
HttpVersion: 'http2',
IPV6Enabled: true,
Origins: [
{
DomainName: 'www.example.com',
Id: 'StackMyDistOrigin1D6D5E535',
CustomOriginConfig: {
OriginProtocolPolicy: 'https-only',
},
},
],
},
});
});

describe('multiple behaviors', () => {

test('a second behavior can\'t be specified with the catch-all path pattern', () => {
Expand Down
72 changes: 72 additions & 0 deletions packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,78 @@ nodeunitShim({
test.done();
},

'ensure long comments will not break the distribution'(test: Test) {
const stack = new cdk.Stack();
const sourceBucket = new s3.Bucket(stack, 'Bucket');

new CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', {
comment: `Adding a comment longer than 128 characters should be trimmed and
added the ellipsis so a user would know there was more to read and everything beyond this point should not show up`,
originConfigs: [
{
s3OriginSource: {
s3BucketSource: sourceBucket,
},
behaviors: [
{
isDefaultBehavior: true,
},
],
},
],
});

expect(stack).toMatch({
Resources: {
Bucket83908E77: {
Type: 'AWS::S3::Bucket',
DeletionPolicy: 'Retain',
UpdateReplacePolicy: 'Retain',
},
AnAmazingWebsiteProbablyCFDistribution47E3983B: {
Type: 'AWS::CloudFront::Distribution',
Properties: {
DistributionConfig: {
DefaultRootObject: 'index.html',
Origins: [
{
ConnectionAttempts: 3,
ConnectionTimeout: 10,
DomainName: {
'Fn::GetAtt': ['Bucket83908E77', 'RegionalDomainName'],
},
Id: 'origin1',
S3OriginConfig: {},
},
],
ViewerCertificate: {
CloudFrontDefaultCertificate: true,
},
PriceClass: 'PriceClass_100',
DefaultCacheBehavior: {
AllowedMethods: ['GET', 'HEAD'],
CachedMethods: ['GET', 'HEAD'],
TargetOriginId: 'origin1',
ViewerProtocolPolicy: 'redirect-to-https',
ForwardedValues: {
QueryString: false,
Cookies: { Forward: 'none' },
},
Compress: true,
},
Comment: `Adding a comment longer than 128 characters should be trimmed and
added the ellipsis so a user would know there was more to ...`,
Enabled: true,
IPV6Enabled: true,
HttpVersion: 'http2',
},
},
},
},
});
test.done();
},

'distribution with bucket and OAI'(test: Test) {
const stack = new cdk.Stack();
const s3BucketSource = new s3.Bucket(stack, 'Bucket');
Expand Down

0 comments on commit 54fddc6

Please sign in to comment.