[Libguestfs] [PATCH v3 3/8] builder: Use the new Curl module for passing parameters to curl.

These are now passed using a curl configuration file, which is a little bit safer than using command lines. virt-builder doesn't need to pass usernames and passwords to curl, but if it ever does in future this will be a lot safer. --- builder/Makefile.am | 1 + builder/builder.ml | 2 +- builder/downloader.ml | 80 ++++++++++++++++++++++++-------------------------- builder/downloader.mli | 10 +------ builder/index.ml | 2 +- builder/index.mli | 2 +- builder/sources.ml | 10 +++---- builder/sources.mli | 2 +- 8 files changed, 49 insertions(+), 60 deletions(-) diff --git a/builder/Makefile.am b/builder/Makefile.am index ad32940..5c41cfa 100644 --- a/builder/Makefile.am +++ b/builder/Makefile.am @@ -144,6 +144,7 @@ BOBJECTS = \ $(top_builddir)/mllib/JSON.cmo \ $(top_builddir)/mllib/URI.cmo \ $(top_builddir)/mllib/mkdtemp.cmo \ + $(top_builddir)/mllib/curl.cmo \ $(top_builddir)/customize/customize_utils.cmo \ $(top_builddir)/customize/urandom.cmo \ $(top_builddir)/customize/random_seed.cmo \ diff --git a/builder/builder.ml b/builder/builder.ml index affce10..873df54 100644 --- a/builder/builder.ml +++ b/builder/builder.ml @@ -185,7 +185,7 @@ let main () = { Sources.name = source; uri = source; gpgkey = Utils.Fingerprint fingerprint; - proxy = Downloader.SystemProxy; + proxy = Curl.SystemProxy; format = Sources.FormatNative; } ) cmdline.sources in diff --git a/builder/downloader.ml b/builder/downloader.ml index 8c47bad..3c9ba18 100644 --- a/builder/downloader.ml +++ b/builder/downloader.ml @@ -32,17 +32,12 @@ type t = { cache : Cache.t option; (* cache for templates *) } -type proxy_mode = - | UnsetProxy - | SystemProxy - | ForcedProxy of string - let create ~curl ~cache = { curl = curl; cache = cache; } -let rec download t ?template ?progress_bar ?(proxy = SystemProxy) uri = +let rec download t ?template ?progress_bar ?(proxy = Curl.SystemProxy) uri = match template with | None -> (* no cache, simple download *) (* Create a temporary name. *) @@ -83,6 +78,7 @@ and download_to t ?(progress_bar = false) ~proxy uri filename = unlink_on_exit filename_new; (match parseduri.URI.protocol with + (* Download (ie. copy) from a local file. *) | "file" -> let path = parseduri.URI.path in let cmd = [ "cp" ] @ @@ -91,15 +87,27 @@ and download_to t ?(progress_bar = false) ~proxy uri filename = let r = run_command cmd in if r <> 0 then error (f_"cp (download) command failed copying '%s'") path; - | _ as protocol -> (* Any other protocol. *) - let outenv = proxy_envvar protocol proxy in + + (* Any other protocol. *) + | _ -> + let common_args = [ "location", None ] (* Follow 3XX redirects. *) in + let quiet_args = [ "silent", None; "show-error", None ] in + (* Get the status code first to ensure the file exists. *) - let cmd = sprintf "%s%s%s -L --max-redirs 5 -g -o /dev/null -I -w '%%{http_code}' %s" - outenv - t.curl - (if verbose () then "" else " -s -S") - (quote uri) in - let lines = external_command cmd in + let curl_h = + let curl_args = + common_args @ + (if verbose () then [] else quiet_args) @ [ + "output", Some "/dev/null"; (* Write output to /dev/null. *) + "head", None; (* Request only HEAD. *) + (* Write HTTP status code to stdout. *) + "write-out", Some "%{http_code}"; + "url", Some uri + ] in + + Curl.create ~curl:t.curl curl_args in + + let lines = Curl.run curl_h in if List.length lines < 1 then error (f_"unexpected output from curl command, enable debug and look at previous messages"); let status_code = List.hd lines in @@ -113,35 +121,23 @@ and download_to t ?(progress_bar = false) ~proxy uri filename = error (f_"failed to download %s: HTTP status code %s") uri status_code; (* Now download the file. *) - let cmd = sprintf "%s%s%s -L --max-redirs 5 -g -o %s %s" - outenv - t.curl - (if verbose () then "" else if progress_bar then " -#" else " -s -S") - (quote filename_new) (quote uri) in - let r = shell_command cmd in - if r <> 0 then - error (f_"curl (download) command failed downloading '%s'") uri; + let curl_h = + let curl_args = + common_args @ [ + "output", Some filename_new; + "url", Some uri + ] in + + let curl_args = + curl_args @ + if verbose () then [] + else if progress_bar then [ "progress-bar", None ] + else quiet_args in + + Curl.create ~curl:t.curl curl_args in + + ignore (Curl.run curl_h) ); (* Rename the file if the download was successful. *) rename filename_new filename - -and proxy_envvar protocol = function - | UnsetProxy -> - (match protocol with - | "http" -> "env http_proxy= no_proxy=* " - | "https" -> "env https_proxy= no_proxy=* " - | "ftp" -> "env ftp_proxy= no_proxy=* " - | _ -> "env no_proxy=* " - ) - | SystemProxy -> - (* No changes required. *) - "" - | ForcedProxy proxy -> - let proxy = quote proxy in - (match protocol with - | "http" -> sprintf "env http_proxy=%s no_proxy= " proxy - | "https" -> sprintf "env https_proxy=%s no_proxy= " proxy - | "ftp" -> sprintf "env ftp_proxy=%s no_proxy= " proxy - | _ -> "" - ) diff --git a/builder/downloader.mli b/builder/downloader.mli index 11ec498..c99aee2 100644 --- a/builder/downloader.mli +++ b/builder/downloader.mli @@ -24,18 +24,10 @@ type filename = string type t (** The abstract data type. *) -(** Type of proxy. *) -type proxy_mode = - | UnsetProxy (* The proxy is forced off. *) - | SystemProxy (* The proxy is not changed (follows the - * system configuration). - *) - | ForcedProxy of string (* The proxy is forced to the specified URL. *) - val create : curl:string -> cache:Cache.t option -> t (** Create the abstract type. *) -val download : t -> ?template:(string*string*Utils.revision) -> ?progress_bar:bool -> ?proxy:proxy_mode -> uri -> (filename * bool) +val download : t -> ?template:(string*string*Utils.revision) -> ?progress_bar:bool -> ?proxy:Curl.proxy -> uri -> (filename * bool) (** Download the URI, returning the downloaded filename and a temporary file flag. The temporary file flag is [true] iff the downloaded file is temporary and should be deleted by the diff --git a/builder/index.ml b/builder/index.ml index 01b4a5f..23b9820 100644 --- a/builder/index.ml +++ b/builder/index.ml @@ -43,7 +43,7 @@ and entry = { aliases : string list option; sigchecker : Sigchecker.t; - proxy : Downloader.proxy_mode; + proxy : Curl.proxy; } let print_entry chan (name, { printable_name = printable_name; diff --git a/builder/index.mli b/builder/index.mli index 07cfb9d..09b277e 100644 --- a/builder/index.mli +++ b/builder/index.mli @@ -35,7 +35,7 @@ and entry = { aliases : string list option; sigchecker : Sigchecker.t; - proxy : Downloader.proxy_mode; + proxy : Curl.proxy; } val print_entry : out_channel -> (string * entry) -> unit diff --git a/builder/sources.ml b/builder/sources.ml index 4c8d6c7..db1a969 100644 --- a/builder/sources.ml +++ b/builder/sources.ml @@ -26,7 +26,7 @@ type source = { name : string; uri : string; gpgkey : Utils.gpgkey_type; - proxy : Downloader.proxy_mode; + proxy : Curl.proxy; format : source_format; } and source_format = @@ -67,12 +67,12 @@ let parse_conf file = let proxy = try (match (List.assoc ("proxy", None) fields) with - | "no" | "off" -> Downloader.UnsetProxy - | "system" -> Downloader.SystemProxy - | _ as proxy -> Downloader.ForcedProxy proxy + | "no" | "off" -> Curl.UnsetProxy + | "system" -> Curl.SystemProxy + | _ as proxy -> Curl.ForcedProxy proxy ) with - Not_found -> Downloader.SystemProxy in + Not_found -> Curl.SystemProxy in let format = try (match (List.assoc ("format", None) fields) with diff --git a/builder/sources.mli b/builder/sources.mli index e621a9f..6594d00 100644 --- a/builder/sources.mli +++ b/builder/sources.mli @@ -20,7 +20,7 @@ type source = { name : string; uri : string; gpgkey : Utils.gpgkey_type; - proxy : Downloader.proxy_mode; + proxy : Curl.proxy; format : source_format; } and source_format = -- 2.7.4