From mboxrd@z Thu Jan  1 00:00:00 1970
From: cgit at cryptocrack.de (Lukas Fleischer)
Date: Fri, 10 Jan 2014 16:02:21 +0100
Subject: [PATCH] Disallow downloading disabled snapshot formats
In-Reply-To: <20140110145014.GO7608@serenity.lan>
References: <1389364686-14089-1-git-send-email-cgit@cryptocrack.de>
 <20140110145014.GO7608@serenity.lan>
Message-ID: <20140110150221.14324.4431@typhoon.lan>

On Fri, 10 Jan 2014 at 15:50:14, John Keeping wrote:
> On Fri, Jan 10, 2014 at 03:38:06PM +0100, Lukas Fleischer wrote:
> [...]
> > +test_expect_success 'try to download a disabled snapshot format' '
> > +     cgit_url "foo/snapshot/master.tar.xz" |
> > +     grep "Unsupported snapshot format"
> 
> I really dislike seeing pipes in the test suite.  Can we redirect to
> file instead and then grep the file?  This helps ensure that the exit
> code from CGit is correct (I don't know if we expect it to be zero or
> non-zero here, but if the latter then at least test_must_fail checks
> that the process didn't segfault - I suspect it should be zero though).

Does this matter here at all? If cgit_url fails in any (unpredictable)
way, e.g. segfaults, it probably won't print the string "Unsupported
snapshot format" and the whole test case will fail. The Git test suite
itself also uses grep(1) with a pipe several times. I agree that we
shouldn't use pipes when inverting the result (looking for non-existent
patterns), though.

> 
> > +'
> > +
> >  test_done
> > diff --git a/ui-snapshot.c b/ui-snapshot.c
> > index 8f82119..ab20a4a 100644
> > --- a/ui-snapshot.c
> > +++ b/ui-snapshot.c
> > @@ -205,7 +205,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
> >       }
> >  
> >       f = get_format(filename);
> > -     if (!f) {
> > +     if (!f || (snapshots & f->bit) == 0) {
> >               show_error("Unsupported snapshot format: %s", filename);
> >               return;
> >       }