Skip to content

Commit

Permalink
Harden the parsing of the result code from curl
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dra27 committed Jun 2, 2024
1 parent c4cf981 commit fdfa01e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 8 deletions.
1 change: 1 addition & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 14 additions & 8 deletions src/repository/opamDownload.ml
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ 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",
Some (FIdent (OpamFilter.ident_of_string "compress")); (* 7.10 1-Oct-2002 *)
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;
]
Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit fdfa01e

Please sign in to comment.