Skip to content

Commit 4944dfa

Browse files
ehiggsehiggs
authored
Support Last-Modified response header and support If-Modified-Since request header. (imgproxy#1147)
* Always return Last-Modified and support If-Modified-Since. * IMGPROXY_USE_LAST_MODIFIED config setting. IMGPROXY_USE_LAST_MODIFIED (default false) when enabled will return the Last-Modified time of the upstream image and also allow the support of the If-Modified-Since request header (returning a 304 if the image hasn't been modified). If-Modified-Since allows If-None-Match to take precedence. * Fixes based on DarthSim's feedback. 1. Don't worry about nil maps. 2. Fix a test now that we use the config.LastModifiedEnabled (and move it's location it he test file to a more sane place). 3. Update GCS transport code based on the refactoring of DarthSim. In this iteration, we pull the Updated time from the GCS object attributes and format them as a string. We then parse it in the notmodified module. Seems a bit silly to do it this way. If we agree on the approach here, then AWS and Azure can follow. * Support azure, fs, s3, and swift. * Grab the headers for If-Modified-Since and Last-Modified before parsing them. * Add tests for last-modified for fs. * Support Last-Modified being passed when streaming an upstream file. * Tests for Last-Modified for GCS and Azure * Support s3 and swift tests. Sadly fakes3 doesn't support Last-Modified * Test against forked gofakes3
1 parent 87faa5a commit 4944dfa

16 files changed

+472
-25
lines changed

‎config/config.go

+6
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ var (
123123
ETagEnabled bool
124124
ETagBuster string
125125

126+
LastModifiedEnabled bool
127+
126128
BaseURL string
127129

128130
Presets []string
@@ -310,6 +312,8 @@ func Reset() {
310312
ETagEnabled = false
311313
ETagBuster = ""
312314

315+
LastModifiedEnabled = false
316+
313317
BaseURL = ""
314318

315319
Presets = make([]string, 0)
@@ -508,6 +512,8 @@ func Configure() error {
508512
configurators.Bool(&ETagEnabled, "IMGPROXY_USE_ETAG")
509513
configurators.String(&ETagBuster, "IMGPROXY_ETAG_BUSTER")
510514

515+
configurators.Bool(&LastModifiedEnabled, "IMGPROXY_USE_LAST_MODIFIED")
516+
511517
configurators.String(&BaseURL, "IMGPROXY_BASE_URL")
512518

513519
configurators.StringSlice(&Presets, "IMGPROXY_PRESETS")

‎imagedata/download.go

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ var (
3434
"Cache-Control",
3535
"Expires",
3636
"ETag",
37+
"Last-Modified",
3738
}
3839

3940
// For tests

‎processing_handler.go

+19-2
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ func setCacheControl(rw http.ResponseWriter, force *time.Time, originHeaders map
9191
}
9292
}
9393

94+
func setLastModified(rw http.ResponseWriter, originHeaders map[string]string) {
95+
if config.LastModifiedEnabled {
96+
if val, ok := originHeaders["Last-Modified"]; ok && len(val) != 0 {
97+
rw.Header().Set("Last-Modified", val)
98+
}
99+
}
100+
}
101+
94102
func setVary(rw http.ResponseWriter) {
95103
if len(headerVaryValue) > 0 {
96104
rw.Header().Set("Vary", headerVaryValue)
@@ -118,6 +126,7 @@ func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, sta
118126
rw.Header().Set("Content-Disposition", contentDisposition)
119127

120128
setCacheControl(rw, po.Expires, originData.Headers)
129+
setLastModified(rw, originData.Headers)
121130
setVary(rw)
122131
setCanonical(rw, originURL)
123132

@@ -260,6 +269,12 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
260269
}
261270
}
262271

272+
if config.LastModifiedEnabled {
273+
if modifiedSince := r.Header.Get("If-Modified-Since"); len(modifiedSince) != 0 {
274+
imgRequestHeader.Set("If-Modified-Since", modifiedSince)
275+
}
276+
}
277+
263278
// The heavy part start here, so we need to restrict concurrency
264279
var processingSemToken *semaphore.Token
265280
func() {
@@ -299,8 +314,10 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
299314

300315
if err == nil {
301316
defer originData.Close()
302-
} else if nmErr, ok := err.(*imagedata.ErrorNotModified); ok && config.ETagEnabled {
303-
rw.Header().Set("ETag", etagHandler.GenerateExpectedETag())
317+
} else if nmErr, ok := err.(*imagedata.ErrorNotModified); ok {
318+
if config.ETagEnabled && len(etagHandler.ImageEtagExpected()) != 0 {
319+
rw.Header().Set("ETag", etagHandler.GenerateExpectedETag())
320+
}
304321
respondWithNotModified(reqID, r, rw, po, imageURL, nmErr.Headers)
305322
return
306323
} else {

‎processing_handler_test.go

+153
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"regexp"
1212
"strings"
1313
"testing"
14+
"time"
1415

1516
"github.com/imgproxy/imgproxy/v3/config"
1617
"github.com/imgproxy/imgproxy/v3/config/configurators"
@@ -550,6 +551,158 @@ func (s *ProcessingHandlerTestSuite) TestETagProcessingOptionsNotMatch() {
550551
require.Equal(s.T(), actualETag, res.Header.Get("ETag"))
551552
}
552553

554+
func (s *ProcessingHandlerTestSuite) TestLastModifiedEnabled() {
555+
config.LastModifiedEnabled = true
556+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
557+
rw.Header().Set("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT")
558+
rw.WriteHeader(200)
559+
rw.Write(s.readTestFile("test1.png"))
560+
}))
561+
defer ts.Close()
562+
563+
rw := s.send("/unsafe/rs:fill:4:4/plain/" + ts.URL)
564+
res := rw.Result()
565+
566+
require.Equal(s.T(), "Wed, 21 Oct 2015 07:28:00 GMT", res.Header.Get("Last-Modified"))
567+
}
568+
569+
func (s *ProcessingHandlerTestSuite) TestLastModifiedDisabled() {
570+
config.LastModifiedEnabled = false
571+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
572+
rw.Header().Set("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT")
573+
rw.WriteHeader(200)
574+
rw.Write(s.readTestFile("test1.png"))
575+
}))
576+
defer ts.Close()
577+
578+
rw := s.send("/unsafe/rs:fill:4:4/plain/" + ts.URL)
579+
res := rw.Result()
580+
581+
require.Equal(s.T(), "", res.Header.Get("Last-Modified"))
582+
}
583+
584+
func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqExactMatchLastModifiedDisabled() {
585+
config.LastModifiedEnabled = false
586+
data := s.readTestFile("test1.png")
587+
lastModified := "Wed, 21 Oct 2015 07:28:00 GMT"
588+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
589+
modifiedSince := r.Header.Get("If-Modified-Since")
590+
require.Equal(s.T(), "", modifiedSince)
591+
rw.WriteHeader(200)
592+
rw.Write(data)
593+
594+
}))
595+
defer ts.Close()
596+
597+
header := make(http.Header)
598+
header.Set("If-Modified-Since", lastModified)
599+
rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header)
600+
res := rw.Result()
601+
602+
require.Equal(s.T(), 200, res.StatusCode)
603+
}
604+
func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqExactMatchLastModifiedEnabled() {
605+
config.LastModifiedEnabled = true
606+
lastModified := "Wed, 21 Oct 2015 07:28:00 GMT"
607+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
608+
modifiedSince := r.Header.Get("If-Modified-Since")
609+
require.Equal(s.T(), lastModified, modifiedSince)
610+
rw.WriteHeader(304)
611+
}))
612+
defer ts.Close()
613+
614+
header := make(http.Header)
615+
header.Set("If-Modified-Since", lastModified)
616+
rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header)
617+
res := rw.Result()
618+
619+
require.Equal(s.T(), 304, res.StatusCode)
620+
}
621+
622+
func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareMoreRecentLastModifiedDisabled() {
623+
data := s.readTestFile("test1.png")
624+
config.LastModifiedEnabled = false
625+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
626+
modifiedSince := r.Header.Get("If-Modified-Since")
627+
require.Equal(s.T(), modifiedSince, "")
628+
rw.WriteHeader(200)
629+
rw.Write(data)
630+
}))
631+
defer ts.Close()
632+
633+
recentTimestamp := "Thu, 25 Feb 2021 01:45:00 GMT"
634+
635+
header := make(http.Header)
636+
header.Set("If-Modified-Since", recentTimestamp)
637+
rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header)
638+
res := rw.Result()
639+
640+
require.Equal(s.T(), 200, res.StatusCode)
641+
}
642+
func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareMoreRecentLastModifiedEnabled() {
643+
config.LastModifiedEnabled = true
644+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
645+
fileLastModified, _ := time.Parse(http.TimeFormat, "Wed, 21 Oct 2015 07:28:00 GMT")
646+
modifiedSince := r.Header.Get("If-Modified-Since")
647+
parsedModifiedSince, err := time.Parse(http.TimeFormat, modifiedSince)
648+
require.Nil(s.T(), err)
649+
require.True(s.T(), fileLastModified.Before(parsedModifiedSince))
650+
rw.WriteHeader(304)
651+
}))
652+
defer ts.Close()
653+
654+
recentTimestamp := "Thu, 25 Feb 2021 01:45:00 GMT"
655+
656+
header := make(http.Header)
657+
header.Set("If-Modified-Since", recentTimestamp)
658+
rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header)
659+
res := rw.Result()
660+
661+
require.Equal(s.T(), 304, res.StatusCode)
662+
}
663+
func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareTooOldLastModifiedDisabled() {
664+
config.LastModifiedEnabled = false
665+
data := s.readTestFile("test1.png")
666+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
667+
modifiedSince := r.Header.Get("If-Modified-Since")
668+
require.Equal(s.T(), modifiedSince, "")
669+
rw.WriteHeader(200)
670+
rw.Write(data)
671+
}))
672+
defer ts.Close()
673+
674+
oldTimestamp := "Tue, 01 Oct 2013 17:31:00 GMT"
675+
676+
header := make(http.Header)
677+
header.Set("If-Modified-Since", oldTimestamp)
678+
rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header)
679+
res := rw.Result()
680+
681+
require.Equal(s.T(), 200, res.StatusCode)
682+
}
683+
func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareTooOldLastModifiedEnabled() {
684+
config.LastModifiedEnabled = true
685+
data := s.readTestFile("test1.png")
686+
ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
687+
fileLastModified, _ := time.Parse(http.TimeFormat, "Wed, 21 Oct 2015 07:28:00 GMT")
688+
modifiedSince := r.Header.Get("If-Modified-Since")
689+
parsedModifiedSince, err := time.Parse(http.TimeFormat, modifiedSince)
690+
require.Nil(s.T(), err)
691+
require.True(s.T(), fileLastModified.After(parsedModifiedSince))
692+
rw.WriteHeader(200)
693+
rw.Write(data)
694+
}))
695+
defer ts.Close()
696+
697+
oldTimestamp := "Tue, 01 Oct 2013 17:31:00 GMT"
698+
699+
header := make(http.Header)
700+
header.Set("If-Modified-Since", oldTimestamp)
701+
rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header)
702+
res := rw.Result()
703+
704+
require.Equal(s.T(), 200, res.StatusCode)
705+
}
553706
func TestProcessingHandler(t *testing.T) {
554707
suite.Run(t, new(ProcessingHandlerTestSuite))
555708
}

