🐛 Fix download-image to properly handle network errors and non-2xx responses (#8554)

The download-image function in app.media silently succeeded when the
remote image URL was unreachable or returned an error status code,
causing create-file-media-object-from-url to report success with no
actual image stored.

Add exception handling for connection refused, timeouts, and I/O errors
around the HTTP request, and validate the HTTP status code in
parse-and-validate before processing the response body.

Fixes #8499

Signed-off-by: Andrey Antukh <niwi@niwi.nz>

backend/src/app/media.clj

@@ -293,12 +293,17 @@
 (defn download-image
   "Download an image from the provided URI and return the media input object"
   [{:keys [::http/client]} uri]
-  (letfn [(parse-and-validate [{:keys [headers] :as response}]
+  (letfn [(parse-and-validate [{:keys [status headers] :as response}]
             (let [size     (some-> (get headers "content-length") d/parse-integer)
                   mtype    (get headers "content-type")
                   format   (cm/mtype->format mtype)
                   max-size (cf/get :media-max-file-size default-max-file-size)]
 
+              (when-not (<= 200 status 299)
+                (ex/raise :type :validation
+                          :code :unable-to-download-image
+                          :hint (str/ffmt "unable to download image from '%': unexpected status code %" uri status)))
+
               (when-not size
                 (ex/raise :type :validation
                           :code :unknown-size
@@ -318,9 +323,32 @@
 
               {:size size :mtype mtype :format format}))]
 
-    (let [{:keys [body] :as response} (http/req! client
+    (let [{:keys [body] :as response}
-                                                 {:method :get :uri uri}
+          (try
-                                                 {:response-type :input-stream})
+            (http/req! client
+                       {:method :get :uri uri}
+                       {:response-type :input-stream})
+            (catch java.net.ConnectException cause
+              (ex/raise :type :validation
+                        :code :unable-to-download-image
+                        :hint (str/ffmt "unable to download image from '%': connection refused or host unreachable" uri)
+                        :cause cause))
+            (catch java.net.http.HttpConnectTimeoutException cause
+              (ex/raise :type :validation
+                        :code :unable-to-download-image
+                        :hint (str/ffmt "unable to download image from '%': connection timeout" uri)
+                        :cause cause))
+            (catch java.net.http.HttpTimeoutException cause
+              (ex/raise :type :validation
+                        :code :unable-to-download-image
+                        :hint (str/ffmt "unable to download image from '%': request timeout" uri)
+                        :cause cause))
+            (catch java.io.IOException cause
+              (ex/raise :type :validation
+                        :code :unable-to-download-image
+                        :hint (str/ffmt "unable to download image from '%': I/O error" uri)
+                        :cause cause)))
+
           {:keys [size mtype]} (parse-and-validate response)
           path    (tmp/tempfile :prefix "penpot.media.download.")
           written (io/write* path body :size size)]

backend/test/backend_tests/rpc_media_test.clj

@@ -9,11 +9,14 @@
    [app.common.time :as ct]
    [app.common.uuid :as uuid]
    [app.db :as db]
+   [app.http.client :as http]
+   [app.media :as media]
    [app.rpc :as-alias rpc]
    [app.storage :as sto]
    [backend-tests.helpers :as th]
    [clojure.test :as t]
-   [datoteka.fs :as fs]))
+   [datoteka.fs :as fs]
+   [mockery.core :refer [with-mocks]]))
 
 (t/use-fixtures :once th/state-init)
 (t/use-fixtures :each th/database-reset)
@@ -278,3 +281,100 @@
           error-data (ex-data error)]
       (t/is (th/ex-info? error))
       (t/is (= (:type error-data) :not-found)))))
+
+
+(t/deftest download-image-connection-error
+  (t/testing "connection refused raises validation error"
+    (with-mocks [http-mock {:target 'app.http.client/req!
+                            :throw (java.net.ConnectException. "Connection refused")}]
+      (let [cfg {::http/client :mock-client}
+            err (try
+                  (media/download-image cfg "http://unreachable.invalid/image.png")
+                  nil
+                  (catch clojure.lang.ExceptionInfo e e))]
+        (t/is (some? err))
+        (t/is (= :validation (:type (ex-data err))))
+        (t/is (= :unable-to-download-image (:code (ex-data err)))))))
+
+  (t/testing "connection timeout raises validation error"
+    (with-mocks [http-mock {:target 'app.http.client/req!
+                            :throw (java.net.http.HttpConnectTimeoutException. "Connect timed out")}]
+      (let [cfg {::http/client :mock-client}
+            err (try
+                  (media/download-image cfg "http://unreachable.invalid/image.png")
+                  nil
+                  (catch clojure.lang.ExceptionInfo e e))]
+        (t/is (some? err))
+        (t/is (= :validation (:type (ex-data err))))
+        (t/is (= :unable-to-download-image (:code (ex-data err)))))))
+
+  (t/testing "request timeout raises validation error"
+    (with-mocks [http-mock {:target 'app.http.client/req!
+                            :throw (java.net.http.HttpTimeoutException. "Request timed out")}]
+      (let [cfg {::http/client :mock-client}
+            err (try
+                  (media/download-image cfg "http://unreachable.invalid/image.png")
+                  nil
+                  (catch clojure.lang.ExceptionInfo e e))]
+        (t/is (some? err))
+        (t/is (= :validation (:type (ex-data err))))
+        (t/is (= :unable-to-download-image (:code (ex-data err)))))))
+
+  (t/testing "I/O error raises validation error"
+    (with-mocks [http-mock {:target 'app.http.client/req!
+                            :throw (java.io.IOException. "Stream closed")}]
+      (let [cfg {::http/client :mock-client}
+            err (try
+                  (media/download-image cfg "http://unreachable.invalid/image.png")
+                  nil
+                  (catch clojure.lang.ExceptionInfo e e))]
+        (t/is (some? err))
+        (t/is (= :validation (:type (ex-data err))))
+        (t/is (= :unable-to-download-image (:code (ex-data err))))))))
+
+
+(t/deftest download-image-status-code-error
+  (t/testing "404 status raises validation error"
+    (with-mocks [http-mock {:target 'app.http.client/req!
+                            :return {:status 404
+                                     :headers {"content-type" "text/html"
+                                               "content-length" "0"}
+                                     :body nil}}]
+      (let [cfg {::http/client :mock-client}
+            err (try
+                  (media/download-image cfg "http://example.com/not-found.png")
+                  nil
+                  (catch clojure.lang.ExceptionInfo e e))]
+        (t/is (some? err))
+        (t/is (= :validation (:type (ex-data err))))
+        (t/is (= :unable-to-download-image (:code (ex-data err)))))))
+
+  (t/testing "500 status raises validation error"
+    (with-mocks [http-mock {:target 'app.http.client/req!
+                            :return {:status 500
+                                     :headers {"content-type" "text/html"
+                                               "content-length" "0"}
+                                     :body nil}}]
+      (let [cfg {::http/client :mock-client}
+            err (try
+                  (media/download-image cfg "http://example.com/server-error.png")
+                  nil
+                  (catch clojure.lang.ExceptionInfo e e))]
+        (t/is (some? err))
+        (t/is (= :validation (:type (ex-data err))))
+        (t/is (= :unable-to-download-image (:code (ex-data err)))))))
+
+  (t/testing "302 status raises validation error"
+    (with-mocks [http-mock {:target 'app.http.client/req!
+                            :return {:status 302
+                                     :headers {"content-type" "text/html"
+                                               "content-length" "0"}
+                                     :body nil}}]
+      (let [cfg {::http/client :mock-client}
+            err (try
+                  (media/download-image cfg "http://example.com/redirect.png")
+                  nil
+                  (catch clojure.lang.ExceptionInfo e e))]
+        (t/is (some? err))
+        (t/is (= :validation (:type (ex-data err))))
+        (t/is (= :unable-to-download-image (:code (ex-data err))))))))