On Fri, Oct 14, 2011 at 04:02:51PM -0500, Jonathan Nieder wrote:
> > Instead, let's print an "ERR" line, which git clients
> > understand since v1.6.1 (2008-12-24).
>
> Just to be clear, "git archive --remote" does not understand ERR lines
> in the 'master' branch (though 908aaceb makes it understand them in
> 'next'). But I consider even distinguishing
>
> a. fatal: git archive: protocol error
> b. fatal: git archive: expected ACK/NAK, got EOF
>
> [(a) is how an ERR response is reported, and (b) a remote hangup] to
> be progress, so it's not so important. :)
Thanks, I forgot to mention that. It's not as nice as the push/fetch
case, but I agree it's a step forward.
> > Because there is a risk of leaking information about
> > non-exported repositories, by default all errors simply say
> > "access denied". Open sites can pass a flag to turn on more
> > specific messages.
>
> I'm not sure what an "open site" is. :) But having this flag for
> sites to declare whether they consider whether a repository exists to
> be privileged information seems reasonable to me.
I meant sites which are just serving a bunch of public repos, like
kernel.org.
> Note that this really would be privileged information in some
> not-too-weird cases. For example, if many users have a repository at
> ~/.git, ~/.config/.git, or ~/src/linux/.git, then someone might try to
> access
>
> /home/alice/.git
> /home/alice/.config/.git
> /home/alice/src/linux/.git
> /home/bob/.git
> ...
>
> in turn to find a valid username, as reconnaisance for a later
> attack not involving git.
I sort of assume everybody serves a specific directory hierarchy, but
maybe that is not the case. I don't run git-daemon myself, so I am
probably guilty of generalizing how other people use it.
Anyway, I think the issue is sufficiently nuanced that we should keep
the default to the conservative "access denied" (i.e., throw away my
second patch for now).
> > --- a/daemon.c
> > +++ b/daemon.c
> > @@ -20,6 +20,7 @@
> [...]
> > @@ -1167,6 +1176,14 @@ int main(int argc, char **argv)
> > make_service_overridable(arg + 18, 0);
> > continue;
> > }
> > + if (!prefixcmp(arg, "--informative-errors")) {
> > + informative_errors = 1;
> > + continue;
> > + }
> > + else if (!prefixcmp(arg, "--no-informative-errors")) {
> > + informative_errors = 0;
> > + continue;
> > + }
> > if (!strcmp(arg, "--")) {
>
> Micronit: uncuddled "else". The style of the surrounding code is to
> just not include the "else" at all and rely on "continue" to
> short-circuit things.
Oops. Yes, it should just drop the else to match the surrounding code.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html