Re: [Xen-devel] [PATCH OSSTEST v7 01/15] TestSupport: Add helper to fetch a URL on a host

2015-07-06 Thread Ian Jackson 
Ian Campbell writes (Re: [PATCH OSSTEST v7 01/15] TestSupport: Add helper to 
fetch a URL on a host):
 On Mon, 2015-07-06 at 15:48 +0100, Ian Jackson wrote:
  Ian Campbell writes ([PATCH OSSTEST v7 01/15] TestSupport: Add helper to 
  fetch a URL on a host):
   +$useproxy wget --progress=dot:mega -O \$path\ \$url\
...
  \Q may be of some help.
 
 Really? I thought that escaped things at the Perl level, is that
 sufficient for the shell stuff within the Perl too?

If you wrote

  +target_cmd_root($ho, END, $timeo);
  +$useproxy wget --progress=dot:mega -O \Q$path\E \Q$url\E

and $url contained

  http://chars $'\ considered harmful.iso

then target_cmd_root would get

  http_proxy=something wget --progress=dot:mega -O \/some\/path \
 http\:\/\/chars\ \$\\'\\\ considered\ harmful\.iso

And:

  mariner:~ echo http\:\/\/chars\ \$\\'\\\ considered\ harmful\.iso
  http://chars $'\ considered harmful.iso
  mariner:~

But maybe this is too ugly, in which case you could use ''
and some regexp like s/['\\]/'\\$'/g (not tested).

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH OSSTEST v7 01/15] TestSupport: Add helper to fetch a URL on a host

2015-07-06 Thread Ian Campbell 
On Mon, 2015-07-06 at 16:59 +0100, Ian Jackson wrote:
 Ian Campbell writes (Re: [PATCH OSSTEST v7 01/15] TestSupport: Add helper to 
 fetch a URL on a host):
  On Mon, 2015-07-06 at 15:48 +0100, Ian Jackson wrote:
   Ian Campbell writes ([PATCH OSSTEST v7 01/15] TestSupport: Add helper to 
   fetch a URL on a host):
+$useproxy wget --progress=dot:mega -O \$path\ \$url\
 ...
   \Q may be of some help.
  
  Really? I thought that escaped things at the Perl level, is that
  sufficient for the shell stuff within the Perl too?
 
 If you wrote
 
   +target_cmd_root($ho, END, $timeo);
   +$useproxy wget --progress=dot:mega -O \Q$path\E \Q$url\E
 
 and $url contained
 
   http://chars $'\ considered harmful.iso
 
 then target_cmd_root would get
 
   http_proxy=something wget --progress=dot:mega -O \/some\/path \
  http\:\/\/chars\ \$\\'\\\ considered\ harmful\.iso

Interesting, I had assumed \Q...\E was some sort of lexer/parser level
thing, when actually it just rewrites what is between it.

And as it happens Perl and shell need a very similar set of things
quoting.

 
 And:
 
   mariner:~ echo http\:\/\/chars\ \$\\'\\\ considered\ harmful\.iso
   http://chars $'\ considered harmful.iso
   mariner:~
 
 But maybe this is too ugly,

I think it is ok actually.

  in which case you could use ''
 and some regexp like s/['\\]/'\\$'/g (not tested).
 
 Ian.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH OSSTEST v7 01/15] TestSupport: Add helper to fetch a URL on a host

2015-07-06 Thread Ian Jackson 
Ian Campbell writes ([PATCH OSSTEST v7 01/15] TestSupport: Add helper to fetch 
a URL on a host):
 Signed-off-by: Ian Campbell ian.campb...@citrix.com
 ---
 v7: Quote $url and $path, switch to a heredoc to avoid resulting over
 long line
...

Last time I wrote:

  Do we care that this will break badly if the url contains shell
  metacharacters ?  I think we may do.

but:

 +$useproxy wget --progress=dot:mega -O \$path\ \$url\

Did you try this with $path or $url containing $ or   or  or \ or ` or
starting with - ?

There are a fair few places in osstest where we're quite lax with this
kind of thing, but (hopefully) only where the information definitely
comes from the configuration (or some other trusted source).  A
general helper like this ought to be robust against that kind of input
(which may well mean failing, but it should not include potentially
executing bits of the input or misinterpreting it as command line
options to wget.

\Q may be of some help.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH OSSTEST v7 01/15] TestSupport: Add helper to fetch a URL on a host

2015-05-27 Thread Ian Campbell 
Signed-off-by: Ian Campbell ian.campb...@citrix.com
---
v7: Quote $url and $path, switch to a heredoc to avoid resulting over
long line
v5: Support http_proxy via $c{HttpProxy}
v3: Make sure wget is installed
---
 Osstest/Debian.pm  |  2 +-
 Osstest/TestSupport.pm | 12 +++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 537ccbe..673ceba 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -685,7 +685,7 @@ d-i apt-setup/another boolean false
 d-i apt-setup/non-free boolean false
 d-i apt-setup/contrib boolean false
 
-d-i pkgsel/include string openssh-server, ntp, ntpdate, ethtool, 
chiark-utils-bin, $extra_packages
+d-i pkgsel/include string openssh-server, ntp, ntpdate, ethtool, 
chiark-utils-bin, wget, $extra_packages
 
 $xopts{ExtraPreseed}
 
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index abb3195..d84ca51 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -55,7 +55,7 @@ BEGIN {
   target_putfilecontents_stash
  target_putfilecontents_root_stash
   target_put_guest_image target_editfile
-  target_editfile_cancel
+  target_editfile_cancel target_fetchurl
   target_editfile_root target_file_exists
   target_editfile_kvp_replace
   target_run_apt
@@ -1589,6 +1589,16 @@ END
 return $cfgpath;
 }
 
+sub target_fetchurl($$$;$) {
+my ($ho, $url, $path, $timeo) = @_;
+$timeo ||= 2000;
+my $useproxy = export http_proxy=$c{HttpProxy}; if $c{HttpProxy};
+target_cmd_root($ho, END, $timeo);
+$useproxy wget --progress=dot:mega -O \$path\ \$url\
+END
+}
+
+
 sub target_put_guest_image ($$;$) {
 my ($ho, $gho, $default) = @_;
 my $specimage = $r{$gho-{Guest}_image};
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel