Переглянути джерело

Move handler errors and path/signature splitting to handlers package

DarthSim 3 тижнів тому
батько
коміт
2fcb39085e

+ 20 - 20
handlers/processing/errors.go → 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))
 }

+ 9 - 8
handlers/processing/path.go → 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),
 		)
 	}
 

+ 4 - 6
handlers/processing/path_test.go → 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
 			}

+ 7 - 10
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

+ 5 - 4
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

+ 7 - 6
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

+ 10 - 9
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 {

+ 3 - 0
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)
 		}