[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libguestfs] [PATCH 3/3] 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              |  6 +--
 builder/downloader.ml           | 94 +++++++++++++++++++++--------------------
 builder/downloader.mli          | 10 +----
 builder/index.ml                |  2 +-
 builder/index.mli               |  2 +-
 builder/index_parser.ml         |  2 +-
 builder/simplestreams_parser.ml |  2 +-
 builder/sources.ml              | 10 ++---
 builder/sources.mli             |  2 +-
 10 files changed, 63 insertions(+), 68 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..e95fcd1 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 = None;
         format = Sources.FormatNative;
       }
   ) cmdline.sources in
@@ -249,7 +249,7 @@ let main () =
             message (f_"Downloading: %s") file_uri;
             let progress_bar = not (quiet ()) in
             ignore (Downloader.download downloader ~template ~progress_bar
-                      ~proxy file_uri)
+                      ?proxy file_uri)
         ) index;
         exit 0
       );
@@ -297,7 +297,7 @@ let main () =
       let template = arg, cmdline.arch, revision in
       message (f_"Downloading: %s") file_uri;
       let progress_bar = not (quiet ()) in
-      Downloader.download downloader ~template ~progress_bar ~proxy
+      Downloader.download downloader ~template ~progress_bar ?proxy
         file_uri in
     if delete_on_exit then unlink_on_exit template;
     template in
diff --git a/builder/downloader.ml b/builder/downloader.ml
index 8c47bad..de9b404 100644
--- a/builder/downloader.ml
+++ b/builder/downloader.ml
@@ -32,29 +32,24 @@ 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 uri =
   match template with
   | None ->                       (* no cache, simple download *)
     (* Create a temporary name. *)
     let tmpfile = Filename.temp_file "vbcache" ".txt" in
-    download_to t ?progress_bar ~proxy uri tmpfile;
+    download_to t ?progress_bar ?proxy uri tmpfile;
     (tmpfile, true)
 
   | Some (name, arch, revision) ->
     match t.cache with
     | None ->
       (* Not using the cache at all? *)
-      download t ?progress_bar ~proxy uri
+      download t ?progress_bar ?proxy uri
 
     | Some cache ->
       let filename = Cache.cache_of_name cache name arch revision in
@@ -63,11 +58,11 @@ let rec download t ?template ?progress_bar ?(proxy = SystemProxy) uri =
        * If not, download it.
        *)
       if not (Sys.file_exists filename) then
-        download_to t ?progress_bar ~proxy uri filename;
+        download_to t ?progress_bar ?proxy uri filename;
 
       (filename, false)
 
-and download_to t ?(progress_bar = false) ~proxy uri filename =
+and download_to t ?(progress_bar = false) ?proxy uri filename =
   let parseduri =
     try URI.parse_uri uri
     with Invalid_argument "URI.parse_uri" ->
@@ -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,33 @@ 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 =
+      Curl.safe_args @
+      [ "location", None ] @ (* Follow 3XX redirects. *)
+      match proxy with
+      | None (* system proxy settings *) -> []
+      | Some proxy -> Curl.args_of_proxy proxy 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 +127,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..fa3c34a 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 option;
 }
 
 let print_entry chan (name, { printable_name = printable_name;
diff --git a/builder/index.mli b/builder/index.mli
index 07cfb9d..8a93ba8 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 option;
 }
 
 val print_entry : out_channel -> (string * entry) -> unit
diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index d232a3a..4ac66c0 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -32,7 +32,7 @@ let get_index ~downloader ~sigchecker
 
   let rec get_index () =
     (* Get the index page. *)
-    let tmpfile, delete_tmpfile = Downloader.download downloader ~proxy uri in
+    let tmpfile, delete_tmpfile = Downloader.download downloader ?proxy uri in
 
     (* Check index file signature (also verifies it was fully
      * downloaded and not corrupted in transit).
diff --git a/builder/simplestreams_parser.ml b/builder/simplestreams_parser.ml
index 13e0b5d..808b121 100644
--- a/builder/simplestreams_parser.ml
+++ b/builder/simplestreams_parser.ml
@@ -86,7 +86,7 @@ let get_index ~downloader ~sigchecker
   let uri = ensure_trailing_slash uri in
 
   let download_and_parse uri =
-    let tmpfile, delete_tmpfile = Downloader.download downloader ~proxy uri in
+    let tmpfile, delete_tmpfile = Downloader.download downloader ?proxy uri in
     if delete_tmpfile then
       unlink_on_exit tmpfile;
     let file =
diff --git a/builder/sources.ml b/builder/sources.ml
index 4c8d6c7..9255702 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 option;
   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" -> Some Curl.UnsetProxy
+            | "system" -> None
+            | _ as proxy -> Some (Curl.ForcedProxy proxy)
             )
           with
-            Not_found -> Downloader.SystemProxy in
+            Not_found -> None in
         let format =
           try
             (match (List.assoc ("format", None) fields) with
diff --git a/builder/sources.mli b/builder/sources.mli
index e621a9f..d7f822e 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 option;
   format : source_format;
 }
 and source_format =
-- 
2.7.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]