From fdfa01e46fb5d92e52d9b713adb858423e9747cd Mon Sep 17 00:00:00 2001
From: David Allsopp <david.allsopp@metastack.com>
Date: Sun, 2 Jun 2024 09:58:57 +0100
Subject: [PATCH] Harden the parsing of the result code from curl

stderr and stdout can become interleaved, certainly on Windows, which
results in the http result code becoming lost.

Change puts a newline both before and after the code. However, the
interleaving means that a blank line may appear _after_ the result code.
Therefore, the logic now attempts to parse the first non-blank line from
the end of the output from curl.
---
 master_changes.md              |  1 +
 src/repository/opamDownload.ml | 22 ++++++++++++++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/master_changes.md b/master_changes.md
index 2684640ed0e..0f26bfb9181 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -64,6 +64,7 @@ users)
 
 ## Repository
   * Fix download URLs containing invalid characters on Windows (e.g. the ? character in `?full_index=1`) [#5921 @dra27]
+  * Harden the parsing of the output from curl - the progress meter can become interleaved with the status code on Windows [#5984 @dra27]
 
 ## Lock
 
diff --git a/src/repository/opamDownload.ml b/src/repository/opamDownload.ml
index f199c248741..c0cfcbaa2f4 100644
--- a/src/repository/opamDownload.ml
+++ b/src/repository/opamDownload.ml
@@ -22,7 +22,7 @@ let user_agent =
 
 let curl_args = [
   CString "--write-out", None;
-  CString "%%{http_code}\\n", None; (* 6.5 13-Mar-2000 *)
+  CString "\\n%%{http_code}\\n", None; (* 6.5 13-Mar-2000 *)
   CString "--retry", None; CIdent "retry", None; (* 7.12.3 20-Dec-2004 *)
   CString "--retry-delay", None; CString "2", None; (* 7.12.3 20-Dec-2004 *)
   CString "--compressed",
@@ -30,6 +30,8 @@ let curl_args = [
   CString "--user-agent", None; user_agent, None; (* 4.5.1 12-Jun-1998 *)
   CString "-L", None; (* 4.9 7-Oct-1998 *)
   CString "-o", None; CIdent "out", None; (* 2.3 21-Aug-1997 *)
+  (* Would suppress the progress bar, but a 2019 option is a bit too young *)
+  (*CString "--no-progress-meter", None;*) (* 7.67.0 6-Nov-2019 *)
   CString "--", None; (* End list of options; 5.0 1-Dec-1998 *)
   CIdent "url", None;
 ]
@@ -108,13 +110,17 @@ let tool_return url ret =
                 Printf.sprintf "curl: empty response while downloading %s"
                   (OpamUrl.to_string url))
     | l  ->
-      let code = List.hd (List.rev l) in
-      let num = try int_of_string code with Failure _ -> 999 in
-      if num >= 400 then
-        fail (Some ("curl error code " ^ code),
-                  Printf.sprintf "curl: code %s while downloading %s"
-                    code (OpamUrl.to_string url))
-      else Done ()
+      let l = List.rev l in
+      try
+        let code = List.find ((<>) "") l in
+        let num = try int_of_string code with Failure _ -> 999 in
+        if num >= 400 then
+          fail (Some ("curl error code " ^ code),
+                    Printf.sprintf "curl: code %s while downloading %s"
+                      code (OpamUrl.to_string url))
+        else Done ()
+      with Not_found ->
+        fail (Some "Curl failed", "No output from the curl command")
 
 let download_command ~compress ?checksum ~url ~dst () =
   let cmd, args =