From 4944dfab30fb3ff66f758af6f13dc7871ace1622 Mon Sep 17 00:00:00 2001 From: Ewan Higgs Date: Wed, 3 May 2023 17:21:46 +0200 Subject: [PATCH] Support Last-Modified response header and support If-Modified-Since request header. (#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 --- config/config.go | 6 ++ imagedata/download.go | 1 + processing_handler.go | 21 +++- processing_handler_test.go | 153 +++++++++++++++++++++++++++ stream.go | 1 + transport/azure/azure.go | 4 + transport/azure/azure_test.go | 50 ++++++++- transport/fs/fs.go | 5 + transport/fs/fs_test.go | 43 +++++++- transport/gcs/gcs.go | 9 +- transport/gcs/gcs_test.go | 47 +++++++- transport/notmodified/notmodified.go | 22 ++++ transport/s3/s3.go | 17 ++- transport/s3/s3_test.go | 51 ++++++++- transport/swift/swift.go | 12 ++- transport/swift/swift_test.go | 55 +++++++++- 16 files changed, 472 insertions(+), 25 deletions(-) diff --git a/config/config.go b/config/config.go index c91f9c94..d7de8890 100644 --- a/config/config.go +++ b/config/config.go @@ -123,6 +123,8 @@ var ( ETagEnabled bool ETagBuster string + LastModifiedEnabled bool + BaseURL string Presets []string @@ -310,6 +312,8 @@ func Reset() { ETagEnabled = false ETagBuster = "" + LastModifiedEnabled = false + BaseURL = "" Presets = make([]string, 0) @@ -508,6 +512,8 @@ func Configure() error { configurators.Bool(&ETagEnabled, "IMGPROXY_USE_ETAG") configurators.String(&ETagBuster, "IMGPROXY_ETAG_BUSTER") + configurators.Bool(&LastModifiedEnabled, "IMGPROXY_USE_LAST_MODIFIED") + configurators.String(&BaseURL, "IMGPROXY_BASE_URL") configurators.StringSlice(&Presets, "IMGPROXY_PRESETS") diff --git a/imagedata/download.go b/imagedata/download.go index 7ee6deb5..e46c92da 100644 --- a/imagedata/download.go +++ b/imagedata/download.go @@ -34,6 +34,7 @@ var ( "Cache-Control", "Expires", "ETag", + "Last-Modified", } // For tests diff --git a/processing_handler.go b/processing_handler.go index 4dec9c04..a913da4a 100644 --- a/processing_handler.go +++ b/processing_handler.go @@ -91,6 +91,14 @@ func setCacheControl(rw http.ResponseWriter, force *time.Time, originHeaders map } } +func setLastModified(rw http.ResponseWriter, originHeaders map[string]string) { + if config.LastModifiedEnabled { + if val, ok := originHeaders["Last-Modified"]; ok && len(val) != 0 { + rw.Header().Set("Last-Modified", val) + } + } +} + func setVary(rw http.ResponseWriter) { if len(headerVaryValue) > 0 { rw.Header().Set("Vary", headerVaryValue) @@ -118,6 +126,7 @@ func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, sta rw.Header().Set("Content-Disposition", contentDisposition) setCacheControl(rw, po.Expires, originData.Headers) + setLastModified(rw, originData.Headers) setVary(rw) setCanonical(rw, originURL) @@ -260,6 +269,12 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) { } } + if config.LastModifiedEnabled { + if modifiedSince := r.Header.Get("If-Modified-Since"); len(modifiedSince) != 0 { + imgRequestHeader.Set("If-Modified-Since", modifiedSince) + } + } + // The heavy part start here, so we need to restrict concurrency var processingSemToken *semaphore.Token func() { @@ -299,8 +314,10 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) { if err == nil { defer originData.Close() - } else if nmErr, ok := err.(*imagedata.ErrorNotModified); ok && config.ETagEnabled { - rw.Header().Set("ETag", etagHandler.GenerateExpectedETag()) + } else if nmErr, ok := err.(*imagedata.ErrorNotModified); ok { + if config.ETagEnabled && len(etagHandler.ImageEtagExpected()) != 0 { + rw.Header().Set("ETag", etagHandler.GenerateExpectedETag()) + } respondWithNotModified(reqID, r, rw, po, imageURL, nmErr.Headers) return } else { diff --git a/processing_handler_test.go b/processing_handler_test.go index d5db67f2..abafff4e 100644 --- a/processing_handler_test.go +++ b/processing_handler_test.go @@ -11,6 +11,7 @@ import ( "regexp" "strings" "testing" + "time" "github.com/imgproxy/imgproxy/v3/config" "github.com/imgproxy/imgproxy/v3/config/configurators" @@ -550,6 +551,158 @@ func (s *ProcessingHandlerTestSuite) TestETagProcessingOptionsNotMatch() { require.Equal(s.T(), actualETag, res.Header.Get("ETag")) } +func (s *ProcessingHandlerTestSuite) TestLastModifiedEnabled() { + config.LastModifiedEnabled = true + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT") + rw.WriteHeader(200) + rw.Write(s.readTestFile("test1.png")) + })) + defer ts.Close() + + rw := s.send("/unsafe/rs:fill:4:4/plain/" + ts.URL) + res := rw.Result() + + require.Equal(s.T(), "Wed, 21 Oct 2015 07:28:00 GMT", res.Header.Get("Last-Modified")) +} + +func (s *ProcessingHandlerTestSuite) TestLastModifiedDisabled() { + config.LastModifiedEnabled = false + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT") + rw.WriteHeader(200) + rw.Write(s.readTestFile("test1.png")) + })) + defer ts.Close() + + rw := s.send("/unsafe/rs:fill:4:4/plain/" + ts.URL) + res := rw.Result() + + require.Equal(s.T(), "", res.Header.Get("Last-Modified")) +} + +func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqExactMatchLastModifiedDisabled() { + config.LastModifiedEnabled = false + data := s.readTestFile("test1.png") + lastModified := "Wed, 21 Oct 2015 07:28:00 GMT" + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + modifiedSince := r.Header.Get("If-Modified-Since") + require.Equal(s.T(), "", modifiedSince) + rw.WriteHeader(200) + rw.Write(data) + + })) + defer ts.Close() + + header := make(http.Header) + header.Set("If-Modified-Since", lastModified) + rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header) + res := rw.Result() + + require.Equal(s.T(), 200, res.StatusCode) +} +func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqExactMatchLastModifiedEnabled() { + config.LastModifiedEnabled = true + lastModified := "Wed, 21 Oct 2015 07:28:00 GMT" + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + modifiedSince := r.Header.Get("If-Modified-Since") + require.Equal(s.T(), lastModified, modifiedSince) + rw.WriteHeader(304) + })) + defer ts.Close() + + header := make(http.Header) + header.Set("If-Modified-Since", lastModified) + rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header) + res := rw.Result() + + require.Equal(s.T(), 304, res.StatusCode) +} + +func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareMoreRecentLastModifiedDisabled() { + data := s.readTestFile("test1.png") + config.LastModifiedEnabled = false + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + modifiedSince := r.Header.Get("If-Modified-Since") + require.Equal(s.T(), modifiedSince, "") + rw.WriteHeader(200) + rw.Write(data) + })) + defer ts.Close() + + recentTimestamp := "Thu, 25 Feb 2021 01:45:00 GMT" + + header := make(http.Header) + header.Set("If-Modified-Since", recentTimestamp) + rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header) + res := rw.Result() + + require.Equal(s.T(), 200, res.StatusCode) +} +func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareMoreRecentLastModifiedEnabled() { + config.LastModifiedEnabled = true + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + fileLastModified, _ := time.Parse(http.TimeFormat, "Wed, 21 Oct 2015 07:28:00 GMT") + modifiedSince := r.Header.Get("If-Modified-Since") + parsedModifiedSince, err := time.Parse(http.TimeFormat, modifiedSince) + require.Nil(s.T(), err) + require.True(s.T(), fileLastModified.Before(parsedModifiedSince)) + rw.WriteHeader(304) + })) + defer ts.Close() + + recentTimestamp := "Thu, 25 Feb 2021 01:45:00 GMT" + + header := make(http.Header) + header.Set("If-Modified-Since", recentTimestamp) + rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header) + res := rw.Result() + + require.Equal(s.T(), 304, res.StatusCode) +} +func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareTooOldLastModifiedDisabled() { + config.LastModifiedEnabled = false + data := s.readTestFile("test1.png") + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + modifiedSince := r.Header.Get("If-Modified-Since") + require.Equal(s.T(), modifiedSince, "") + rw.WriteHeader(200) + rw.Write(data) + })) + defer ts.Close() + + oldTimestamp := "Tue, 01 Oct 2013 17:31:00 GMT" + + header := make(http.Header) + header.Set("If-Modified-Since", oldTimestamp) + rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header) + res := rw.Result() + + require.Equal(s.T(), 200, res.StatusCode) +} +func (s *ProcessingHandlerTestSuite) TestModifiedSinceReqCompareTooOldLastModifiedEnabled() { + config.LastModifiedEnabled = true + data := s.readTestFile("test1.png") + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + fileLastModified, _ := time.Parse(http.TimeFormat, "Wed, 21 Oct 2015 07:28:00 GMT") + modifiedSince := r.Header.Get("If-Modified-Since") + parsedModifiedSince, err := time.Parse(http.TimeFormat, modifiedSince) + require.Nil(s.T(), err) + require.True(s.T(), fileLastModified.After(parsedModifiedSince)) + rw.WriteHeader(200) + rw.Write(data) + })) + defer ts.Close() + + oldTimestamp := "Tue, 01 Oct 2013 17:31:00 GMT" + + header := make(http.Header) + header.Set("If-Modified-Since", oldTimestamp) + rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header) + res := rw.Result() + + require.Equal(s.T(), 200, res.StatusCode) +} func TestProcessingHandler(t *testing.T) { suite.Run(t, new(ProcessingHandlerTestSuite)) } diff --git a/stream.go b/stream.go index 692e81a9..2cb20ff2 100644 --- a/stream.go +++ b/stream.go @@ -37,6 +37,7 @@ var ( "Content-Encoding", "Content-Range", "Accept-Ranges", + "Last-Modified", } streamBufPool = sync.Pool{ diff --git a/transport/azure/azure.go b/transport/azure/azure.go index 3e29a0a7..e6a4d928 100644 --- a/transport/azure/azure.go +++ b/transport/azure/azure.go @@ -123,6 +123,10 @@ func (t transport) RoundTrip(req *http.Request) (*http.Response, error) { etag := string(*result.ETag) header.Set("ETag", etag) } + if config.LastModifiedEnabled && result.LastModified != nil { + lastModified := result.LastModified.Format(http.TimeFormat) + header.Set("Last-Modified", lastModified) + } if resp := notmodified.Response(req, header); resp != nil { if result.Body != nil { diff --git a/transport/azure/azure_test.go b/transport/azure/azure_test.go index 9ae8c563..cf9b7ac5 100644 --- a/transport/azure/azure_test.go +++ b/transport/azure/azure_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "os" "testing" + "time" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" @@ -16,9 +17,10 @@ import ( type AzureTestSuite struct { suite.Suite - server *httptest.Server - transport http.RoundTripper - etag string + server *httptest.Server + transport http.RoundTripper + etag string + lastModified time.Time } func (s *AzureTestSuite) SetupSuite() { @@ -27,11 +29,13 @@ func (s *AzureTestSuite) SetupSuite() { logrus.SetOutput(os.Stdout) s.etag = "testetag" + s.lastModified, _ = time.Parse(http.TimeFormat, "Wed, 21 Oct 2015 07:28:00 GMT") s.server = httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { require.Equal(s.T(), "/test/foo/test.png", r.URL.Path) rw.Header().Set("Etag", s.etag) + rw.Header().Set("Last-Modified", s.lastModified.Format(http.TimeFormat)) rw.WriteHeader(200) rw.Write(data) })) @@ -91,6 +95,46 @@ func (s *AzureTestSuite) TestRoundTripWithUpdatedETagReturns200() { require.Equal(s.T(), http.StatusOK, response.StatusCode) } +func (s *AzureTestSuite) TestRoundTripWithLastModifiedDisabledReturns200() { + config.LastModifiedEnabled = false + request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) +} + +func (s *AzureTestSuite) TestRoundTripWithLastModifiedEnabled() { + config.LastModifiedEnabled = true + request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) + require.Equal(s.T(), s.lastModified.Format(http.TimeFormat), response.Header.Get("Last-Modified")) +} + +func (s *AzureTestSuite) TestRoundTripWithIfModifiedSinceReturns304() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil) + request.Header.Set("If-Modified-Since", s.lastModified.Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusNotModified, response.StatusCode) +} + +func (s *AzureTestSuite) TestRoundTripWithUpdatedLastModifiedReturns200() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "abs://test/foo/test.png", nil) + request.Header.Set("If-Modified-Since", s.lastModified.Add(-24*time.Hour).Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusOK, response.StatusCode) +} func TestAzureTransport(t *testing.T) { suite.Run(t, new(AzureTestSuite)) } diff --git a/transport/fs/fs.go b/transport/fs/fs.go index 0bac8d33..1f733e4d 100644 --- a/transport/fs/fs.go +++ b/transport/fs/fs.go @@ -78,6 +78,11 @@ func (t transport) RoundTrip(req *http.Request) (resp *http.Response, err error) etag := BuildEtag(req.URL.Path, fi) header.Set("ETag", etag) } + + if config.LastModifiedEnabled { + lastModified := fi.ModTime().Format(http.TimeFormat) + header.Set("Last-Modified", lastModified) + } } if resp := notmodified.Response(req, header); resp != nil { diff --git a/transport/fs/fs_test.go b/transport/fs/fs_test.go index 5675a8f0..1d114184 100644 --- a/transport/fs/fs_test.go +++ b/transport/fs/fs_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -17,6 +18,7 @@ type FsTestSuite struct { transport http.RoundTripper etag string + modTime time.Time } func (s *FsTestSuite) SetupSuite() { @@ -29,6 +31,7 @@ func (s *FsTestSuite) SetupSuite() { require.Nil(s.T(), err) s.etag = BuildEtag("/test1.png", fi) + s.modTime = fi.ModTime() s.transport = New() } @@ -50,7 +53,6 @@ func (s *FsTestSuite) TestRoundTripWithETagEnabled() { require.Equal(s.T(), 200, response.StatusCode) require.Equal(s.T(), s.etag, response.Header.Get("ETag")) } - func (s *FsTestSuite) TestRoundTripWithIfNoneMatchReturns304() { config.ETagEnabled = true @@ -72,7 +74,46 @@ func (s *FsTestSuite) TestRoundTripWithUpdatedETagReturns200() { require.Nil(s.T(), err) require.Equal(s.T(), http.StatusOK, response.StatusCode) } +func (s *FsTestSuite) TestRoundTripWithLastModifiedDisabledReturns200() { + config.LastModifiedEnabled = false + request, _ := http.NewRequest("GET", "local:///test1.png", nil) + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) +} + +func (s *FsTestSuite) TestRoundTripWithLastModifiedEnabledReturns200() { + config.LastModifiedEnabled = true + request, _ := http.NewRequest("GET", "local:///test1.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) + require.Equal(s.T(), s.modTime.Format(http.TimeFormat), response.Header.Get("Last-Modified")) +} + +func (s *FsTestSuite) TestRoundTripWithIfModifiedSinceReturns304() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "local:///test1.png", nil) + request.Header.Set("If-Modified-Since", s.modTime.Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusNotModified, response.StatusCode) +} + +func (s *FsTestSuite) TestRoundTripWithUpdatedLastModifiedReturns200() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "local:///test1.png", nil) + request.Header.Set("If-Modified-Since", s.modTime.Add(-time.Minute).Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusOK, response.StatusCode) +} func TestS3Transport(t *testing.T) { suite.Run(t, new(FsTestSuite)) } diff --git a/transport/gcs/gcs.go b/transport/gcs/gcs.go index 9a7a51fa..0f9cb5c2 100644 --- a/transport/gcs/gcs.go +++ b/transport/gcs/gcs.go @@ -122,12 +122,17 @@ func (t transport) RoundTrip(req *http.Request) (*http.Response, error) { // We haven't initialize reader yet, this means that we need non-ranged reader if reader == nil { - if config.ETagEnabled { + if config.ETagEnabled || config.LastModifiedEnabled { attrs, err := obj.Attrs(req.Context()) if err != nil { return handleError(req, err) } - header.Set("ETag", attrs.Etag) + if config.ETagEnabled { + header.Set("ETag", attrs.Etag) + } + if config.LastModifiedEnabled { + header.Set("Last-Modified", attrs.Updated.Format(http.TimeFormat)) + } } if resp := notmodified.Response(req, header); resp != nil { diff --git a/transport/gcs/gcs_test.go b/transport/gcs/gcs_test.go index d0a30169..594fc4eb 100644 --- a/transport/gcs/gcs_test.go +++ b/transport/gcs/gcs_test.go @@ -5,6 +5,7 @@ import ( "net" "net/http" "testing" + "time" "github.com/fsouza/fake-gcs-server/fakestorage" "github.com/stretchr/testify/require" @@ -30,15 +31,17 @@ func getFreePort() (int, error) { type GCSTestSuite struct { suite.Suite - server *fakestorage.Server - transport http.RoundTripper - etag string + server *fakestorage.Server + transport http.RoundTripper + etag string + lastModified time.Time } func (s *GCSTestSuite) SetupSuite() { noAuth = true // s.etag = "testetag" + s.lastModified, _ = time.Parse(http.TimeFormat, "Wed, 21 Oct 2015 07:28:00 GMT") port, err := getFreePort() require.Nil(s.T(), err) @@ -53,6 +56,7 @@ func (s *GCSTestSuite) SetupSuite() { BucketName: "test", Name: "foo/test.png", // Etag: s.etag, + Updated: s.lastModified, }, Content: make([]byte, 32), }, @@ -116,6 +120,43 @@ func (s *GCSTestSuite) TestRoundTripWithUpdatedETagReturns200() { require.Equal(s.T(), http.StatusOK, response.StatusCode) } +func (s *GCSTestSuite) TestRoundTripWithLastModifiedDisabledReturns200() { + config.LastModifiedEnabled = false + request, _ := http.NewRequest("GET", "gcs://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) +} +func (s *GCSTestSuite) TestRoundTripWithLastModifiedEnabled() { + config.LastModifiedEnabled = true + request, _ := http.NewRequest("GET", "gcs://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) + require.Equal(s.T(), s.lastModified.Format(http.TimeFormat), response.Header.Get("Last-Modified")) +} +func (s *GCSTestSuite) TestRoundTripWithIfModifiedSinceReturns304() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "gcs://test/foo/test.png", nil) + request.Header.Set("If-Modified-Since", s.lastModified.Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusNotModified, response.StatusCode) +} +func (s *GCSTestSuite) TestRoundTripWithUpdatedLastModifiedReturns200() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "gcs://test/foo/test.png", nil) + request.Header.Set("If-Modified-Sicne", s.lastModified.Add(-24*time.Hour).Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusOK, response.StatusCode) +} func TestGCSTransport(t *testing.T) { suite.Run(t, new(GCSTestSuite)) } diff --git a/transport/notmodified/notmodified.go b/transport/notmodified/notmodified.go index 26e108cc..25469c72 100644 --- a/transport/notmodified/notmodified.go +++ b/transport/notmodified/notmodified.go @@ -2,6 +2,7 @@ package notmodified import ( "net/http" + "time" "github.com/imgproxy/imgproxy/v3/config" ) @@ -15,6 +16,27 @@ func Response(req *http.Request, header http.Header) *http.Response { return response(req, header) } } + if config.LastModifiedEnabled { + lastModifiedRaw := header.Get("Last-Modified") + if len(lastModifiedRaw) == 0 { + return nil + } + ifModifiedSinceRaw := req.Header.Get("If-Modified-Since") + if len(ifModifiedSinceRaw) == 0 { + return nil + } + lastModified, err := time.Parse(http.TimeFormat, lastModifiedRaw) + if err != nil { + return nil + } + ifModifiedSince, err := time.Parse(http.TimeFormat, ifModifiedSinceRaw) + if err != nil { + return nil + } + if !ifModifiedSince.Before(lastModified) { + return response(req, header) + } + } return nil } diff --git a/transport/s3/s3.go b/transport/s3/s3.go index ea26b263..05e551a9 100644 --- a/transport/s3/s3.go +++ b/transport/s3/s3.go @@ -5,6 +5,7 @@ import ( "io" http "net/http" "strings" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -64,9 +65,19 @@ func (t transport) RoundTrip(req *http.Request) (resp *http.Response, err error) if r := req.Header.Get("Range"); len(r) != 0 { input.Range = aws.String(r) - } else if config.ETagEnabled { - if ifNoneMatch := req.Header.Get("If-None-Match"); len(ifNoneMatch) > 0 { - input.IfNoneMatch = aws.String(ifNoneMatch) + } else { + if config.ETagEnabled { + if ifNoneMatch := req.Header.Get("If-None-Match"); len(ifNoneMatch) > 0 { + input.IfNoneMatch = aws.String(ifNoneMatch) + } + } + if config.LastModifiedEnabled { + if ifModifiedSince := req.Header.Get("If-Modified-Since"); len(ifModifiedSince) > 0 { + parsedIfModifiedSince, err := time.Parse(http.TimeFormat, ifModifiedSince) + if err == nil { + input.IfModifiedSince = &parsedIfModifiedSince + } + } } } diff --git a/transport/s3/s3_test.go b/transport/s3/s3_test.go index 37d5d8b2..80b1b8d2 100644 --- a/transport/s3/s3_test.go +++ b/transport/s3/s3_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "os" "testing" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" @@ -20,9 +21,10 @@ import ( type S3TestSuite struct { suite.Suite - server *httptest.Server - transport http.RoundTripper - etag string + server *httptest.Server + transport http.RoundTripper + etag string + lastModified time.Time } func (s *S3TestSuite) SetupSuite() { @@ -63,6 +65,7 @@ func (s *S3TestSuite) SetupSuite() { defer obj.Body.Close() s.etag = *obj.ETag + s.lastModified = *obj.LastModified } func (s *S3TestSuite) TearDownSuite() { @@ -110,6 +113,48 @@ func (s *S3TestSuite) TestRoundTripWithUpdatedETagReturns200() { require.Equal(s.T(), http.StatusOK, response.StatusCode) } +func (s *S3TestSuite) TestRoundTripWithLastModifiedDisabledReturns200() { + config.LastModifiedEnabled = false + request, _ := http.NewRequest("GET", "s3://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) +} + +func (s *S3TestSuite) TestRoundTripWithLastModifiedEnabled() { + config.ETagEnabled = true + request, _ := http.NewRequest("GET", "s3://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) + require.Equal(s.T(), s.lastModified.Format(http.TimeFormat), response.Header.Get("Last-Modified")) +} + +// gofakes3 doesn't support If-Modified-Since (yet?) +func (s *S3TestSuite) TestRoundTripWithIfModifiedSinceReturns304() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "s3://test/foo/test.png", nil) + request.Header.Set("If-Modified-Since", s.lastModified.Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusNotModified, response.StatusCode) +} + +func (s *S3TestSuite) TestRoundTripWithUpdatedLastModifiedReturns200() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "s3://test/foo/test.png", nil) + request.Header.Set("If-Modified-Since", s.lastModified.Add(-24*time.Hour).Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusOK, response.StatusCode) +} + func TestS3Transport(t *testing.T) { suite.Run(t, new(S3TestSuite)) } diff --git a/transport/swift/swift.go b/transport/swift/swift.go index 38a7b866..ba00e498 100644 --- a/transport/swift/swift.go +++ b/transport/swift/swift.go @@ -84,13 +84,19 @@ func (t transport) RoundTrip(req *http.Request) (resp *http.Response, err error) if etag, ok := objectHeaders["Etag"]; ok { header.Set("ETag", etag) } + } - if resp := notmodified.Response(req, header); resp != nil { - object.Close() - return resp, nil + if config.LastModifiedEnabled { + if lastModified, ok := objectHeaders["Last-Modified"]; ok { + header.Set("Last-Modified", lastModified) } } + if resp := notmodified.Response(req, header); resp != nil { + object.Close() + return resp, nil + } + for k, v := range objectHeaders { header.Set(k, v) } diff --git a/transport/swift/swift_test.go b/transport/swift/swift_test.go index 182774f0..b3e57aaa 100644 --- a/transport/swift/swift_test.go +++ b/transport/swift/swift_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "testing" + "time" "github.com/ncw/swift/v2" "github.com/ncw/swift/v2/swifttest" @@ -20,9 +21,10 @@ const ( type SwiftTestSuite struct { suite.Suite - server *swifttest.SwiftServer - transport http.RoundTripper - etag string + server *swifttest.SwiftServer + transport http.RoundTripper + etag string + lastModified time.Time } func (s *SwiftTestSuite) SetupSuite() { @@ -71,10 +73,12 @@ func (s *SwiftTestSuite) setupTestFile() { require.Nil(t, err) f.Close() - - h, err := f.Headers() + // The Etag is written on file close; but Last-Modified is only available when we get the object again. + _, h, err := c.Object(ctx, testContainer, testObject) require.Nil(t, err) s.etag = h["Etag"] + s.lastModified, err = time.Parse(http.TimeFormat, h["Date"]) + require.Nil(t, err) } func (s *SwiftTestSuite) TearDownSuite() { @@ -140,6 +144,47 @@ func (s *SwiftTestSuite) TestRoundTripWithUpdatedETagReturns200() { require.Equal(s.T(), http.StatusOK, response.StatusCode) } +func (s *SwiftTestSuite) TestRoundTripWithLastModifiedDisabledReturns200() { + config.LastModifiedEnabled = false + request, _ := http.NewRequest("GET", "swift://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) +} + +func (s *SwiftTestSuite) TestRoundTripWithLastModifiedEnabled() { + config.LastModifiedEnabled = true + request, _ := http.NewRequest("GET", "swift://test/foo/test.png", nil) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), 200, response.StatusCode) + require.Equal(s.T(), s.lastModified.Format(http.TimeFormat), response.Header.Get("Last-Modified")) +} + +func (s *SwiftTestSuite) TestRoundTripWithIfModifiedSinceReturns304() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "swift://test/foo/test.png", nil) + request.Header.Set("If-Modified-Since", s.lastModified.Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusNotModified, response.StatusCode) +} + +func (s *SwiftTestSuite) TestRoundTripWithUpdatedLastModifiedReturns200() { + config.LastModifiedEnabled = true + + request, _ := http.NewRequest("GET", "swift://test/foo/test.png", nil) + request.Header.Set("If-Modified-Since", s.lastModified.Add(-24*time.Hour).Format(http.TimeFormat)) + + response, err := s.transport.RoundTrip(request) + require.Nil(s.T(), err) + require.Equal(s.T(), http.StatusOK, response.StatusCode) +} + func TestSwiftTransport(t *testing.T) { suite.Run(t, new(SwiftTestSuite)) }