‎stream.go

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ var (
3737
"Content-Encoding",
3838
"Content-Range",
3939
"Accept-Ranges",
40+
"Last-Modified",
4041
}
4142

4243
streamBufPool = sync.Pool{

‎transport/azure/azure.go

+4
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ func (t transport) RoundTrip(req *http.Request) (*http.Response, error) {
123123
etag := string(*result.ETag)
124124
header.Set("ETag", etag)
125125
}
126+
if config.LastModifiedEnabled && result.LastModified != nil {
127+
lastModified := result.LastModified.Format(http.TimeFormat)
128+
header.Set("Last-Modified", lastModified)
129+
}
126130

127131
if resp := notmodified.Response(req, header); resp != nil {
128132
if result.Body != nil {

‎transport/azure/azure_test.go

+47-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/http/httptest"
66
"os"
77
"testing"
8+
"time"
89

910
"github.com/sirupsen/logrus"
1011
"github.com/stretchr/testify/require"
@@ -16,9 +17,10 @@ import (
1617
type AzureTestSuite struct {
1718
suite.Suite
1819

19-
server *httptest.Server
20-
transport http.RoundTripper
21-
etag string
20+
server *httptest.Server
21+
transport http.RoundTripper
22+
etag string
23+
lastModified time.Time
2224
}
2325

2426
func (s *AzureTestSuite) SetupSuite() {
@@ -27,11 +29,13 @@ func (s *AzureTestSuite) SetupSuite() {
2729
logrus.SetOutput(os.Stdout)
2830

2931
s.etag = "testetag"
32+
s.lastModified, _ = time.Parse(http.TimeFormat, "Wed, 21 Oct 2015 07:28:00 GMT")
3033

3134
s.server = httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
3235
require.Equal(s.T(), "/test/foo/test.png", r.URL.Path)
3336

3437
rw.Header().Set("Etag", s.etag)
38+
rw.Header().Set("Last-Modified", s.lastModified.Format(http.TimeFormat))
3539
rw.WriteHeader(200)
3640
rw.Write(data)
3741
}))
@@ -91,6 +95,46 @@ func (s *AzureTestSuite) TestRoundTripWithUpdatedETagReturns200() {
9195
require.Equal(s.T(), http.StatusOK, response.StatusCode)
9296
}
9397

98+
func (s *AzureTestSuite) TestRoundTripWithLastModifiedDisabledReturns200() {
99+
config.LastModifiedEnabled = false
100+
request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil)
101+
102+
response, err := s.transport.RoundTrip(request)
103+
require.Nil(s.T(), err)
104+
require.Equal(s.T(), 200, response.StatusCode)
105+
}
106+
107+
func (s *AzureTestSuite) TestRoundTripWithLastModifiedEnabled() {
108+
config.LastModifiedEnabled = true
109+
request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil)
110+
111+
response, err := s.transport.RoundTrip(request)
112+
require.Nil(s.T(), err)
113+
require.Equal(s.T(), 200, response.StatusCode)
114+
require.Equal(s.T(), s.lastModified.Format(http.TimeFormat), response.Header.Get("Last-Modified"))
115+
}
116+
117+
func (s *AzureTestSuite) TestRoundTripWithIfModifiedSinceReturns304() {
118+
config.LastModifiedEnabled = true
119+
120+
request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil)
121+
request.Header.Set("If-Modified-Since", s.lastModified.Format(http.TimeFormat))
122+
123+
response, err := s.transport.RoundTrip(request)
124+
require.Nil(s.T(), err)
125+
require.Equal(s.T(), http.StatusNotModified, response.StatusCode)
126+
}
127+
128+
func (s *AzureTestSuite) TestRoundTripWithUpdatedLastModifiedReturns200() {
129+
config.LastModifiedEnabled = true
130+
131+
request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil)
132+
request.Header.Set("If-Modified-Since", s.lastModified.Add(-24*time.Hour).Format(http.TimeFormat))
133+
134+
response, err := s.transport.RoundTrip(request)
135+
require.Nil(s.T(), err)
136+
require.Equal(s.T(), http.StatusOK, response.StatusCode)
137+
}
94138
func TestAzureTransport(t *testing.T) {
95139
suite.Run(t, new(AzureTestSuite))
96140
}

‎transport/fs/fs.go

+5
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ func (t transport) RoundTrip(req *http.Request) (resp *http.Response, err error)
7878
etag := BuildEtag(req.URL.Path, fi)
7979
header.Set("ETag", etag)
8080
}
81+
82+
if config.LastModifiedEnabled {
83+
lastModified := fi.ModTime().Format(http.TimeFormat)
84+
header.Set("Last-Modified", lastModified)
85+
}
8186
}
8287

8388
if resp := notmodified.Response(req, header); resp != nil {

0 commit comments

Comments
 (0)