On Mon, Oct 03, 2011 at 10:44:23AM +0200, Johannes Sixt wrote:
> Am 10/3/2011 9:42, schrieb Jeff King:
> > I still think push-over-git:// is a bit insane, and especially now with
> > smart-http, you'd be crazy to run it. And in that sense, I wouldn't mind
> > seeing it deprecated.
>
> You must be kidding ;) It is so much easier to type
>
> git daemon --export-all --enable=receive-pack
>
> for a one-shot, temporary git connection compared to setting up a
> smart-http, ssh, or even a rsh server.
Ah, yeah, I didn't think about one-shot invocations like that (I think
the original motivation was somebody actually running it all the time).
So yeah, that makes it even worse for the client to start refusing this
without even contacting the server. I forgot that we added the "ERR"
response way back in a807328 (connect.c: add a way for git-daemon to
pass an error back to client, 2008-11-01).
GitHub uses it to make nice messages:
$ git push origin
fatal: remote error:
You can't push to git://github.com/gitster/git.git
Use git@xxxxxxxxxx:gitster/git.git
We should maybe do something like the patch below:
diff --git a/daemon.c b/daemon.c
index 4c8346d..c1fa55f 100644
--- a/daemon.c
+++ b/daemon.c
@@ -255,6 +255,7 @@ static int run_service(char *dir, struct daemon_service *service)
loginfo("Request %s for '%s'", service->name, dir);
if (!enabled && !service->overridable) {
+ packet_write(1, "ERR %s: service not enabled", service->name);
logerror("'%s': service not enabled.", service->name);
errno = EACCES;
return -1;
@@ -288,6 +289,8 @@ static int run_service(char *dir, struct daemon_service *service)
enabled = service_enabled;
}
if (!enabled) {
+ packet_write(1, "ERR %s: service not enabled for '%s'",
+ service->name, path);
logerror("'%s': service not enabled for '%s'",
service->name, path);
errno = EACCES;
but:
1. There is some information leakage there. In particular, one can
tell the difference now between "repo does not exist" and
"receive-pack is not turned on". Personally, I think the tradeoff
to have actual error messages is worth it. HTTP has had real error
codes for decades, and I don't think anybody is too up-in-arms that
I can probe which pages are 404, and which are 401.
2. It probably makes sense to have a more human-friendly error
message.
3. It may be worth adding error messages for lots of other conditions
(e.g., no such repo). Assuming we accept the information leakage
for (1).
-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