diff --git a/auximageprovider/static_provider.go b/auximageprovider/static_provider.go index 353abf18..c169ea29 100644 --- a/auximageprovider/static_provider.go +++ b/auximageprovider/static_provider.go @@ -21,7 +21,7 @@ func (s *staticProvider) Get(_ context.Context, po *options.ProcessingOptions) ( } // NewStaticFromTriple creates a new ImageProvider from either a base64 string, file path, or URL -func NewStaticProvider(ctx context.Context, c *StaticConfig, desc string) (Provider, error) { +func NewStaticProvider(ctx context.Context, c *StaticConfig, desc string, idf *imagedata.Factory) (Provider, error) { var ( data imagedata.ImageData headers = make(http.Header) @@ -30,11 +30,11 @@ func NewStaticProvider(ctx context.Context, c *StaticConfig, desc string) (Provi switch { case len(c.Base64Data) > 0: - data, err = imagedata.NewFromBase64(c.Base64Data) + data, err = idf.NewFromBase64(c.Base64Data) case len(c.Path) > 0: - data, err = imagedata.NewFromPath(c.Path) + data, err = idf.NewFromPath(c.Path) case len(c.URL) > 0: - data, headers, err = imagedata.DownloadSync( + data, headers, err = idf.DownloadSync( ctx, c.URL, desc, imagedata.DownloadOptions{}, ) default: diff --git a/auximageprovider/static_provider_test.go b/auximageprovider/static_provider_test.go index fef9d4b1..44c90bfb 100644 --- a/auximageprovider/static_provider_test.go +++ b/auximageprovider/static_provider_test.go @@ -12,9 +12,11 @@ import ( "github.com/stretchr/testify/suite" "github.com/imgproxy/imgproxy/v3/config" + "github.com/imgproxy/imgproxy/v3/fetcher" "github.com/imgproxy/imgproxy/v3/httpheaders" "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/options" + "github.com/imgproxy/imgproxy/v3/transport" ) type ImageProviderTestSuite struct { @@ -62,8 +64,6 @@ func (s *ImageProviderTestSuite) SetupSuite() { rw.WriteHeader(s.status) rw.Write(data) })) - - s.Require().NoError(imagedata.Init()) } func (s *ImageProviderTestSuite) TearDownSuite() { @@ -168,13 +168,23 @@ func (s *ImageProviderTestSuite) TestNewProvider() { }, } + trc := transport.NewDefaultConfig() + tr, err := transport.New(trc) + s.Require().NoError(err) + + fc := fetcher.NewDefaultConfig() + f, err := fetcher.New(tr, fc) + s.Require().NoError(err) + + idf := imagedata.NewFactory(f) + for _, tt := range tests { s.T().Run(tt.name, func(t *testing.T) { if tt.setupFunc != nil { tt.setupFunc() } - provider, err := NewStaticProvider(s.T().Context(), tt.config, "test image") + provider, err := NewStaticProvider(s.T().Context(), tt.config, "test image", idf) if tt.expectError { s.Require().Error(err) diff --git a/fetcher/fetcher.go b/fetcher/fetcher.go index 5ef81c65..5796be84 100644 --- a/fetcher/fetcher.go +++ b/fetcher/fetcher.go @@ -23,8 +23,8 @@ type Fetcher struct { config *Config // Configuration for the image fetcher } -// NewFetcher creates a new ImageFetcher with the provided transport -func NewFetcher(transport *transport.Transport, config *Config) (*Fetcher, error) { +// New creates a new ImageFetcher with the provided transport +func New(transport *transport.Transport, config *Config) (*Fetcher, error) { if err := config.Validate(); err != nil { return nil, err } diff --git a/fetcher/request.go b/fetcher/request.go index 93ed5bfa..52a46711 100644 --- a/fetcher/request.go +++ b/fetcher/request.go @@ -67,9 +67,11 @@ func (r *Request) Send() (*http.Response, error) { } } -// FetchImage fetches the image using the request and returns the response or an error. -// It checks for the NotModified status and handles partial content responses. -func (r *Request) FetchImage() (*http.Response, error) { +// Fetch fetches the image using the request and returns the response or an error. +// Unlike Send, it checks the request status and converts it into typed errors. +// Specifically, it checks for Not Modified, ensures that Partial Content response +// contains the entire image, and wraps gzip-encoded responses. +func (r *Request) Fetch() (*http.Response, error) { res, err := r.Send() if err != nil { return nil, err diff --git a/handlers/processing/config.go b/handlers/processing/config.go index 493368fe..2368d297 100644 --- a/handlers/processing/config.go +++ b/handlers/processing/config.go @@ -17,9 +17,6 @@ type Config struct { ReportIOErrors bool // Whether to report IO errors FallbackImageHTTPCode int // Fallback image HTTP status code EnableDebugHeaders bool // Whether to enable debug headers - FallbackImageData string // Fallback image data (base64) - FallbackImagePath string // Fallback image path (local file system) - FallbackImageURL string // Fallback image URL (remote) } // NewDefaultConfig creates a new configuration with defaults diff --git a/handlers/processing/handler.go b/handlers/processing/handler.go index 95d37a0d..28d3bf74 100644 --- a/handlers/processing/handler.go +++ b/handlers/processing/handler.go @@ -10,6 +10,7 @@ import ( "github.com/imgproxy/imgproxy/v3/handlers/stream" "github.com/imgproxy/imgproxy/v3/headerwriter" "github.com/imgproxy/imgproxy/v3/ierrors" + "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/monitoring" "github.com/imgproxy/imgproxy/v3/monitoring/stats" "github.com/imgproxy/imgproxy/v3/options" @@ -19,11 +20,13 @@ import ( // Handler handles image processing requests type Handler struct { - hw *headerwriter.Writer // Configured HeaderWriter instance - stream *stream.Handler // Stream handler for raw image streaming - config *Config // Handler configuration - semaphores *semaphores.Semaphores - fallbackImage auximageprovider.Provider + hw *headerwriter.Writer // Configured HeaderWriter instance + stream *stream.Handler // Stream handler for raw image streaming + config *Config // Handler configuration + semaphores *semaphores.Semaphores + fallbackImage auximageprovider.Provider + watermarkImage auximageprovider.Provider + imageData *imagedata.Factory } // New creates new handler object @@ -32,6 +35,8 @@ func New( hw *headerwriter.Writer, semaphores *semaphores.Semaphores, fi auximageprovider.Provider, + wi auximageprovider.Provider, + idf *imagedata.Factory, config *Config, ) (*Handler, error) { if err := config.Validate(); err != nil { @@ -39,11 +44,13 @@ func New( } return &Handler{ - hw: hw, - config: config, - stream: stream, - semaphores: semaphores, - fallbackImage: fi, + hw: hw, + config: config, + stream: stream, + semaphores: semaphores, + fallbackImage: fi, + watermarkImage: wi, + imageData: idf, }, nil } @@ -81,6 +88,7 @@ func (h *Handler) Execute( monitoringMeta: mm, semaphores: h.semaphores, hwr: h.hw.NewRequest(), + idf: h.imageData, } return req.execute(ctx) diff --git a/handlers/processing/request.go b/handlers/processing/request.go index db178ef5..ae35ae8a 100644 --- a/handlers/processing/request.go +++ b/handlers/processing/request.go @@ -8,6 +8,7 @@ import ( "github.com/imgproxy/imgproxy/v3/fetcher" "github.com/imgproxy/imgproxy/v3/headerwriter" "github.com/imgproxy/imgproxy/v3/ierrors" + "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/imagetype" "github.com/imgproxy/imgproxy/v3/monitoring" "github.com/imgproxy/imgproxy/v3/monitoring/stats" @@ -29,6 +30,7 @@ type request struct { monitoringMeta monitoring.Meta semaphores *semaphores.Semaphores hwr *headerwriter.Request + idf *imagedata.Factory } // execute handles the actual processing logic diff --git a/handlers/processing/request_methods.go b/handlers/processing/request_methods.go index 219ad41c..599a5778 100644 --- a/handlers/processing/request_methods.go +++ b/handlers/processing/request_methods.go @@ -81,7 +81,7 @@ func (r *request) fetchImage(ctx context.Context, do imagedata.DownloadOptions) } } - return imagedata.DownloadAsync(ctx, r.imageURL, "source image", do) + return r.idf.DownloadAsync(ctx, r.imageURL, "source image", do) } // handleDownloadError replaces the image data with fallback image if needed @@ -154,7 +154,7 @@ func (r *request) getFallbackImage( // processImage calls actual image processing func (r *request) processImage(ctx context.Context, originData imagedata.ImageData) (*processing.Result, error) { defer monitoring.StartProcessingSegment(ctx, r.monitoringMeta.Filter(monitoring.MetaProcessingOptions))() - return processing.ProcessImage(ctx, originData, r.po) + return processing.ProcessImage(ctx, originData, r.po, r.handler.watermarkImage, r.handler.imageData) } // writeDebugHeaders writes debug headers (X-Origin-*, X-Result-*) to the response diff --git a/handlers/stream/handler_test.go b/handlers/stream/handler_test.go index 843ce352..aff41edb 100644 --- a/handlers/stream/handler_test.go +++ b/handlers/stream/handler_test.go @@ -57,7 +57,7 @@ func (s *HandlerTestSuite) SetupTest() { fc := fetcher.NewDefaultConfig() - fetcher, err := fetcher.NewFetcher(tr, fc) + fetcher, err := fetcher.New(tr, fc) s.Require().NoError(err) cfg := NewDefaultConfig() @@ -358,7 +358,7 @@ func (s *HandlerTestSuite) TestHandlerCacheControl() { fc := fetcher.NewDefaultConfig() - fetcher, err := fetcher.NewFetcher(tr, fc) + fetcher, err := fetcher.New(tr, fc) s.Require().NoError(err) cfg := NewDefaultConfig() @@ -454,7 +454,7 @@ func (s *HandlerTestSuite) TestHandlerCookiePassthrough() { s.Require().NoError(err) fc := fetcher.NewDefaultConfig() - fetcher, err := fetcher.NewFetcher(tr, fc) + fetcher, err := fetcher.New(tr, fc) s.Require().NoError(err) cfg := NewDefaultConfig() @@ -514,7 +514,7 @@ func (s *HandlerTestSuite) TestHandlerCanonicalHeader() { s.Require().NoError(err) fc := fetcher.NewDefaultConfig() - fetcher, err := fetcher.NewFetcher(tr, fc) + fetcher, err := fetcher.New(tr, fc) s.Require().NoError(err) cfg := NewDefaultConfig() diff --git a/imagedata/download.go b/imagedata/download.go index 8132f980..482008b0 100644 --- a/imagedata/download.go +++ b/imagedata/download.go @@ -3,16 +3,9 @@ package imagedata import ( "context" "net/http" - - "github.com/imgproxy/imgproxy/v3/config" - "github.com/imgproxy/imgproxy/v3/fetcher" - "github.com/imgproxy/imgproxy/v3/ierrors" - "github.com/imgproxy/imgproxy/v3/transport" ) var ( - Fetcher *fetcher.Fetcher - // For tests. This needs to move to fetcher once we will have a way to isolate // the fetcher in tests. redirectAllRequestsTo string @@ -25,39 +18,7 @@ type DownloadOptions struct { DownloadFinished context.CancelFunc } -func DefaultDownloadOptions() DownloadOptions { - return DownloadOptions{ - Header: nil, - CookieJar: nil, - MaxSrcFileSize: config.MaxSrcFileSize, - DownloadFinished: nil, - } -} - -func initDownloading() error { - trc, err := transport.LoadFromEnv(transport.NewDefaultConfig()) - if err != nil { - return err - } - - ts, err := transport.New(trc) - if err != nil { - return err - } - - c, err := fetcher.LoadFromEnv(fetcher.NewDefaultConfig()) - if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithPrefix("configuration error")) - } - - Fetcher, err = fetcher.NewFetcher(ts, c) - if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithPrefix("can't create image fetcher")) - } - - return nil -} - +// TODO: get rid of this global variable func RedirectAllRequestsTo(u string) { redirectAllRequestsTo = u } diff --git a/imagedata/factory.go b/imagedata/factory.go index 72cd0989..525af107 100644 --- a/imagedata/factory.go +++ b/imagedata/factory.go @@ -14,6 +14,18 @@ import ( "github.com/imgproxy/imgproxy/v3/security" ) +// Factory represents ImageData factory +type Factory struct { + fetcher *fetcher.Fetcher +} + +// NewFactory creates a new factory +func NewFactory(fetcher *fetcher.Fetcher) *Factory { + return &Factory{ + fetcher: fetcher, + } +} + // NewFromBytesWithFormat creates a new ImageData instance from the provided format // and byte slice. func NewFromBytesWithFormat(format imagetype.Type, b []byte) ImageData { @@ -25,7 +37,7 @@ func NewFromBytesWithFormat(format imagetype.Type, b []byte) ImageData { } // NewFromBytes creates a new ImageData instance from the provided byte slice. -func NewFromBytes(b []byte) (ImageData, error) { +func (f *Factory) NewFromBytes(b []byte) (ImageData, error) { r := bytes.NewReader(b) format, err := imagetype.Detect(r) @@ -37,7 +49,7 @@ func NewFromBytes(b []byte) (ImageData, error) { } // NewFromPath creates a new ImageData from an os.File -func NewFromPath(path string) (ImageData, error) { +func (f *Factory) NewFromPath(path string) (ImageData, error) { fl, err := os.Open(path) if err != nil { return nil, err @@ -49,21 +61,21 @@ func NewFromPath(path string) (ImageData, error) { return nil, err } - return NewFromBytes(b) + return f.NewFromBytes(b) } // NewFromBase64 creates a new ImageData from a base64 encoded byte slice -func NewFromBase64(encoded string) (ImageData, error) { +func (f *Factory) NewFromBase64(encoded string) (ImageData, error) { b, err := base64.StdEncoding.DecodeString(encoded) if err != nil { return nil, err } - return NewFromBytes(b) + return f.NewFromBytes(b) } // sendRequest is a common logic between sync and async download. -func sendRequest(ctx context.Context, url string, opts DownloadOptions) (*fetcher.Request, *http.Response, http.Header, error) { +func (f *Factory) sendRequest(ctx context.Context, url string, opts DownloadOptions) (*fetcher.Request, *http.Response, http.Header, error) { h := make(http.Header) // NOTE: This will be removed in the future when our test context gets better isolation @@ -71,12 +83,12 @@ func sendRequest(ctx context.Context, url string, opts DownloadOptions) (*fetche url = redirectAllRequestsTo } - req, err := Fetcher.BuildRequest(ctx, url, opts.Header, opts.CookieJar) + req, err := f.fetcher.BuildRequest(ctx, url, opts.Header, opts.CookieJar) if err != nil { return req, nil, h, err } - res, err := req.FetchImage() + res, err := req.Fetch() if res != nil { h = res.Header.Clone() } @@ -103,12 +115,12 @@ func sendRequest(ctx context.Context, url string, opts DownloadOptions) (*fetche } // DownloadSync downloads the image synchronously and returns the ImageData and HTTP headers. -func DownloadSync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { +func (f *Factory) DownloadSync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { if opts.DownloadFinished != nil { defer opts.DownloadFinished() } - req, res, h, err := sendRequest(ctx, imageURL, opts) + req, res, h, err := f.sendRequest(ctx, imageURL, opts) if res != nil { defer res.Body.Close() } @@ -137,10 +149,10 @@ func DownloadSync(ctx context.Context, imageURL, desc string, opts DownloadOptio // DownloadAsync downloads the image asynchronously and returns the ImageData // backed by AsyncBuffer and HTTP headers. -func DownloadAsync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { +func (f *Factory) DownloadAsync(ctx context.Context, imageURL, desc string, opts DownloadOptions) (ImageData, http.Header, error) { // We pass this responsibility to AsyncBuffer //nolint:bodyclose - req, res, h, err := sendRequest(ctx, imageURL, opts) + req, res, h, err := f.sendRequest(ctx, imageURL, opts) if err != nil { if opts.DownloadFinished != nil { defer opts.DownloadFinished() diff --git a/imagedata/image_data.go b/imagedata/image_data.go index 4a663c2c..4d56d9ad 100644 --- a/imagedata/image_data.go +++ b/imagedata/image_data.go @@ -7,8 +7,6 @@ import ( "sync" "github.com/imgproxy/imgproxy/v3/asyncbuffer" - "github.com/imgproxy/imgproxy/v3/config" - "github.com/imgproxy/imgproxy/v3/ierrors" "github.com/imgproxy/imgproxy/v3/imagetype" ) @@ -126,48 +124,3 @@ func (d *imageDataAsyncBuffer) Error() error { } return nil } - -func Init() error { - if err := initDownloading(); err != nil { - return err - } - - if err := loadWatermark(); err != nil { - return err - } - - return nil -} - -func loadWatermark() error { - var err error - - switch { - case len(config.WatermarkData) > 0: - Watermark, err = NewFromBase64(config.WatermarkData) - - // NOTE: this should be something like err = ierrors.Wrap(err).WithStackDeep(0).WithPrefix("watermark") - // In the NewFromBase64 all errors should be wrapped to something like - // .WithPrefix("load from base64") - if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithPrefix("can't load watermark from Base64")) - } - - case len(config.WatermarkPath) > 0: - Watermark, err = NewFromPath(config.WatermarkPath) - if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithPrefix("can't read watermark from file")) - } - - case len(config.WatermarkURL) > 0: - Watermark, _, err = DownloadSync(context.Background(), config.WatermarkURL, "watermark", DefaultDownloadOptions()) - if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithPrefix("can't download from URL")) - } - - default: - Watermark = nil - } - - return nil -} diff --git a/imagedata/image_data_test.go b/imagedata/image_data_test.go index 553e5d2d..3c49ae60 100644 --- a/imagedata/image_data_test.go +++ b/imagedata/image_data_test.go @@ -17,10 +17,12 @@ import ( "github.com/stretchr/testify/suite" "github.com/imgproxy/imgproxy/v3/config" + "github.com/imgproxy/imgproxy/v3/fetcher" "github.com/imgproxy/imgproxy/v3/httpheaders" "github.com/imgproxy/imgproxy/v3/ierrors" "github.com/imgproxy/imgproxy/v3/imagetype" "github.com/imgproxy/imgproxy/v3/testutil" + "github.com/imgproxy/imgproxy/v3/transport" ) type ImageDataTestSuite struct { @@ -28,10 +30,11 @@ type ImageDataTestSuite struct { server *httptest.Server - status int - data []byte - header http.Header - check func(*http.Request) + status int + data []byte + header http.Header + check func(*http.Request) + factory *Factory defaultData []byte } @@ -40,8 +43,6 @@ func (s *ImageDataTestSuite) SetupSuite() { config.Reset() config.ClientKeepAliveTimeout = 0 - Init() - f, err := os.Open("../testdata/test1.jpg") s.Require().NoError(err) defer f.Close() @@ -68,6 +69,20 @@ func (s *ImageDataTestSuite) SetupSuite() { rw.WriteHeader(s.status) rw.Write(data) })) + + ctr, err := transport.LoadFromEnv(transport.NewDefaultConfig()) + s.Require().NoError(err) + + ts, err := transport.New(ctr) + s.Require().NoError(err) + + c, err := fetcher.LoadFromEnv(fetcher.NewDefaultConfig()) + s.Require().NoError(err) + + fetcher, err := fetcher.New(ts, c) + s.Require().NoError(err) + + s.factory = NewFactory(fetcher) } func (s *ImageDataTestSuite) TearDownSuite() { @@ -84,10 +99,11 @@ func (s *ImageDataTestSuite) SetupTest() { s.header = http.Header{} s.header.Set("Content-Type", "image/jpeg") + } func (s *ImageDataTestSuite) TestDownloadStatusOK() { - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) s.Require().NoError(err) s.Require().NotNil(imgdata) @@ -154,7 +170,7 @@ func (s *ImageDataTestSuite) TestDownloadStatusPartialContent() { s.Run(tc.name, func() { s.header.Set("Content-Range", tc.contentRange) - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) if tc.expectErr { s.Require().Error(err) @@ -174,7 +190,7 @@ func (s *ImageDataTestSuite) TestDownloadStatusNotFound() { s.data = []byte("Not Found") s.header.Set("Content-Type", "text/plain") - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) s.Require().Error(err) s.Require().Equal(404, ierrors.Wrap(err, 0).StatusCode()) @@ -186,7 +202,7 @@ func (s *ImageDataTestSuite) TestDownloadStatusForbidden() { s.data = []byte("Forbidden") s.header.Set("Content-Type", "text/plain") - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) s.Require().Error(err) s.Require().Equal(404, ierrors.Wrap(err, 0).StatusCode()) @@ -198,7 +214,7 @@ func (s *ImageDataTestSuite) TestDownloadStatusInternalServerError() { s.data = []byte("Internal Server Error") s.header.Set("Content-Type", "text/plain") - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) s.Require().Error(err) s.Require().Equal(500, ierrors.Wrap(err, 0).StatusCode()) @@ -212,7 +228,7 @@ func (s *ImageDataTestSuite) TestDownloadUnreachable() { serverURL := fmt.Sprintf("http://%s", l.Addr().String()) - imgdata, _, err := DownloadSync(context.Background(), serverURL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), serverURL, "Test image", DownloadOptions{}) s.Require().Error(err) s.Require().Equal(500, ierrors.Wrap(err, 0).StatusCode()) @@ -222,7 +238,7 @@ func (s *ImageDataTestSuite) TestDownloadUnreachable() { func (s *ImageDataTestSuite) TestDownloadInvalidImage() { s.data = []byte("invalid") - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) s.Require().Error(err) s.Require().Equal(422, ierrors.Wrap(err, 0).StatusCode()) @@ -232,7 +248,7 @@ func (s *ImageDataTestSuite) TestDownloadInvalidImage() { func (s *ImageDataTestSuite) TestDownloadSourceAddressNotAllowed() { config.AllowLoopbackSourceAddresses = false - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) s.Require().Error(err) s.Require().Equal(404, ierrors.Wrap(err, 0).StatusCode()) @@ -240,10 +256,11 @@ func (s *ImageDataTestSuite) TestDownloadSourceAddressNotAllowed() { } func (s *ImageDataTestSuite) TestDownloadImageFileTooLarge() { - config.MaxSrcFileSize = 1 - - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{ + MaxSrcFileSize: 1, + }) + fmt.Println(err) s.Require().Error(err) s.Require().Equal(422, ierrors.Wrap(err, 0).StatusCode()) s.Require().Nil(imgdata) @@ -261,7 +278,7 @@ func (s *ImageDataTestSuite) TestDownloadGzip() { s.data = buf.Bytes() s.header.Set("Content-Encoding", "gzip") - imgdata, _, err := DownloadSync(context.Background(), s.server.URL, "Test image", DefaultDownloadOptions()) + imgdata, _, err := s.factory.DownloadSync(context.Background(), s.server.URL, "Test image", DownloadOptions{}) s.Require().NoError(err) s.Require().NotNil(imgdata) @@ -270,7 +287,7 @@ func (s *ImageDataTestSuite) TestDownloadGzip() { } func (s *ImageDataTestSuite) TestFromFile() { - imgdata, err := NewFromPath("../testdata/test1.jpg") + imgdata, err := s.factory.NewFromPath("../testdata/test1.jpg") s.Require().NoError(err) s.Require().NotNil(imgdata) @@ -281,7 +298,7 @@ func (s *ImageDataTestSuite) TestFromFile() { func (s *ImageDataTestSuite) TestFromBase64() { b64 := base64.StdEncoding.EncodeToString(s.defaultData) - imgdata, err := NewFromBase64(b64) + imgdata, err := s.factory.NewFromBase64(b64) s.Require().NoError(err) s.Require().NotNil(imgdata) diff --git a/main.go b/main.go index e1e476f1..440458fd 100644 --- a/main.go +++ b/main.go @@ -17,6 +17,7 @@ import ( "github.com/imgproxy/imgproxy/v3/config" "github.com/imgproxy/imgproxy/v3/config/loadenv" "github.com/imgproxy/imgproxy/v3/errorreport" + "github.com/imgproxy/imgproxy/v3/fetcher" "github.com/imgproxy/imgproxy/v3/gliblog" "github.com/imgproxy/imgproxy/v3/handlers" processingHandler "github.com/imgproxy/imgproxy/v3/handlers/processing" @@ -32,6 +33,7 @@ import ( "github.com/imgproxy/imgproxy/v3/processing" "github.com/imgproxy/imgproxy/v3/semaphores" "github.com/imgproxy/imgproxy/v3/server" + "github.com/imgproxy/imgproxy/v3/transport" "github.com/imgproxy/imgproxy/v3/version" "github.com/imgproxy/imgproxy/v3/vips" ) @@ -59,7 +61,29 @@ func callHandleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) } - stream, err := stream.New(sc, hw, imagedata.Fetcher) + tcfg, err := transport.LoadFromEnv(transport.NewDefaultConfig()) + if err != nil { + return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) + } + + tr, err := transport.New(tcfg) + if err != nil { + return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) + } + + fc, err := fetcher.LoadFromEnv(fetcher.NewDefaultConfig()) + if err != nil { + return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) + } + + fetcher, err := fetcher.New(tr, fc) + if err != nil { + return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) + } + + idf := imagedata.NewFactory(fetcher) + + stream, err := stream.New(sc, hw, fetcher) if err != nil { return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) } @@ -89,12 +113,29 @@ func callHandleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) r.Context(), fic, "fallback image", + idf, ) if err != nil { return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) } - h, err := processingHandler.New(stream, hw, semaphores, fi, phc) + wic := auximageprovider.NewDefaultStaticConfig() + wic, err = auximageprovider.LoadFallbackStaticConfigFromEnv(wic) + if err != nil { + return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) + } + + wi, err := auximageprovider.NewStaticProvider( + r.Context(), + wic, + "watermark image", + idf, + ) + if err != nil { + return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) + } + + h, err := processingHandler.New(stream, hw, semaphores, fi, wi, idf, phc) if err != nil { return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig)) } @@ -144,10 +185,6 @@ func initialize() error { return err } - if err := imagedata.Init(); err != nil { - return err - } - errorreport.Init() if err := vips.Init(); err != nil { diff --git a/processing/pipeline.go b/processing/pipeline.go index ab9a0744..6536aa91 100644 --- a/processing/pipeline.go +++ b/processing/pipeline.go @@ -3,6 +3,7 @@ package processing import ( "context" + "github.com/imgproxy/imgproxy/v3/auximageprovider" "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/imagetype" "github.com/imgproxy/imgproxy/v3/options" @@ -15,6 +16,9 @@ type pipelineContext struct { imgtype imagetype.Type + // The watermark image provider, if any watermarking is to be done. + watermarkProvider auximageprovider.Provider + trimmed bool srcWidth int @@ -67,7 +71,13 @@ type pipelineContext struct { type pipelineStep func(pctx *pipelineContext, img *vips.Image, po *options.ProcessingOptions, imgdata imagedata.ImageData) error type pipeline []pipelineStep -func (p pipeline) Run(ctx context.Context, img *vips.Image, po *options.ProcessingOptions, imgdata imagedata.ImageData) error { +func (p pipeline) Run( + ctx context.Context, + img *vips.Image, + po *options.ProcessingOptions, + imgdata imagedata.ImageData, + watermark auximageprovider.Provider, +) error { pctx := pipelineContext{ ctx: ctx, @@ -77,7 +87,8 @@ func (p pipeline) Run(ctx context.Context, img *vips.Image, po *options.Processi dprScale: 1.0, vectorBaseScale: 1.0, - cropGravity: po.Crop.Gravity, + cropGravity: po.Crop.Gravity, + watermarkProvider: watermark, } if pctx.cropGravity.Type == options.GravityUnknown { diff --git a/processing/processing.go b/processing/processing.go index 81275bae..9bf12e58 100644 --- a/processing/processing.go +++ b/processing/processing.go @@ -8,6 +8,7 @@ import ( log "github.com/sirupsen/logrus" + "github.com/imgproxy/imgproxy/v3/auximageprovider" "github.com/imgproxy/imgproxy/v3/config" "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/imagetype" @@ -83,6 +84,8 @@ func ProcessImage( ctx context.Context, imgdata imagedata.ImageData, po *options.ProcessingOptions, + watermarkProvider auximageprovider.Provider, + idf *imagedata.Factory, ) (*Result, error) { runtime.LockOSThread() defer runtime.UnlockOSThread() @@ -136,12 +139,12 @@ func ProcessImage( } // Transform the image (resize, crop, etc) - if err = transformImage(ctx, img, po, imgdata, animated); err != nil { + if err = transformImage(ctx, img, po, imgdata, animated, watermarkProvider); err != nil { return nil, err } // Finalize the image (colorspace conversion, metadata stripping, etc) - if err = finalizePipeline.Run(ctx, img, po, imgdata); err != nil { + if err = finalizePipeline.Run(ctx, img, po, imgdata, watermarkProvider); err != nil { return nil, err } @@ -385,18 +388,20 @@ func transformImage( po *options.ProcessingOptions, imgdata imagedata.ImageData, asAnimated bool, + watermark auximageprovider.Provider, ) error { if asAnimated { - return transformAnimated(ctx, img, po) + return transformAnimated(ctx, img, po, watermark) } - return mainPipeline.Run(ctx, img, po, imgdata) + return mainPipeline.Run(ctx, img, po, imgdata, watermark) } func transformAnimated( ctx context.Context, img *vips.Image, po *options.ProcessingOptions, + watermark auximageprovider.Provider, ) error { if po.Trim.Enabled { log.Warning("Trim is not supported for animated images") @@ -450,7 +455,8 @@ func transformAnimated( // Transform the frame using the main pipeline. // We don't provide imgdata here to prevent scale-on-load. - if err = mainPipeline.Run(ctx, frame, po, nil); err != nil { + // Let's skip passing watermark here since in would be applied later to all frames at once. + if err = mainPipeline.Run(ctx, frame, po, nil, nil); err != nil { return err } @@ -474,7 +480,7 @@ func transformAnimated( // Apply watermark to all frames at once if it was requested. // This is much more efficient than applying watermark to individual frames. - if watermarkEnabled && imagedata.Watermark != nil { + if watermarkEnabled && watermark != nil { // Get DPR scale to apply watermark correctly on HiDPI images. // `imgproxy-dpr-scale` is set by the pipeline. dprScale, derr := img.GetDoubleDefault("imgproxy-dpr-scale", 1.0) @@ -483,7 +489,7 @@ func transformAnimated( } if err = applyWatermark( - img, imagedata.Watermark, &po.Watermark, dprScale, framesCount, + img, watermark, &po.Watermark, dprScale, framesCount, ); err != nil { return err } diff --git a/processing/processing_test.go b/processing/processing_test.go index 07903548..a23bea61 100644 --- a/processing/processing_test.go +++ b/processing/processing_test.go @@ -12,14 +12,17 @@ import ( "github.com/stretchr/testify/suite" "github.com/imgproxy/imgproxy/v3/config" + "github.com/imgproxy/imgproxy/v3/fetcher" "github.com/imgproxy/imgproxy/v3/ierrors" "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/options" + "github.com/imgproxy/imgproxy/v3/transport" "github.com/imgproxy/imgproxy/v3/vips" ) type ProcessingTestSuite struct { suite.Suite + idf *imagedata.Factory } func (s *ProcessingTestSuite) SetupSuite() { @@ -30,10 +33,19 @@ func (s *ProcessingTestSuite) SetupSuite() { config.MaxAnimationFrames = 100 config.MaxAnimationFrameResolution = 10 * 1024 * 1024 - s.Require().NoError(imagedata.Init()) s.Require().NoError(vips.Init()) logrus.SetOutput(io.Discard) + + trc := transport.NewDefaultConfig() + tr, err := transport.New(trc) + s.Require().NoError(err) + + fc := fetcher.NewDefaultConfig() + f, err := fetcher.New(tr, fc) + s.Require().NoError(err) + + s.idf = imagedata.NewFactory(f) } func (s *ProcessingTestSuite) openFile(name string) imagedata.ImageData { @@ -41,7 +53,7 @@ func (s *ProcessingTestSuite) openFile(name string) imagedata.ImageData { s.Require().NoError(err) path := filepath.Join(wd, "..", "testdata", name) - imagedata, err := imagedata.NewFromPath(path) + imagedata, err := s.idf.NewFromPath(path) s.Require().NoError(err) return imagedata @@ -82,7 +94,7 @@ func (s *ProcessingTestSuite) TestResizeToFit() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -121,7 +133,7 @@ func (s *ProcessingTestSuite) TestResizeToFitEnlarge() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -165,7 +177,7 @@ func (s *ProcessingTestSuite) TestResizeToFitExtend() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -209,7 +221,7 @@ func (s *ProcessingTestSuite) TestResizeToFitExtendAR() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -247,7 +259,7 @@ func (s *ProcessingTestSuite) TestResizeToFill() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -286,7 +298,7 @@ func (s *ProcessingTestSuite) TestResizeToFillEnlarge() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -332,7 +344,7 @@ func (s *ProcessingTestSuite) TestResizeToFillExtend() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -378,7 +390,7 @@ func (s *ProcessingTestSuite) TestResizeToFillExtendAR() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -416,7 +428,7 @@ func (s *ProcessingTestSuite) TestResizeToFillDown() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -455,7 +467,7 @@ func (s *ProcessingTestSuite) TestResizeToFillDownEnlarge() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -501,7 +513,7 @@ func (s *ProcessingTestSuite) TestResizeToFillDownExtend() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -545,7 +557,7 @@ func (s *ProcessingTestSuite) TestResizeToFillDownExtendAR() { po.Width = tc.width po.Height = tc.height - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -974,7 +986,7 @@ func (s *ProcessingTestSuite) TestResultSizeLimit() { po.Rotate = tc.rotate po.Padding = tc.padding - result, err := ProcessImage(context.Background(), imgdata, po) + result, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().NoError(err) s.Require().NotNil(result) @@ -989,7 +1001,7 @@ func (s *ProcessingTestSuite) TestImageResolutionTooLarge() { po.SecurityOptions.MaxSrcResolution = 1 imgdata := s.openFile("test2.jpg") - _, err := ProcessImage(context.Background(), imgdata, po) + _, err := ProcessImage(context.Background(), imgdata, po, nil, s.idf) s.Require().Error(err) s.Require().Equal(422, ierrors.Wrap(err, 0).StatusCode()) diff --git a/processing/watermark.go b/processing/watermark.go index d6d56012..6d235edf 100644 --- a/processing/watermark.go +++ b/processing/watermark.go @@ -4,6 +4,7 @@ import ( "context" "math" + "github.com/imgproxy/imgproxy/v3/auximageprovider" "github.com/imgproxy/imgproxy/v3/config" "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/imath" @@ -59,17 +60,14 @@ func prepareWatermark(wm *vips.Image, wmData imagedata.ImageData, opts *options. po.Padding.Bottom = offY - po.Padding.Top } - if err := watermarkPipeline.Run(context.Background(), wm, po, wmData); err != nil { + if err := watermarkPipeline.Run(context.Background(), wm, po, wmData, nil); err != nil { return err } - if opts.ShouldReplicate() || framesCount > 1 { - // We need to copy image if we're going to replicate. - // Replication requires image to be read several times, and this requires - // random access to pixels - if err := wm.CopyMemory(); err != nil { - return err - } + // We need to copy the image to ensure that it is in memory since we will + // close it after watermark processing is done. + if err := wm.CopyMemory(); err != nil { + return err } if opts.ShouldReplicate() { @@ -82,7 +80,20 @@ func prepareWatermark(wm *vips.Image, wmData imagedata.ImageData, opts *options. return wm.StripAll() } -func applyWatermark(img *vips.Image, wmData imagedata.ImageData, opts *options.WatermarkOptions, offsetScale float64, framesCount int) error { +func applyWatermark(img *vips.Image, watermark auximageprovider.Provider, opts *options.WatermarkOptions, offsetScale float64, framesCount int) error { + if watermark == nil { + return nil + } + + wmData, _, err := watermark.Get(context.Background(), nil) + if err != nil { + return err + } + if wmData == nil { + return nil + } + defer wmData.Close() + wm := new(vips.Image) defer wm.Clear() @@ -164,9 +175,9 @@ func applyWatermark(img *vips.Image, wmData imagedata.ImageData, opts *options.W } func watermark(pctx *pipelineContext, img *vips.Image, po *options.ProcessingOptions, imgdata imagedata.ImageData) error { - if !po.Watermark.Enabled || imagedata.Watermark == nil { + if !po.Watermark.Enabled || pctx.watermarkProvider == nil { return nil } - return applyWatermark(img, imagedata.Watermark, &po.Watermark, pctx.dprScale, 1) + return applyWatermark(img, pctx.watermarkProvider, &po.Watermark, pctx.dprScale, 1) } diff --git a/processing_handler_test.go b/processing_handler_test.go index b2bd2f18..60950897 100644 --- a/processing_handler_test.go +++ b/processing_handler_test.go @@ -21,12 +21,14 @@ import ( "github.com/imgproxy/imgproxy/v3/config" "github.com/imgproxy/imgproxy/v3/config/configurators" + "github.com/imgproxy/imgproxy/v3/fetcher" "github.com/imgproxy/imgproxy/v3/httpheaders" "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/imagetype" "github.com/imgproxy/imgproxy/v3/server" "github.com/imgproxy/imgproxy/v3/svg" "github.com/imgproxy/imgproxy/v3/testutil" + "github.com/imgproxy/imgproxy/v3/transport" "github.com/imgproxy/imgproxy/v3/vips" ) @@ -63,10 +65,13 @@ func (s *ProcessingHandlerTestSuite) TeardownSuite() { } func (s *ProcessingHandlerTestSuite) SetupTest() { - // We don't need config.LocalFileSystemRoot anymore as it is used - // only during initialization + wd, err := os.Getwd() + s.Require().NoError(err) + config.Reset() config.AllowLoopbackSourceAddresses = true + config.LocalFileSystemRoot = filepath.Join(wd, "/testdata") + config.ClientKeepAliveTimeout = 0 } func (s *ProcessingHandlerTestSuite) send(path string, header ...http.Header) *httptest.ResponseRecorder { @@ -99,7 +104,26 @@ func (s *ProcessingHandlerTestSuite) readTestImageData(name string) imagedata.Im data, err := os.ReadFile(filepath.Join(wd, "testdata", name)) s.Require().NoError(err) - imgdata, err := imagedata.NewFromBytes(data) + // NOTE: Temporary workaround. We create imagedata.Factory here + // because currently configuration is changed via env vars + // or config. We need to pick up those config changes. + // This will be addressed in the next PR + trc, err := transport.LoadFromEnv(transport.NewDefaultConfig()) + s.Require().NoError(err) + + tr, err := transport.New(trc) + s.Require().NoError(err) + + fc, err := fetcher.LoadFromEnv(fetcher.NewDefaultConfig()) + s.Require().NoError(err) + + f, err := fetcher.New(tr, fc) + s.Require().NoError(err) + + idf := imagedata.NewFactory(f) + // end of workaround + + imgdata, err := idf.NewFromBytes(data) s.Require().NoError(err) return imgdata diff --git a/svg/svg_test.go b/svg/svg_test.go index 0d5d326c..30d670e5 100644 --- a/svg/svg_test.go +++ b/svg/svg_test.go @@ -8,19 +8,29 @@ import ( "github.com/stretchr/testify/suite" "github.com/imgproxy/imgproxy/v3/config" + "github.com/imgproxy/imgproxy/v3/fetcher" "github.com/imgproxy/imgproxy/v3/imagedata" "github.com/imgproxy/imgproxy/v3/testutil" + "github.com/imgproxy/imgproxy/v3/transport" ) type SvgTestSuite struct { + idf *imagedata.Factory suite.Suite } func (s *SvgTestSuite) SetupSuite() { config.Reset() - err := imagedata.Init() + trc := transport.NewDefaultConfig() + tr, err := transport.New(trc) s.Require().NoError(err) + + fc := fetcher.NewDefaultConfig() + f, err := fetcher.New(tr, fc) + s.Require().NoError(err) + + s.idf = imagedata.NewFactory(f) } func (s *SvgTestSuite) readTestFile(name string) imagedata.ImageData { @@ -30,7 +40,7 @@ func (s *SvgTestSuite) readTestFile(name string) imagedata.ImageData { data, err := os.ReadFile(filepath.Join(wd, "..", "testdata", name)) s.Require().NoError(err) - d, err := imagedata.NewFromBytes(data) + d, err := s.idf.NewFromBytes(data) s.Require().NoError(err) return d