From 63dfe842bb129221b350d66f3e04c068f7a91233 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 21 Jun 2023 10:27:00 +0200
Subject: [PATCH] compat API create/pull: fix error handling

Make sure that the create endpoint does not always return 200 even in
case of a failure.  Some of the code had to be massaged since encoding a
report implies sending a 200.

Fixes: #15828
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 go.mod                            |  2 +-
 pkg/api/handlers/compat/images.go | 29 ++++++++++++++++++++++-------
 test/apiv2/10-images.at           |  1 +
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/go.mod b/go.mod
index d895507c70..319c87d14e 100644
--- a/go.mod
+++ b/go.mod
@@ -25,6 +25,7 @@ require (
 	github.com/crc-org/vfkit v0.0.5-0.20230602131541-3d57f09010c9
 	github.com/cyphar/filepath-securejoin v0.2.3
 	github.com/digitalocean/go-qemu v0.0.0-20221209210016-f035778c97f7
+	github.com/docker/distribution v2.8.2+incompatible
 	github.com/docker/docker v24.0.2+incompatible
 	github.com/docker/go-connections v0.4.1-0.20210727194412-58542c764a11
 	github.com/docker/go-plugins-helpers v0.0.0-20211224144127-6eecb7beb651
@@ -90,7 +91,6 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/digitalocean/go-libvirt v0.0.0-20220804181439-8648fbde413e // indirect
 	github.com/disiqueira/gotree/v3 v3.0.2 // indirect
-	github.com/docker/distribution v2.8.2+incompatible // indirect
 	github.com/docker/docker-credential-helpers v0.7.0 // indirect
 	github.com/felixge/httpsnoop v1.0.3 // indirect
 	github.com/fsouza/go-dockerclient v1.9.7 // indirect
diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go
index e3e97ce89b..c3f4a0373f 100644
--- a/pkg/api/handlers/compat/images.go
+++ b/pkg/api/handlers/compat/images.go
@@ -23,6 +23,7 @@ import (
 	"github.com/containers/podman/v4/pkg/domain/infra/abi"
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/storage"
+	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/gorilla/schema"
 	"github.com/opencontainers/go-digest"
@@ -313,18 +314,24 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) {
 		pullResChan <- pullResult{images: pulledImages, err: err}
 	}()
 
+	enc := json.NewEncoder(w)
+	enc.SetEscapeHTML(true)
+
 	flush := func() {
 		if flusher, ok := w.(http.Flusher); ok {
 			flusher.Flush()
 		}
 	}
 
-	w.WriteHeader(http.StatusOK)
-	w.Header().Set("Content-Type", "application/json")
-	flush()
-
-	enc := json.NewEncoder(w)
-	enc.SetEscapeHTML(true)
+	statusWritten := false
+	writeStatusCode := func(code int) {
+		if !statusWritten {
+			w.WriteHeader(code)
+			w.Header().Set("Content-Type", "application/json")
+			flush()
+			statusWritten = true
+		}
+	}
 
 loop: // break out of for/select infinite loop
 	for {
@@ -332,6 +339,7 @@ loop: // break out of for/select infinite loop
 		report.Progress = &jsonmessage.JSONProgress{}
 		select {
 		case e := <-progress:
+			writeStatusCode(http.StatusOK)
 			switch e.Event {
 			case types.ProgressEventNewArtifact:
 				report.Status = "Pulling fs layer"
@@ -352,14 +360,20 @@ loop: // break out of for/select infinite loop
 			flush()
 		case pullRes := <-pullResChan:
 			err := pullRes.err
-			pulledImages := pullRes.images
 			if err != nil {
+				var errcd errcode.ErrorCoder
+				if errors.As(err, &errcd) {
+					writeStatusCode(errcd.ErrorCode().Descriptor().HTTPStatusCode)
+				} else {
+					writeStatusCode(http.StatusInternalServerError)
+				}
 				msg := err.Error()
 				report.Error = &jsonmessage.JSONError{
 					Message: msg,
 				}
 				report.ErrorMessage = msg
 			} else {
+				pulledImages := pullRes.images
 				if len(pulledImages) > 0 {
 					img := pulledImages[0].ID()
 					if utils.IsLibpodRequest(r) {
@@ -374,6 +388,7 @@ loop: // break out of for/select infinite loop
 						Message: msg,
 					}
 					report.ErrorMessage = msg
+					writeStatusCode(http.StatusInternalServerError)
 				}
 			}
 			if err := enc.Encode(report); err != nil {
diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at
index 9eb6de317f..94a8bee480 100644
--- a/test/apiv2/10-images.at
+++ b/test/apiv2/10-images.at
@@ -72,6 +72,7 @@ t POST "images/create?fromSrc=-&repo=myimage&tag=mytag" - 200
 t GET "images/myimage:mytag/json" 200 \
   .Id~'^sha256:[0-9a-f]\{64\}$' \
   .RepoTags[0]="docker.io/library/myimage:mytag"
+t POST /images/create?fromImage=busybox:invalidtag123 404
 
 # Display the image history
 t GET libpod/images/nonesuch/history 404