Better expected errors handling

download.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
-	"errors"
 	"fmt"
 	"image"
 	"io"
@@ -25,11 +24,13 @@ var (
 	imageTypeCtxKey = ctxKey("imageType")
 	imageDataCtxKey = ctxKey("imageData")
 
-	errSourceDimensionsTooBig      = errors.New("Source image dimensions are too big")
-	errSourceResolutionTooBig      = errors.New("Source image resolution are too big")
-	errSourceImageTypeNotSupported = errors.New("Source image type not supported")
+	errSourceDimensionsTooBig      = newError(422, "Source image dimensions are too big", "Invalid source image")
+	errSourceResolutionTooBig      = newError(422, "Source image resolution are too big", "Invalid source image")
+	errSourceImageTypeNotSupported = newError(422, "Source image type not supported", "Invalid source image")
 )
 
+const msgSourceImageIsUnreachable = "Source image is unreachable"
+
 var downloadBufPool = sync.Pool{
 	New: func() interface{} {
 		return new(bytes.Buffer)
@@ -78,7 +79,7 @@ func checkDimensions(width, height int) error {
 func checkTypeAndDimensions(r io.Reader) (imageType, error) {
 	imgconf, imgtypeStr, err := image.DecodeConfig(r)
 	if err != nil {
-		return imageTypeUnknown, err
+		return imageTypeUnknown, errSourceImageTypeNotSupported
 	}
 
 	imgtype, imgtypeOk := imageTypes[imgtypeStr]
@@ -105,12 +106,14 @@ func readAndCheckImage(ctx context.Context, res *http.Response) (context.Context
 		return ctx, cancel, err
 	}
 
-	if _, err = buf.ReadFrom(res.Body); err == nil {
-		ctx = context.WithValue(ctx, imageTypeCtxKey, imgtype)
-		ctx = context.WithValue(ctx, imageDataCtxKey, buf)
+	if _, err = buf.ReadFrom(res.Body); err != nil {
+		return ctx, cancel, newError(404, err.Error(), msgSourceImageIsUnreachable)
 	}
 
-	return ctx, cancel, err
+	ctx = context.WithValue(ctx, imageTypeCtxKey, imgtype)
+	ctx = context.WithValue(ctx, imageDataCtxKey, buf)
+
+	return ctx, cancel, nil
 }
 
 func downloadImage(ctx context.Context) (context.Context, context.CancelFunc, error) {
@@ -127,20 +130,21 @@ func downloadImage(ctx context.Context) (context.Context, context.CancelFunc, er
 
 	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
-		return ctx, func() {}, err
+		return ctx, func() {}, newError(404, err.Error(), msgSourceImageIsUnreachable)
 	}
 
 	req.Header.Set("User-Agent", conf.UserAgent)
 
 	res, err := downloadClient.Do(req)
 	if err != nil {
-		return ctx, func() {}, err
+		return ctx, func() {}, newError(404, err.Error(), msgSourceImageIsUnreachable)
 	}
 	defer res.Body.Close()
 
 	if res.StatusCode != 200 {
 		body, _ := ioutil.ReadAll(res.Body)
-		return ctx, func() {}, fmt.Errorf("Can't download image; Status: %d; %s", res.StatusCode, string(body))
+		msg := fmt.Sprintf("Can't download image; Status: %d; %s", res.StatusCode, string(body))
+		return ctx, func() {}, newError(404, msg, msgSourceImageIsUnreachable)
 	}
 
 	return readAndCheckImage(ctx, res)

errors.go

@@ -13,17 +13,17 @@ type imgproxyError struct {
 	PublicMessage string
 }
 
-func (e imgproxyError) Error() string {
+func (e *imgproxyError) Error() string {
 	return e.Message
 }
 
-func newError(status int, msg string, pub string) imgproxyError {
-	return imgproxyError{status, msg, pub}
+func newError(status int, msg string, pub string) *imgproxyError {
+	return &imgproxyError{status, msg, pub}
 }
 
-func newUnexpectedError(err error, skip int) imgproxyError {
+func newUnexpectedError(err error, skip int) *imgproxyError {
 	msg := fmt.Sprintf("Unexpected error: %s\n%s", err, stacktrace(skip+1))
-	return imgproxyError{500, msg, "Internal error"}
+	return &imgproxyError{500, msg, "Internal error"}
 }
 
 func stacktrace(skip int) string {

etag.go

@@ -9,8 +9,6 @@ import (
 	"sync"
 )
 
-var errNotModified = newError(304, "Not modified", "Not modified")
-
 type eTagCalc struct {
 	hash hash.Hash
 	enc  *json.Encoder

processing_options.go

@@ -141,13 +141,16 @@ const (
 	processingOptionsCtxKey = ctxKey("processingOptions")
 	urlTokenPlain           = "plain"
 	maxClientHintDPR        = 8
+
+	msgForbidden  = "Forbidden"
+	msgInvalidURL = "Invalid URL"
 )
 
 var (
 	errInvalidImageURL                    = errors.New("Invalid image url")
 	errInvalidURLEncoding                 = errors.New("Invalid url encoding")
-	errInvalidPath                        = errors.New("Invalid path")
 	errResultingImageFormatIsNotSupported = errors.New("Resulting image format is not supported")
+	errInvalidPath                        = newError(404, "Invalid path", msgInvalidURL)
 )
 
 func (it imageType) String() string {
@@ -836,7 +839,7 @@ func parsePath(ctx context.Context, r *http.Request) (context.Context, error) {
 
 	if !conf.AllowInsecure {
 		if err := validatePath(parts[0], strings.TrimPrefix(path, fmt.Sprintf("/%s", parts[0]))); err != nil {
-			return ctx, err
+			return ctx, newError(403, err.Error(), msgForbidden)
 		}
 	}
 
@@ -858,13 +861,13 @@ func parsePath(ctx context.Context, r *http.Request) (context.Context, error) {
 	}
 
 	if err != nil {
-		return ctx, err
+		return ctx, newError(404, err.Error(), msgInvalidURL)
 	}
 
 	ctx = context.WithValue(ctx, imageURLCtxKey, imageURL)
 	ctx = context.WithValue(ctx, processingOptionsCtxKey, po)
 
-	return ctx, err
+	return ctx, nil
 }
 
 func getImageURL(ctx context.Context) string {

server.go

@@ -135,7 +135,7 @@ func respondWithImage(ctx context.Context, reqID string, r *http.Request, rw htt
 	logResponse(200, fmt.Sprintf("[%s] Processed in %s: %s; %+v", reqID, getTimerSince(ctx), getImageURL(ctx), po))
 }
 
-func respondWithError(reqID string, rw http.ResponseWriter, err imgproxyError) {
+func respondWithError(reqID string, rw http.ResponseWriter, err *imgproxyError) {
 	logResponse(err.StatusCode, fmt.Sprintf("[%s] %s", reqID, err.Message))
 
 	rw.WriteHeader(err.StatusCode)
@@ -147,6 +147,11 @@ func respondWithOptions(reqID string, rw http.ResponseWriter) {
 	rw.WriteHeader(200)
 }
 
+func respondWithNotModified(reqID string, rw http.ResponseWriter) {
+	logResponse(200, fmt.Sprintf("[%s] Not modified", reqID))
+	rw.WriteHeader(304)
+}
+
 func prepareAuthHeaderMust() []byte {
 	if len(authHeaderMust) == 0 {
 		authHeaderMust = []byte(fmt.Sprintf("Bearer %s", conf.Secret))
@@ -180,11 +185,9 @@ func (h *httpHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	defer func() {
 		if rerr := recover(); rerr != nil {
 			if err, ok := rerr.(error); ok {
-				if err != errNotModified {
-					reportError(err, r)
-				}
+				reportError(err, r)
 
-				if ierr, ok := err.(imgproxyError); ok {
+				if ierr, ok := err.(*imgproxyError); ok {
 					respondWithError(reqID, rw, ierr)
 				} else {
 					respondWithError(reqID, rw, newUnexpectedError(err, 4))
@@ -239,7 +242,7 @@ func (h *httpHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 
 	ctx, err := parsePath(ctx, r)
 	if err != nil {
-		panic(newError(404, err.Error(), "Invalid image url"))
+		panic(err)
 	}
 
 	ctx, downloadcancel, err := downloadImage(ctx)
@@ -251,7 +254,7 @@ func (h *httpHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 		if prometheusEnabled {
 			incrementPrometheusErrorsTotal("download")
 		}
-		panic(newError(404, err.Error(), "Image is unreachable"))
+		panic(err)
 	}
 
 	checkTimeout(ctx)
@@ -261,7 +264,8 @@ func (h *httpHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 		rw.Header().Set("ETag", eTag)
 
 		if eTag == r.Header.Get("If-None-Match") {
-			panic(errNotModified)
+			respondWithNotModified(reqID, rw)
+			return
 		}
 	}
 
@@ -275,7 +279,7 @@ func (h *httpHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 		if prometheusEnabled {
 			incrementPrometheusErrorsTotal("processing")
 		}
-		panic(newError(500, err.Error(), "Error occurred while processing image"))
+		panic(err)
 	}
 
 	checkTimeout(ctx)