cshum / imagor

Fast, secure image processing server and Go library, using libvips
Apache License 2.0
3.46k stars 138 forks source link

Question: is it possible to have cacheable responses whilst using the expires() filter? #375

Closed headegg closed 1 year ago

headegg commented 1 year ago

HI @cshum. First off, thanks for such a fantastic project.

I'm using signed urls containing the expires() filter. I want the browser to be able to cache the results up until the expiry timestamp (I'm setting to "end of today"). But I'm always getting Cache-Control: private, no-cache, no-store, must-revalidate in every response. I have no Go experience, but looking at the code and TestExpires test, it appears that this is intentional and any request using the expires() filter should be treated as if the client sent Cache-Control: no-cache in the request?

Have I understood this correctly and do you think it would be a reasonable feature request to be able to set Cache-Control: and Expires: to cache until the timestamp specified in the expires() filter?

Many thanks.

cshum commented 1 year ago

Hi the expires() filter is set no-cache intentionally because it is time sensitive. But you are correct that it is reasonable to set Expires: to cache until the timestamp specified, a detail being overlooked.

headegg commented 1 year ago

Thanks very much for the response @cshum. I wonder if you'd be good enough to look at this proposed pull request.

Thanks again, Alex

headegg commented 1 year ago

Hi. Really sorry the vips tests failed on the pull request github actions. Are the test failures really due to the changes I made in imagor.go? I can't see the connection. I must admit, before opening the PR, I had just been running imagor_test.go and not the full make test, which I have now tried to do.

I tried to follow the steps in .github.workflow.test.yml on my Ubuntu 22 laptop. I installed the dependencies and built and installed libvips 8.14.2 locally. But I can't even get all the test cases in processor_test.go to pass, even on the master branch. Many of the tests run while lots fail e.g. png/tiff/gif tests seem to be working but I'm getting "image mismatch" errors with other formats.

Apologies for the annoying newbie questions but where should I look next to try and get the PR pipeline working?

Thanks :)

cshum commented 1 year ago

Hi @headegg you may test the docker build at master branch: https://github.com/cshum/imagor/pkgs/container/imagor/112906715?tag=master

ghcr.io/cshum/imagor@sha256:55bfeaa55aa3cdbd29a963801305236078a010ebc668de18859147fa09a3964c
headegg commented 1 year ago

Awesome! Thanks a million @cshum.

headegg commented 1 year ago

Hi @cshum. I tried the new :master image with and without the expire() filter and the cache headers are working as expected. I also tried specifying "cache-control: no-cache" request header which continues to work as before and you can also now send "cache-control: private" in the request and the response will have "private" instead of "public" in the cache-control header. So, all good!

Thanks very much for seeing this change through to master.

Couple of questions:

Thanks again and keep up the amazing work.

cshum commented 1 year ago

Changes available in imagor v1.4.7 and imagorvideo v0.4.7