Patchwork [1,of,2,STABLE] tests-subrepo-git: make the "pwned" message output in a stable order

login
register
mail settings
Submitter Danek Duvall
Date May 27, 2016, 10:32 p.m.
Message ID <c2c56e205a51017b4a2a.1464388363@smelly.us.oracle.com>
Download mbox | patch
Permalink /patch/15227/
State Superseded
Commit 1f8b861ba15c0306f253ddaa1dfc86e7f23116e8
Headers show

Comments

Danek Duvall - May 27, 2016, 10:32 p.m.
# HG changeset patch
# User Danek Duvall <danek.duvall@oracle.com>
# Date 1464387038 25200
#      Fri May 27 15:10:38 2016 -0700
# Branch stable
# Node ID c2c56e205a51017b4a2a205efab75882b9f86099
# Parent  89bba2beb03ea62e7fc8bcf3272e3cda1065ad89
tests-subrepo-git: make the "pwned" message output in a stable order

The "pwned" message from this test gets gets sent to stderr, and so may get
emitted in different places from run to run in the rest of mercurial's
output.  This patch forces the message to go to a specific file instead,
whose existence and contents we can examine at a stable point in the test's
execution.
timeless - May 29, 2016, 2:07 a.m.
This series is looking much better, thanks...

> +  $ test -f pwned.txt && cat pwned.txt || true

I wonder if this should be a shell macro that we use in both places...
Danek Duvall - May 30, 2016, 4:53 a.m.
timeless wrote:

> This series is looking much better, thanks...
> 
> > +  $ test -f pwned.txt && cat pwned.txt || true
> 
> I wonder if this should be a shell macro that we use in both places...

V3 is on its way.

Thanks,
Danek
Danek Duvall - May 31, 2016, 4:57 p.m.
Kevin Bullock wrote:

> > diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
> > --- a/tests/test-subrepo-git.t
> > +++ b/tests/test-subrepo-git.t
> > @@ -1135,7 +1135,7 @@ make sure we show changed files, rather 
> > test for Git CVE-2016-3068
> >   $ hg init malicious-subrepository
> >   $ cd malicious-subrepository
> > -  $ echo "s = [git]ext::sh -c echo% pwned% >&2" > .hgsub
> > +  $ echo "s = [git]ext::sh -c echo% pwned% >pwned.txt" > .hgsub
> >   $ git init s
> >   Initialized empty Git repository in $TESTTMP/tc/malicious-subrepository/s/.git/
> >   $ cd s
> > @@ -1145,23 +1145,27 @@ test for Git CVE-2016-3068
> >   $ hg add .hgsub
> >   $ hg commit -m "add subrepo"
> >   $ cd ..
> > +  $ rm -f pwned.txt
> >   $ env -u GIT_ALLOW_PROTOCOL hg clone malicious-subrepository malicious-subrepository-protected
> >   Cloning into '$TESTTMP/tc/malicious-subrepository-protected/s'... (glob)
> >   fatal: transport 'ext' not allowed
> >   updating to branch default
> > -  cloning subrepo s from ext::sh -c echo% pwned% >&2
> > +  cloning subrepo s from ext::sh -c echo% pwned% >pwned.txt
> >   abort: git clone error 128 in s (in subrepo s)
> >   [255]
> > +  $ test -f pwned.txt && cat pwned.txt || true
> 
> Is this actually needed? Are there shells that wait to see if there's any
> output instead of opening the target file for writing right away?

That's there because Solaris cat and GNU cat have different error messages
if the file doesn't exist:

    $ cat nope
    cat: cannot open nope: No such file or directory
    $ gcat nope
    gcat: nope: No such file or directory

Also, the Solaris cat exit code is 2, and the GNU cat exit code is 1.
<headdesk/>

I suppose f could grow a "dump contents if it exists and exit 1 if not"
function.  My change to put this bit into a shell function didn't make it
in, and Yuya requested static content in the file, too, so I was planning a
follow-up, anyway.  Should I change the shell function into something in f?

Thanks,
Danek
timeless - May 31, 2016, 6:58 p.m.
Having something in `f` seems like the right way to go. It's annoying
to keep having to get this right.

Patch

diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
--- a/tests/test-subrepo-git.t
+++ b/tests/test-subrepo-git.t
@@ -1135,7 +1135,7 @@  make sure we show changed files, rather 
 test for Git CVE-2016-3068
   $ hg init malicious-subrepository
   $ cd malicious-subrepository
-  $ echo "s = [git]ext::sh -c echo% pwned% >&2" > .hgsub
+  $ echo "s = [git]ext::sh -c echo% pwned% >pwned.txt" > .hgsub
   $ git init s
   Initialized empty Git repository in $TESTTMP/tc/malicious-subrepository/s/.git/
   $ cd s
@@ -1145,23 +1145,27 @@  test for Git CVE-2016-3068
   $ hg add .hgsub
   $ hg commit -m "add subrepo"
   $ cd ..
+  $ rm -f pwned.txt
   $ env -u GIT_ALLOW_PROTOCOL hg clone malicious-subrepository malicious-subrepository-protected
   Cloning into '$TESTTMP/tc/malicious-subrepository-protected/s'... (glob)
   fatal: transport 'ext' not allowed
   updating to branch default
-  cloning subrepo s from ext::sh -c echo% pwned% >&2
+  cloning subrepo s from ext::sh -c echo% pwned% >pwned.txt
   abort: git clone error 128 in s (in subrepo s)
   [255]
+  $ test -f pwned.txt && cat pwned.txt || true
 
 whitelisting of ext should be respected (that's the git submodule behaviour)
+  $ rm -f pwned.txt
   $ env GIT_ALLOW_PROTOCOL=ext hg clone malicious-subrepository malicious-subrepository-clone-allowed
   Cloning into '$TESTTMP/tc/malicious-subrepository-clone-allowed/s'... (glob)
-  pwned
   fatal: Could not read from remote repository.
   
   Please make sure you have the correct access rights
   and the repository exists.
   updating to branch default
-  cloning subrepo s from ext::sh -c echo% pwned% >&2
+  cloning subrepo s from ext::sh -c echo% pwned% >pwned.txt
   abort: git clone error 128 in s (in subrepo s)
   [255]
+  $ cat pwned.txt
+  pwned