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

Cache-Control not automatically generating ETags and Last-Modified, RE: Image Optimization Next10 #18563

Closed
Braedencraig opened this issue Oct 30, 2020 · 2 comments · Fixed by #18986
Assignees
Milestone

Comments

@Braedencraig
Copy link

Braedencraig commented Oct 30, 2020

Bug report

When using the new Image Optimization feature, it appears as though even though Cache-Control is being set, the response headers aren't including what should be automatically generated.

Describe the bug

As an example for all my static images, I am adding cache-control.
e.g.

const setHeadersForImages = (req, res) => {
    res.setHeader('Cache-Control', 'public,max-age=2592000000,immutable')
    return handle(req, res)
}

//This is how I was adding the headers PRE NEW IMAGE OPTIMIZATION
server.get('*.jpg', (req, res) => setHeadersForImages(req, res))
server.get('*.svg', (req, res) => setHeadersForImages(req, res))
server.get('*.png', (req, res) => setHeadersForImages(req, res))

//This is how I am adding the headers POST NEW IMAGE OPTIMIZATION
server.get(/^\/_next\/image/, (req, res) => setHeadersForImages(req, res))

I can see in the response headers that the cache control is being added just fine. But pre using the new Image Optimization, my response headers looked like:

cache-control: max-age=31536000
content-length: 774746
content-type: image/png
date: Fri, 30 Oct 2020 20:32:17 GMT
etag: "92703e9f4803a0755947a4b150c7d9af"
last-modified: Mon, 28 Sep 2020 14:56:05 GMT

and now while using the new feature they look like.

Cache-Control: public,max-age=2592000000,immutable
Connection: keep-alive
Content-Type: image/webp
Date: Fri, 30 Oct 2020 20:39:42 GMT
Transfer-Encoding: chunked
X-Powered-By: Express

The ETag and date last modified is missing...hmmmm.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Implement a new Image Optimization component, using a static image.
  2. Ensure in server.js you are doing
const setHeadersForImages = (req, res) => {
    // res.setHeader('Last-Modified', (new Date()).toUTCString())
    // res.setHeader('ETag', etag(req.originalUrl, { weak: true }))
    res.setHeader('Cache-Control', 'public,max-age=2592000000,immutable')

    return handle(req, res)
}


 server.get(/^\/_next\/image/, (req, res) => setHeadersForImages(req, res))
  1. Check your response headers for the image in the network tab. See that ETag and Last-Modified are not present.

Expected behavior

Expected behaviour is that the ETag and Last-Modified would be generated as they normally are when normal images are used, using the normal img tag.

System information

  • OS: macOS
  • Browser Chrome
  • Version of Next.js: [e.g. 10.0.0]
  • Version of Node.js: [e.g. 12.18.3]
@klaaz0r
Copy link

klaaz0r commented Oct 31, 2020

This also is likely the result of the Serve static assets with an efficient cache policy error/warning in google's lighthouse

@Timer Timer added the kind: bug label Nov 1, 2020
@Timer Timer modified the milestones: iteration 12, iteration 11 Nov 1, 2020
@Timer Timer assigned styfle and Timer Nov 1, 2020
@styfle styfle added point: 3 and removed point: 2 labels Nov 9, 2020
@kodiakhq kodiakhq bot closed this as completed in #18986 Nov 10, 2020
kodiakhq bot pushed a commit that referenced this issue Nov 10, 2020
Fixes #18563 by adding the etag header to the optimized image response.

This does _not_ change the expireAt (TTL) for cached files on the server, which still uses the max-age of the upstream response.

The new file format on disk for cached image files is the following:

```
.next/cache/images/<HASHED_QUERYSTRING>/<EXPIREAT>.<ETAG>.<EXT>
```
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants