Skip to content

Set cache-control headers for file uploads to align with DAM policy #3820

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dkarnutsch
Copy link
Contributor

No description provided.

@auto-assign auto-assign bot requested a review from johnnyomair May 5, 2025 12:54
@@ -74,6 +74,7 @@ export function createFileUploadsDownloadController(options: { public: boolean }
"content-type": file.mimetype,
"last-modified": file.updatedAt?.toUTCString(),
"content-length": file.size,
"cache-control": options.public ? "max-age=86400, public" : "max-age=31536000, private", // same as DAM
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated download URLs have a 1 h timeout. Shouldn't this be reflected here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I didn't see that. So one hour might also be wrong as the URL might be called anytime from hash creation up to the lifetime.

What do you suggest?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Options:

  1. Calculate the remaining time and set it as max-age for both
  2. Set no-store (like we did for download folders)
  3. Set private and allow infinite caching

I'm not sure which option is the best, but all are fine probably.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this should be cached by CDN: 1️⃣

if we don't care about cdn caching: 2️⃣

As createDownloadUrl create a new, unique url for every usage, CDN cache can't be used anyway, It would just fill up with "garbage". For that reason 2️⃣ (or 3️⃣ is also ok)

(and the same should be applied to image function below)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants