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

aws-s3-deployment: CacheControl.sMaxAge(t) returns wrong value #6292

Closed
d2kx opened this issue Feb 16, 2020 · 1 comment · Fixed by #8434
Closed

aws-s3-deployment: CacheControl.sMaxAge(t) returns wrong value #6292

d2kx opened this issue Feb 16, 2020 · 1 comment · Fixed by #8434
Assignees
Labels
@aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. p1

Comments

@d2kx
Copy link

d2kx commented Feb 16, 2020

Reproduction Steps

const deployment = new BucketDeployment(this, "BucketDeployment", {
  cacheControl: [
    CacheControl.setPublic(),
    CacheControl.maxAge(cdk.Duration.seconds(0)),
    CacheControl.sMaxAge(cdk.Duration.days(365))
  ]
});

// Cache-Control: 'public, max-age=0, s-max-age=31536000'

Error Log

CacheControl.sMaxAge(t) returns s-max-age=<t>, which gets ignored by CloudFront & everyone else.

It should have been s-maxage=<t>, without the '-' in between 'max age'. See below.

Environment

  • CLI Version : 1.24.0
  • Framework Version: 1.24.0
  • OS : Windows 10
  • Language : German

Other

This is regarding aws-s3-deployment: CacheControl.sMaxAge(t), see CDK API Doc

I am not too surprised about this issue, since the Cache-Control standard is a bit weird/inconsistent, because it is max-age with a '-' on the browser side, but s-maxage without a '-' for the proxy side. s-max-age as it is being set by CacheControl.sMaxAge(t) is being ignored by CloudFront and other web servers, as it doesn't exist.

It can currently be worked around by doing CacheControl.fromString('s-maxage=<t>').

See
Cache-Control, MDN
Cache-Control, AWS CloudFront documentation


This is 🐛 Bug Report

@d2kx d2kx added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 16, 2020
@SomayaB SomayaB added the @aws-cdk/aws-s3 Related to Amazon S3 label Feb 17, 2020
@skinny85
Copy link
Contributor

This also needs to be fixed in the S3DeployAction in the @aws-cdk/aws-codepipeline module.

@iliapolo iliapolo added the p1 label Mar 3, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Mar 5, 2020
@mergify mergify bot closed this as completed in #8434 Jun 8, 2020
mergify bot pushed a commit that referenced this issue Jun 8, 2020
Both the aws-s3-deployment and aws-codepipeline-actions CacheControl class uses
"s-max-age" instead of the correct "s-maxage". This change fixes to the correct
header value.

fixes #6292


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants