Respond with 500 for "temporary" downloading errors

CHANGELOG.md

@@ -7,6 +7,8 @@
 ### Change
 - `dpr` processing option doesn't enlarge image unless `enlarge` is true.
 - `304 Not Modified` responses includes `Cache-Control`, `Expires`, and `Vary` headers.
+- imgproxy responds with `500` HTTP code when the source image downloading error seems temporary (timeout, server error, etc).
+- When `IMGPROXY_FALLBACK_IMAGE_HTTP_CODE` is zero, imgproxy responds with the usual HTTP code.
 
 ### Fix
 - Fix Client Hints behavior. `Width` is physical size, so we should divide it by `DPR` value.

docs/configuration.md

@@ -221,7 +221,7 @@ You can set up a fallback image that will be used in case imgproxy can't fetch t
 * `IMGPROXY_FALLBACK_IMAGE_DATA`: Base64-encoded image data. You can easily calculate it with `base64 tmp/fallback.png | tr -d '\n'`;
 * `IMGPROXY_FALLBACK_IMAGE_PATH`: path to the locally stored image;
 * `IMGPROXY_FALLBACK_IMAGE_URL`: fallback image URL.
-* `IMGPROXY_FALLBACK_IMAGE_HTTP_CODE`: <i class='badge badge-v3'></i> HTTP code for the fallback image response. Default: `200`.
+* `IMGPROXY_FALLBACK_IMAGE_HTTP_CODE`: <i class='badge badge-v3'></i> HTTP code for the fallback image response. When set to zero, imgproxy will respond with the usual HTTP code. Default: `200`.
 * `IMGPROXY_FALLBACK_IMAGES_CACHE_SIZE`: <i class='badge badge-pro'></i> <i class='badge badge-v3'></i> size of custom fallback images cache. When set to `0`, fallback images cache is disabled. By default 256 fallback images are cached.
 
 ## Skip processing

imagedata/download.go

@@ -21,6 +21,11 @@ import (
 var (
 	downloadClient *http.Client
 
+	enabledSchemes = map[string]struct{}{
+		"http":  {},
+		"https": {},
+	}
+
 	imageHeadersToStore = []string{
 		"Cache-Control",
 		"Expires",
@@ -55,15 +60,20 @@ func initDownloading() error {
 		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
 	}
 
+	registerProtocol := func(scheme string, rt http.RoundTripper) {
+		transport.RegisterProtocol(scheme, rt)
+		enabledSchemes[scheme] = struct{}{}
+	}
+
 	if config.LocalFileSystemRoot != "" {
-		transport.RegisterProtocol("local", fsTransport.New())
+		registerProtocol("local", fsTransport.New())
 	}
 
 	if config.S3Enabled {
 		if t, err := s3Transport.New(); err != nil {
 			return err
 		} else {
-			transport.RegisterProtocol("s3", t)
+			registerProtocol("s3", t)
 		}
 	}
 
@@ -71,7 +81,7 @@ func initDownloading() error {
 		if t, err := gcsTransport.New(); err != nil {
 			return err
 		} else {
-			transport.RegisterProtocol("gs", t)
+			registerProtocol("gs", t)
 		}
 	}
 
@@ -79,7 +89,7 @@ func initDownloading() error {
 		if t, err := azureTransport.New(); err != nil {
 			return err
 		} else {
-			transport.RegisterProtocol("abs", t)
+			registerProtocol("abs", t)
 		}
 	}
 
@@ -109,6 +119,14 @@ func requestImage(imageURL string, header http.Header) (*http.Response, error) {
 		return nil, ierrors.New(404, err.Error(), msgSourceImageIsUnreachable).SetUnexpected(config.ReportDownloadingErrors)
 	}
 
+	if _, ok := enabledSchemes[req.URL.Scheme]; !ok {
+		return nil, ierrors.New(
+			404,
+			fmt.Sprintf("Unknown sheme: %s", req.URL.Scheme),
+			msgSourceImageIsUnreachable,
+		).SetUnexpected(config.ReportDownloadingErrors)
+	}
+
 	req.Header.Set("User-Agent", config.UserAgent)
 
 	for k, v := range header {
@@ -119,7 +137,7 @@ func requestImage(imageURL string, header http.Header) (*http.Response, error) {
 
 	res, err := downloadClient.Do(req)
 	if err != nil {
-		return res, ierrors.New(404, checkTimeoutErr(err).Error(), msgSourceImageIsUnreachable).SetUnexpected(config.ReportDownloadingErrors)
+		return nil, ierrors.New(500, checkTimeoutErr(err).Error(), msgSourceImageIsUnreachable).SetUnexpected(config.ReportDownloadingErrors)
 	}
 
 	if res.StatusCode == http.StatusNotModified {
@@ -130,8 +148,13 @@ func requestImage(imageURL string, header http.Header) (*http.Response, error) {
 		body, _ := ioutil.ReadAll(res.Body)
 		res.Body.Close()
 
+		status := 404
+		if res.StatusCode >= 500 {
+			status = 500
+		}
+
 		msg := fmt.Sprintf("Status: %d; %s", res.StatusCode, string(body))
-		return res, ierrors.New(404, msg, msgSourceImageIsUnreachable).SetUnexpected(config.ReportDownloadingErrors)
+		return nil, ierrors.New(status, msg, msgSourceImageIsUnreachable).SetUnexpected(config.ReportDownloadingErrors)
 	}
 
 	return res, nil
@@ -168,7 +191,7 @@ func download(imageURL string, header http.Header) (*ImageData, error) {
 
 	imgdata, err := readAndCheckImage(body, contentLength)
 	if err != nil {
-		return nil, err
+		return nil, ierrors.Wrap(err, 0).SetUnexpected(config.ReportDownloadingErrors)
 	}
 
 	imgdata.Headers = headersToStore(res)

imagedata/read.go

@@ -58,7 +58,7 @@ func readAndCheckImage(r io.Reader, contentLength int) (*ImageData, error) {
 		return nil, ErrSourceImageTypeNotSupported
 	}
 	if err != nil {
-		return nil, ierrors.Wrap(checkTimeoutErr(err), 0)
+		return nil, checkTimeoutErr(err)
 	}
 
 	if err = security.CheckDimensions(meta.Width(), meta.Height()); err != nil {
@@ -67,7 +67,7 @@ func readAndCheckImage(r io.Reader, contentLength int) (*ImageData, error) {
 
 	if err = br.Flush(); err != nil {
 		cancel()
-		return nil, ierrors.New(404, checkTimeoutErr(err).Error(), msgSourceImageIsUnreachable).SetUnexpected(config.ReportDownloadingErrors)
+		return nil, checkTimeoutErr(err)
 	}
 
 	return &ImageData{

processing_handler.go

@@ -30,8 +30,6 @@ var (
 	headerVaryValue string
 )
 
-type fallbackImageUsedCtxKey struct{}
-
 func initProcessingHandler() {
 	processingSem = make(chan struct{}, config.Concurrency)
 
@@ -79,7 +77,7 @@ func setVary(rw http.ResponseWriter) {
 	}
 }
 
-func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, resultData *imagedata.ImageData, po *options.ProcessingOptions, originURL string, originData *imagedata.ImageData) {
+func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, statusCode int, resultData *imagedata.ImageData, po *options.ProcessingOptions, originURL string, originData *imagedata.ImageData) {
 	var contentDisposition string
 	if len(po.Filename) > 0 {
 		contentDisposition = resultData.Type.ContentDisposition(po.Filename)
@@ -111,10 +109,6 @@ func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, res
 	}
 
 	rw.Header().Set("Content-Length", strconv.Itoa(len(resultData.Data)))
-	statusCode := 200
-	if getFallbackImageUsed(r.Context()) {
-		statusCode = config.FallbackImageHTTPCode
-	}
 	rw.WriteHeader(statusCode)
 	rw.Write(resultData.Data)
 
@@ -215,6 +209,8 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 	}
 	defer func() { <-processingSem }()
 
+	statusCode := http.StatusOK
+
 	originData, err := func() (*imagedata.ImageData, error) {
 		defer metrics.StartDownloadingSegment(ctx)()
 		return imagedata.Download(imageURL, "source image", imgRequestHeader)
@@ -227,7 +223,11 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 		respondWithNotModified(reqID, r, rw, po, imageURL, nmErr.Headers)
 		return
 	} else {
-		if ierr, ok := err.(*ierrors.Error); !ok || ierr.Unexpected {
+		ierr, ierrok := err.(*ierrors.Error)
+		if ierrok {
+			statusCode = ierr.StatusCode
+		}
+		if !ierrok || ierr.Unexpected {
 			errorreport.Report(err, r)
 		}
 
@@ -238,13 +238,15 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 		}
 
 		log.Warningf("Could not load image %s. Using fallback image. %s", imageURL, err.Error())
-		r = r.WithContext(setFallbackImageUsedCtx(r.Context()))
+		if config.FallbackImageHTTPCode > 0 {
+			statusCode = config.FallbackImageHTTPCode
+		}
 		originData = imagedata.FallbackImage
 	}
 
 	router.CheckTimeout(ctx)
 
-	if config.ETagEnabled && !getFallbackImageUsed(ctx) {
+	if config.ETagEnabled && statusCode == http.StatusOK {
 		imgDataMatch := etagHandler.SetActualImageData(originData)
 
 		rw.Header().Set("ETag", etagHandler.GenerateActualETag())
@@ -260,14 +262,14 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 	if originData.Type == po.Format || po.Format == imagetype.Unknown {
 		// Don't process SVG
 		if originData.Type == imagetype.SVG {
-			respondWithImage(reqID, r, rw, originData, po, imageURL, originData)
+			respondWithImage(reqID, r, rw, statusCode, originData, po, imageURL, originData)
 			return
 		}
 
 		if len(po.SkipProcessingFormats) > 0 {
 			for _, f := range po.SkipProcessingFormats {
 				if f == originData.Type {
-					respondWithImage(reqID, r, rw, originData, po, imageURL, originData)
+					respondWithImage(reqID, r, rw, statusCode, originData, po, imageURL, originData)
 					return
 				}
 			}
@@ -299,14 +301,5 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 
 	router.CheckTimeout(ctx)
 
-	respondWithImage(reqID, r, rw, resultData, po, imageURL, originData)
-}
-
-func setFallbackImageUsedCtx(ctx context.Context) context.Context {
-	return context.WithValue(ctx, fallbackImageUsedCtxKey{}, true)
-}
-
-func getFallbackImageUsed(ctx context.Context) bool {
-	result, _ := ctx.Value(fallbackImageUsedCtxKey{}).(bool)
-	return result
+	respondWithImage(reqID, r, rw, statusCode, resultData, po, imageURL, originData)
 }