From 2fcb39085e16796accb733f52d6183b70a1b1d30 Mon Sep 17 00:00:00 2001 From: DarthSim Date: Wed, 10 Sep 2025 18:47:43 +0300 Subject: [PATCH] Move handler errors and path/signature splitting to handlers package --- handlers/{processing => }/errors.go | 40 +++++++++++++------------- handlers/{processing => }/path.go | 17 +++++------ handlers/{processing => }/path_test.go | 10 +++---- handlers/processing/config.go | 17 +++++------ handlers/processing/handler.go | 9 +++--- handlers/processing/request.go | 13 +++++---- handlers/processing/request_methods.go | 19 ++++++------ server/router.go | 3 ++ 8 files changed, 65 insertions(+), 63 deletions(-) rename handlers/{processing => }/errors.go (52%) rename handlers/{processing => }/path.go (68%) rename handlers/{processing => }/path_test.go (95%) diff --git a/handlers/processing/errors.go b/handlers/errors.go similarity index 52% rename from handlers/processing/errors.go rename to handlers/errors.go index bf58d0d8..89486cdf 100644 --- a/handlers/processing/errors.go +++ b/handlers/errors.go @@ -1,4 +1,4 @@ -package processing +package handlers import ( "fmt" @@ -10,15 +10,15 @@ import ( // Monitoring error categories const ( - categoryTimeout = "timeout" - categoryImageDataSize = "image_data_size" - categoryPathParsing = "path_parsing" - categorySecurity = "security" - categoryQueue = "queue" - categoryDownload = "download" - categoryProcessing = "processing" - categoryIO = "IO" - categoryConfig = "config(tmp)" // NOTE: THIS IS TEMPORARY + CategoryTimeout = "timeout" + CategoryImageDataSize = "image_data_size" + CategoryPathParsing = "path_parsing" + CategorySecurity = "security" + CategoryQueue = "queue" + CategoryDownload = "download" + CategoryProcessing = "processing" + CategoryIO = "IO" + CategoryConfig = "config(tmp)" // NOTE: THIS IS TEMPORARY ) type ( @@ -26,7 +26,7 @@ type ( InvalidURLError string ) -func newResponseWriteError(cause error) *ierrors.Error { +func NewResponseWriteError(cause error) *ierrors.Error { return ierrors.Wrap( ResponseWriteError{cause}, 1, @@ -42,7 +42,7 @@ func (e ResponseWriteError) Unwrap() error { return e.error } -func newInvalidURLErrorf(status int, format string, args ...interface{}) error { +func NewInvalidURLErrorf(status int, format string, args ...interface{}) error { return ierrors.Wrap( InvalidURLError(fmt.Sprintf(format, args...)), 1, @@ -54,18 +54,18 @@ func newInvalidURLErrorf(status int, format string, args ...interface{}) error { func (e InvalidURLError) Error() string { return string(e) } -// newCantSaveError creates "resulting image not supported" error -func newCantSaveError(format imagetype.Type) error { - return ierrors.Wrap(newInvalidURLErrorf( +// NewCantSaveError creates "resulting image not supported" error +func NewCantSaveError(format imagetype.Type) error { + return ierrors.Wrap(NewInvalidURLErrorf( http.StatusUnprocessableEntity, "Resulting image format is not supported: %s", format, - ), 1, ierrors.WithCategory(categoryPathParsing)) + ), 1, ierrors.WithCategory(CategoryPathParsing)) } -// newCantLoadError creates "source image not supported" error -func newCantLoadError(format imagetype.Type) error { - return ierrors.Wrap(newInvalidURLErrorf( +// NewCantLoadError creates "source image not supported" error +func NewCantLoadError(format imagetype.Type) error { + return ierrors.Wrap(NewInvalidURLErrorf( http.StatusUnprocessableEntity, "Source image format is not supported: %s", format, - ), 1, ierrors.WithCategory(categoryProcessing)) + ), 1, ierrors.WithCategory(CategoryProcessing)) } diff --git a/handlers/processing/path.go b/handlers/path.go similarity index 68% rename from handlers/processing/path.go rename to handlers/path.go index be6c5d64..c2367659 100644 --- a/handlers/processing/path.go +++ b/handlers/path.go @@ -1,4 +1,4 @@ -package processing +package handlers import ( "fmt" @@ -12,16 +12,17 @@ import ( // fixPathRe is used in path re-denormalization var fixPathRe = regexp.MustCompile(`/plain/(\S+)\:/([^/])`) -// splitPathSignature splits signature and path components from the request URI -func splitPathSignature(r *http.Request, config *Config) (string, string, error) { +// SplitPathSignature splits signature and path components from the request URI +func SplitPathSignature(r *http.Request) (string, string, error) { uri := r.RequestURI // cut query params uri, _, _ = strings.Cut(uri, "?") - // cut path prefix - if len(config.PathPrefix) > 0 { - uri = strings.TrimPrefix(uri, config.PathPrefix) + // Cut path prefix. + // r.Pattern is set by the router and contains both global and route-specific prefixes combined. + if len(r.Pattern) > 0 { + uri = strings.TrimPrefix(uri, r.Pattern) } // cut leading slash @@ -30,8 +31,8 @@ func splitPathSignature(r *http.Request, config *Config) (string, string, error) signature, path, _ := strings.Cut(uri, "/") if len(signature) == 0 || len(path) == 0 { return "", "", ierrors.Wrap( - newInvalidURLErrorf(http.StatusNotFound, "Invalid path: %s", path), 0, - ierrors.WithCategory(categoryPathParsing), + NewInvalidURLErrorf(http.StatusNotFound, "Invalid path: %s", path), 0, + ierrors.WithCategory(CategoryPathParsing), ) } diff --git a/handlers/processing/path_test.go b/handlers/path_test.go similarity index 95% rename from handlers/processing/path_test.go rename to handlers/path_test.go index 12e19f60..d360c9aa 100644 --- a/handlers/processing/path_test.go +++ b/handlers/path_test.go @@ -1,4 +1,4 @@ -package processing +package handlers import ( "net/http" @@ -84,19 +84,17 @@ func (s *PathTestSuite) TestParsePath() { for _, tc := range testCases { s.Run(tc.name, func() { - config := &Config{ - PathPrefix: tc.pathPrefix, - } req := s.createRequest(tc.requestPath) - path, signature, err := splitPathSignature(req, config) + req.Pattern = tc.pathPrefix + path, signature, err := SplitPathSignature(req) if tc.expectedError { var ierr *ierrors.Error s.Require().Error(err) s.Require().ErrorAs(err, &ierr) - s.Require().Equal(categoryPathParsing, ierr.Category()) + s.Require().Equal(CategoryPathParsing, ierr.Category()) return } diff --git a/handlers/processing/config.go b/handlers/processing/config.go index 1569be48..e2bb8460 100644 --- a/handlers/processing/config.go +++ b/handlers/processing/config.go @@ -10,20 +10,18 @@ import ( // Config represents handler config type Config struct { - PathPrefix string // Route path prefix - CookiePassthrough bool // Whether to passthrough cookies - ReportDownloadingErrors bool // Whether to report downloading errors - LastModifiedEnabled bool // Whether to enable Last-Modified - ETagEnabled bool // Whether to enable ETag - ReportIOErrors bool // Whether to report IO errors - FallbackImageHTTPCode int // Fallback image HTTP status code - EnableDebugHeaders bool // Whether to enable debug headers + CookiePassthrough bool // Whether to passthrough cookies + ReportDownloadingErrors bool // Whether to report downloading errors + LastModifiedEnabled bool // Whether to enable Last-Modified + ETagEnabled bool // Whether to enable ETag + ReportIOErrors bool // Whether to report IO errors + FallbackImageHTTPCode int // Fallback image HTTP status code + EnableDebugHeaders bool // Whether to enable debug headers } // NewDefaultConfig creates a new configuration with defaults func NewDefaultConfig() Config { return Config{ - PathPrefix: "", CookiePassthrough: false, ReportDownloadingErrors: true, LastModifiedEnabled: true, @@ -38,7 +36,6 @@ func NewDefaultConfig() Config { func LoadConfigFromEnv(c *Config) (*Config, error) { c = ensure.Ensure(c, NewDefaultConfig) - c.PathPrefix = config.PathPrefix c.CookiePassthrough = config.CookiePassthrough c.ReportDownloadingErrors = config.ReportDownloadingErrors c.LastModifiedEnabled = config.LastModifiedEnabled diff --git a/handlers/processing/handler.go b/handlers/processing/handler.go index 130db1a2..14ded987 100644 --- a/handlers/processing/handler.go +++ b/handlers/processing/handler.go @@ -7,6 +7,7 @@ import ( "github.com/imgproxy/imgproxy/v3/auximageprovider" "github.com/imgproxy/imgproxy/v3/errorreport" + "github.com/imgproxy/imgproxy/v3/handlers" "github.com/imgproxy/imgproxy/v3/handlers/stream" "github.com/imgproxy/imgproxy/v3/headerwriter" "github.com/imgproxy/imgproxy/v3/ierrors" @@ -100,20 +101,20 @@ func (h *Handler) newRequest( imageRequest *http.Request, ) (string, *options.ProcessingOptions, monitoring.Meta, error) { // let's extract signature and valid request path from a request - path, signature, err := splitPathSignature(imageRequest, h.config) + path, signature, err := handlers.SplitPathSignature(imageRequest) if err != nil { return "", nil, nil, err } // verify the signature (if any) if err = security.VerifySignature(signature, path); err != nil { - return "", nil, nil, ierrors.Wrap(err, 0, ierrors.WithCategory(categorySecurity)) + return "", nil, nil, ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategorySecurity)) } // parse image url and processing options po, imageURL, err := options.ParsePath(path, imageRequest.Header) if err != nil { - return "", nil, nil, ierrors.Wrap(err, 0, ierrors.WithCategory(categoryPathParsing)) + return "", nil, nil, ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategoryPathParsing)) } // get image origin and create monitoring meta object @@ -135,7 +136,7 @@ func (h *Handler) newRequest( // verify that image URL came from the valid source err = security.VerifySourceURL(imageURL) if err != nil { - return "", nil, mm, ierrors.Wrap(err, 0, ierrors.WithCategory(categorySecurity)) + return "", nil, mm, ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategorySecurity)) } return imageURL, po, mm, nil diff --git a/handlers/processing/request.go b/handlers/processing/request.go index ae35ae8a..eb95f558 100644 --- a/handlers/processing/request.go +++ b/handlers/processing/request.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/imgproxy/imgproxy/v3/fetcher" + "github.com/imgproxy/imgproxy/v3/handlers" "github.com/imgproxy/imgproxy/v3/headerwriter" "github.com/imgproxy/imgproxy/v3/ierrors" "github.com/imgproxy/imgproxy/v3/imagedata" @@ -41,7 +42,7 @@ func (r *request) execute(ctx context.Context) error { r.po.Format == imagetype.SVG if !canSave { - return newCantSaveError(r.po.Format) + return handlers.NewCantSaveError(r.po.Format) } // Acquire queue semaphore (if enabled) @@ -79,7 +80,7 @@ func (r *request) execute(ctx context.Context) error { // Check that image detection didn't take too long if terr := server.CheckTimeout(ctx); terr != nil { - return ierrors.Wrap(terr, 0, ierrors.WithCategory(categoryTimeout)) + return ierrors.Wrap(terr, 0, ierrors.WithCategory(handlers.CategoryTimeout)) } // Respond with NotModified if image was not modified @@ -104,7 +105,7 @@ func (r *request) execute(ctx context.Context) error { // Check if image supports load from origin format if !vips.SupportsLoad(originData.Format()) { - return newCantLoadError(originData.Format()) + return handlers.NewCantLoadError(originData.Format()) } // Actually process the image @@ -117,19 +118,19 @@ func (r *request) execute(ctx context.Context) error { // First, check if the processing error wasn't caused by an image data error if derr := originData.Error(); derr != nil { - return ierrors.Wrap(derr, 0, ierrors.WithCategory(categoryDownload)) + return ierrors.Wrap(derr, 0, ierrors.WithCategory(handlers.CategoryDownload)) } // If it wasn't, than it was a processing error if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryProcessing)) + return ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategoryProcessing)) } // Write debug headers. It seems unlogical to move they to headerwriter since they're // not used anywhere else. err = r.writeDebugHeaders(result, originData) if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryImageDataSize)) + return ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategoryImageDataSize)) } // Responde with actual image diff --git a/handlers/processing/request_methods.go b/handlers/processing/request_methods.go index c2791105..92baea73 100644 --- a/handlers/processing/request_methods.go +++ b/handlers/processing/request_methods.go @@ -8,6 +8,7 @@ import ( "github.com/imgproxy/imgproxy/v3/cookies" "github.com/imgproxy/imgproxy/v3/errorreport" + "github.com/imgproxy/imgproxy/v3/handlers" "github.com/imgproxy/imgproxy/v3/httpheaders" "github.com/imgproxy/imgproxy/v3/ierrors" "github.com/imgproxy/imgproxy/v3/imagedata" @@ -45,12 +46,12 @@ func (r *request) acquireProcessingSem(ctx context.Context) (context.CancelFunc, // but it's an easy way to check if this is an actual timeout // or the request was canceled if terr := server.CheckTimeout(ctx); terr != nil { - return nil, ierrors.Wrap(terr, 0, ierrors.WithCategory(categoryTimeout)) + return nil, ierrors.Wrap(terr, 0, ierrors.WithCategory(handlers.CategoryTimeout)) } // We should never reach this line as err could be only ctx.Err() // and we've already checked for it. But beter safe than sorry - return nil, ierrors.Wrap(err, 0, ierrors.WithCategory(categoryQueue)) + return nil, ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategoryQueue)) } return fn, nil @@ -77,7 +78,7 @@ func (r *request) fetchImage(ctx context.Context, do imagedata.DownloadOptions) if r.config.CookiePassthrough { do.CookieJar, err = cookies.JarFromRequest(r.imageRequest) if err != nil { - return nil, nil, ierrors.Wrap(err, 0, ierrors.WithCategory(categoryDownload)) + return nil, nil, ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategoryDownload)) } } @@ -98,7 +99,7 @@ func (r *request) handleDownloadError( } // Just send error - monitoring.SendError(ctx, categoryDownload, err) + monitoring.SendError(ctx, handlers.CategoryDownload, err) // We didn't return, so we have to report error if err.ShouldReport() { @@ -173,7 +174,7 @@ func (r *request) writeDebugHeaders(result *processing.Result, originData imaged // Try to read origin image size size, err := originData.Size() if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryImageDataSize)) + return ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategoryImageDataSize)) } r.rw.Header().Set(httpheaders.XOriginContentLength, strconv.Itoa(size)) @@ -215,7 +216,7 @@ func (r *request) respondWithImage(statusCode int, resultData imagedata.ImageDat // errors happened. resultSize, err := resultData.Size() if err != nil { - return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryImageDataSize)) + return ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategoryImageDataSize)) } r.hwr.SetContentType(resultData.Format().Mime()) @@ -247,10 +248,10 @@ func (r *request) respondWithImage(statusCode int, resultData imagedata.ImageDat var ierr *ierrors.Error if err != nil { - ierr = newResponseWriteError(err) + ierr = handlers.NewResponseWriteError(err) if r.config.ReportIOErrors { - return ierrors.Wrap(ierr, 0, ierrors.WithCategory(categoryIO), ierrors.WithShouldReport(true)) + return ierrors.Wrap(ierr, 0, ierrors.WithCategory(handlers.CategoryIO), ierrors.WithShouldReport(true)) } } @@ -267,7 +268,7 @@ func (r *request) respondWithImage(statusCode int, resultData imagedata.ImageDat // wrapDownloadingErr wraps original error to download error func (r *request) wrapDownloadingErr(originalErr error) *ierrors.Error { - err := ierrors.Wrap(originalErr, 0, ierrors.WithCategory(categoryDownload)) + err := ierrors.Wrap(originalErr, 0, ierrors.WithCategory(handlers.CategoryDownload)) // we report this error only if enabled if r.config.ReportDownloadingErrors { diff --git a/server/router.go b/server/router.go index c32a6e27..4349bbeb 100644 --- a/server/router.go +++ b/server/router.go @@ -131,6 +131,9 @@ func (r *Router) ServeHTTP(rw http.ResponseWriter, req *http.Request) { continue } + // Set req.Pattern. We use it to trim path prefixes in handlers. + req.Pattern = rr.path + if !rr.silent { LogRequest(reqID, req) }