Jeff King wrote:
> When the git-daemon is asked about an inaccessible
> repository, it simply hangs up the connection without saying
> anything further. This makes it hard to distinguish between
> a repository we cannot access (e.g., due to typo), and a
> service or network outage.
*nod*
> 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. :)
> 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.
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.
Luckily, this can be avoided with
git daemon --user-path=public_git --base-path=/pub/git
which only allows access to subdirectories of /pub/git and
public_git in home directories. With
git daemon --base-path=/pub/git
there is still no problem, since access to home directories is not
allowed at all in that case. I suppose the documentation should
mention that --informative-errors is best not used unless --base-path
is also in use. (Almost everyone is already using --base-path, so
this isn't a very serious problem.)
> --- 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.
Anyway, except for the documentation nits mentioned above (and Junio's
nit, too),
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks a lot for this.
--
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