[v1] virt-xml-validate: Allow input to be read from stdin

[libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

Posted by Johannes Holmberg 5 years, 6 months ago

---

Hello,

This is an updated version of a patch I submitted on 2015-06-10. I got
some feedback on it but then moved on to a different project and
forgot about it. Anyway, I've updated the patch according to the
feedback so if you are still interested, here it is! :)

 /Johannes

 tools/virt-xml-validate.in | 44 ++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
index 64aeaaaa33..2d2afb74ec 100644
--- a/tools/virt-xml-validate.in
+++ b/tools/virt-xml-validate.in
@@ -16,6 +16,16 @@
 
 set -e
 
+TMPFILE=
+
+cleanup() {
+  if [ $TMPFILE ]; then
+    rm -f $TMPFILE
+  fi
+}
+
+trap cleanup EXIT
+
 case $1 in
   -h | --h | --he | --hel | --help)
     cat <<EOF
@@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@
 EOF
     exit ;;
   --) shift ;;
-  -*)
+  -?*)
     echo "$0: unrecognized option '$1'" >&2
     exit 1 ;;
 esac
@@ -42,18 +52,27 @@ esac
 XMLFILE="$1"
 TYPE="$2"
 
-if [ -z "$XMLFILE" ]; then
-  echo "syntax: $0 XMLFILE [TYPE]" >&2
-  exit 1
-fi
+if [ "$XMLFILE" = "-" ]; then
+    TMPFILE=`mktemp --tmpdir virt-xml.XXXX`
+    cat > $TMPFILE
+else
+  if [ -z "$XMLFILE" ]; then
+    echo "syntax: $0 XMLFILE [TYPE]" >&2
+    exit 1
+  fi
 
-if [ ! -f "$XMLFILE" ]; then
-  echo "$0: document $XMLFILE does not exist" >&2
-  exit 2
+  if [ ! -f "$XMLFILE" ]; then
+    echo "$0: document $XMLFILE does not exist" >&2
+    exit 2
+  fi
 fi
 
 if [ -z "$TYPE" ]; then
-  ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'`
+  if [ $TMPFILE ]; then
+    ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'`
+  else
+    ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'`
+  fi
   case "$ROOT" in
      *domainsnapshot*) # Must come first, since *domain* is a substring
         TYPE="domainsnapshot"
@@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then
   exit 4
 fi
 
-xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
-
+if [ $TMPFILE ]; then
+  xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE"
+else
+  xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
+fi
 exit
-- 
2.17.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

Posted by Martin Kletzander 5 years, 6 months ago

On Mon, May 20, 2019 at 11:57:13AM +0000, Johannes Holmberg wrote:
>---
>
>Hello,
>
>This is an updated version of a patch I submitted on 2015-06-10. I got
>some feedback on it but then moved on to a different project and
>forgot about it. Anyway, I've updated the patch according to the
>feedback so if you are still interested, here it is! :)
>

Hi, thanks for the patch.  Wouldn't it be easier and cleaner if we just did:

if [ "$XMLFILE" = "-" ]; then
  XMLFILE=/dev/stdin
fi

??
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

Posted by Johannes Holmberg 5 years, 6 months ago

On Mon, 2019-05-20 at 14:18 +0200, Martin Kletzander wrote:
> On Mon, May 20, 2019 at 11:57:13AM +0000, Johannes Holmberg wrote:
> > ---
> > 
> > Hello,
> > 
> > This is an updated version of a patch I submitted on 2015-06-10. I
> > got
> > some feedback on it but then moved on to a different project and
> > forgot about it. Anyway, I've updated the patch according to the
> > feedback so if you are still interested, here it is! :)
> > 
> 
> Hi, thanks for the patch.  Wouldn't it be easier and cleaner if we
> just did:
> 
> if [ "$XMLFILE" = "-" ]; then
>   XMLFILE=/dev/stdin
> fi
> 
> ??

I tried it but the problem is that if the SCHEMA-NAME argument is not
supplied, the validation needs two passes, one to read the root tag,
and another one to do the actual xml validation. If I use /dev/stdin
the second pass will (sometimes) fail because the input is already
exhausted.

It works if the input is redirected from a file:

$ ./virt-xml-validate - < asdf.xml 
/dev/stdin validates

But if I pipe it from another program it fails:

$ cat asdf.xml | ./virt-xml-validate -
/dev/stdin:1: parser error : Document is empty

 /Johannes

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

Posted by Philipp Hahn 5 years, 6 months ago

Hello,

Some nits:

Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
> diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
> index 64aeaaaa33..2d2afb74ec 100644
> --- a/tools/virt-xml-validate.in
> +++ b/tools/virt-xml-validate.in
> @@ -16,6 +16,16 @@
>  
>  set -e
>  
> +TMPFILE=
> +
> +cleanup() {
> +  if [ $TMPFILE ]; then

Missing Quoting.
Better also give "-n"

> +    rm -f $TMPFILE

Quoting

> +  fi
> +}
> +
> +trap cleanup EXIT
> +
>  case $1 in

Not your fault, but also missing quoting.

>    -h | --h | --he | --hel | --help)
>      cat <<EOF
> @@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@
>  EOF
>      exit ;;
>    --) shift ;;
> -  -*)
> +  -?*)
>      echo "$0: unrecognized option '$1'" >&2
>      exit 1 ;;
>  esac
> @@ -42,18 +52,27 @@ esac
>  XMLFILE="$1"
>  TYPE="$2"
>  
> -if [ -z "$XMLFILE" ]; then
> -  echo "syntax: $0 XMLFILE [TYPE]" >&2
> -  exit 1
> -fi
> +if [ "$XMLFILE" = "-" ]; then
> +    TMPFILE=`mktemp --tmpdir virt-xml.XXXX`> +    cat > $TMPFILE

