From d0505d6bac30267296295e47268006893f092ae4 Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Fri, 23 Jun 2023 16:48:57 +0200
Subject: [PATCH 1/2] pkg/api: top return error to client

Wait before sending status code 200 for the first top call and if that
fails return a proper error code.

This was leading to some confusion in [1] because podman just reported
200 but did not wirte anything back.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2215572

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 pkg/api/handlers/compat/containers_top.go | 20 +++++++++++++-------
 test/apiv2/20-containers.at               |  9 +++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/pkg/api/handlers/compat/containers_top.go b/pkg/api/handlers/compat/containers_top.go
index 69e4160430..8da18ae446 100644
--- a/pkg/api/handlers/compat/containers_top.go
+++ b/pkg/api/handlers/compat/containers_top.go
@@ -48,12 +48,8 @@ func TopContainer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// We are committed now - all errors logged but not reported to client, ship has sailed
-	w.WriteHeader(http.StatusOK)
+	statusWritten := false
 	w.Header().Set("Content-Type", "application/json")
-	if f, ok := w.(http.Flusher); ok {
-		f.Flush()
-	}
 
 	encoder := json.NewEncoder(w)
 
@@ -65,7 +61,11 @@ loop: // break out of for/select infinite` loop
 		default:
 			output, err := c.Top(strings.Split(query.PsArgs, ","))
 			if err != nil {
-				logrus.Infof("Error from %s %q : %v", r.Method, r.URL, err)
+				if !statusWritten {
+					utils.InternalServerError(w, err)
+				} else {
+					logrus.Errorf("From %s %q : %v", r.Method, r.URL, err)
+				}
 				break loop
 			}
 
@@ -86,9 +86,15 @@ loop: // break out of for/select infinite` loop
 				}
 
 				if err := encoder.Encode(body); err != nil {
-					logrus.Infof("Error from %s %q : %v", r.Method, r.URL, err)
+					if !statusWritten {
+						utils.InternalServerError(w, err)
+					} else {
+						logrus.Errorf("From %s %q : %v", r.Method, r.URL, err)
+					}
 					break loop
 				}
+				// after the first write we can no longer send a different status code
+				statusWritten = true
 				if f, ok := w.(http.Flusher); ok {
 					f.Flush()
 				}
diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at
index 3a38976e73..af22997247 100644
--- a/test/apiv2/20-containers.at
+++ b/test/apiv2/20-containers.at
@@ -132,6 +132,15 @@ if root; then
     podman rm -f $CTRNAME
 fi
 
+CTRNAME=test123
+podman run --name $CTRNAME -d $IMAGE top
+t GET libpod/containers/$CTRNAME/top?ps_args=--invalid 500 \
+  .cause~".*unrecognized option.*"
+t GET containers/$CTRNAME/top?ps_args=--invalid 500 \
+  .cause~".*unrecognized option.*"
+
+podman rm -f $CTRNAME
+
 # Issue #15765: make sure the memory limit is capped
 if root; then
     CTRNAME=ctr-with-limit

From 0ee19f08cfd75df8f1080dc67a362e8d476c9578 Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Fri, 23 Jun 2023 16:51:22 +0200
Subject: [PATCH 2/2] pkg/api: BufferedResponseWriter flush correctly

Somehow my error message in top was never printed for the compat API,
the libpod one using the same code worked fine. Turns out the compat one
is using this buffered writter instaed but never made sure to flush it
before closing the connection.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 pkg/api/server/handler_api.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pkg/api/server/handler_api.go b/pkg/api/server/handler_api.go
index 57cafa5f6f..316cad5df0 100644
--- a/pkg/api/server/handler_api.go
+++ b/pkg/api/server/handler_api.go
@@ -56,7 +56,9 @@ func (s *APIServer) apiWrapper(h http.HandlerFunc, w http.ResponseWriter, r *htt
 	}
 
 	if buffer {
-		w = newBufferedResponseWriter(w)
+		bw := newBufferedResponseWriter(w)
+		defer bw.b.Flush()
+		w = bw
 	}
 
 	h(w, r)