List for cgit developers and users
 help / color / mirror / Atom feed
From: cgit at cryptocrack.de (Lukas Fleischer)
Subject: [PATCH] Disallow downloading disabled snapshot formats
Date: Fri, 10 Jan 2014 16:02:21 +0100	[thread overview]
Message-ID: <20140110150221.14324.4431@typhoon.lan> (raw)
In-Reply-To: <20140110145014.GO7608@serenity.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;
> >       }


  reply	other threads:[~2014-01-10 15:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-10 14:38 cgit
2014-01-10 14:50 ` john
2014-01-10 15:02   ` cgit [this message]
2014-01-10 16:11 ` Jason
2014-01-10 17:14   ` cgit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140110150221.14324.4431@typhoon.lan \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).