Quoting

> +else
> +  if [ -z "$XMLFILE" ]; then
> +    echo "syntax: $0 XMLFILE [TYPE]" >&2
> +    exit 1
> +  fi
>  
> -if [ ! -f "$XMLFILE" ]; then
> -  echo "$0: document $XMLFILE does not exist" >&2
> -  exit 2
> +  if [ ! -f "$XMLFILE" ]; then
> +    echo "$0: document $XMLFILE does not exist" >&2
> +    exit 2
> +  fi
>  fi
>  
>  if [ -z "$TYPE" ]; then
> -  ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'`
> +  if [ $TMPFILE ]; then
> +    ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'`
> +  else
> +    ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'`
> +  fi
>    case "$ROOT" in
>       *domainsnapshot*) # Must come first, since *domain* is a substring
>          TYPE="domainsnapshot"
> @@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then
>    exit 4
>  fi
>  
> -xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> -
> +if [ $TMPFILE ]; then

Quoting
"-n"

> +  xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE"
> +else
> +  xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> +fi
>  exit

Philipp

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

Posted by Johannes Holmberg 5 years, 6 months ago

On Tue, 2019-05-21 at 07:36 +0200, Philipp Hahn wrote:
> Hello,
> 
> Some nits:
> 
> Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
> > diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-
> > validate.in
> > index 64aeaaaa33..2d2afb74ec 100644
> > --- a/tools/virt-xml-validate.in
> > +++ b/tools/virt-xml-validate.in
> > @@ -16,6 +16,16 @@
> >  
> >  set -e
> >  
> > +TMPFILE=
> > +
> > +cleanup() {
> > +  if [ $TMPFILE ]; then
> 
> Missing Quoting.
> Better also give "-n"
> 
> > +    rm -f $TMPFILE
> 
> Quoting
> 
> > +  fi
> > +}
> > +
> > +trap cleanup EXIT
> > +
> >  case $1 in
> 
> Not your fault, but also missing quoting.
> 
> >    -h | --h | --he | --hel | --help)
> >      cat <<EOF
> > @@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@
> >  EOF
> >      exit ;;
> >    --) shift ;;
> > -  -*)
> > +  -?*)
> >      echo "$0: unrecognized option '$1'" >&2
> >      exit 1 ;;
> >  esac
> > @@ -42,18 +52,27 @@ esac
> >  XMLFILE="$1"
> >  TYPE="$2"
> >  
> > -if [ -z "$XMLFILE" ]; then
> > -  echo "syntax: $0 XMLFILE [TYPE]" >&2
> > -  exit 1
> > -fi
> > +if [ "$XMLFILE" = "-" ]; then
> > +    TMPFILE=`mktemp --tmpdir virt-xml.XXXX`> +    cat > $TMPFILE
> 
> Quoting
> 
> > +else
> > +  if [ -z "$XMLFILE" ]; then
> > +    echo "syntax: $0 XMLFILE [TYPE]" >&2
> > +    exit 1
> > +  fi
> >  
> > -if [ ! -f "$XMLFILE" ]; then
> > -  echo "$0: document $XMLFILE does not exist" >&2
> > -  exit 2
> > +  if [ ! -f "$XMLFILE" ]; then
> > +    echo "$0: document $XMLFILE does not exist" >&2
> > +    exit 2
> > +  fi
> >  fi
> >  
> >  if [ -z "$TYPE" ]; then
> > -  ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0
> > 1 " | awk '{ print $3 }'`
> > +  if [ $TMPFILE ]; then
> > +    ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null |
> > grep "^0 1 " | awk '{ print $3 }'`
> > +  else
> > +    ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep
> > "^0 1 " | awk '{ print $3 }'`
> > +  fi
> >    case "$ROOT" in
> >       *domainsnapshot*) # Must come first, since *domain* is a
> > substring
> >          TYPE="domainsnapshot"
> > @@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then
> >    exit 4
> >  fi
> >  
> > -xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> > -
> > +if [ $TMPFILE ]; then
> 
> Quoting
> "-n"
> 
> > +  xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE"
> > +else
> > +  xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> > +fi
> >  exit

Good point(s). I can only imagine I skipped the quoting because TMPFILE
is known to contain a safe string, but that is a poor excuse really.
I'll post a v2 shortly.

 /Johannes


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

Posted by Philipp Hahn 5 years, 6 months ago

Hello,

Am 21.05.19 um 09:35 schrieb Johannes Holmberg:
> On Tue, 2019-05-21 at 07:36 +0200, Philipp Hahn wrote:
>> Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
>>> diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
...
>>> +if [ "$XMLFILE" = "-" ]; then
>>> +    TMPFILE=`mktemp --tmpdir virt-xml.XXXX`
...
>> Quoting
> 
> Good point(s). I can only imagine I skipped the quoting because TMPFILE
> is known to contain a safe string,

No, it's not guaranteed, as it honors TMPDIR:

> $ LANG=C TMPDIR='/tmp/un safe' mktemp --tmpdir virt-xml.XXXX
> mktemp: failed to create file via template '/tmp/un safe/virt-xml.XXXX': ...

Philipp

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list