{"thread":{"id":"23557","subject":"Useless error message?","startedAt":"2010-04-21T21:17:26Z","lastAt":"2010-04-22T22:21:53Z","messageCount":13,"participants":["Aghiles","Kim Ebert","Jonathan Nieder","Junio C Hamano","Andreas Ericsson","Petr Baudis","Ilari Liusvaara"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"140055","messageId":"z2o3abd05a91004211417v263d5a0eg497341ddf7bd79a5@mail.gmail.com","threadId":"23557","inReplyTo":null,"subject":"Useless error message?","fromName":"Aghiles","fromEmail":"aghilesk@gmail.com","sentAt":"2010-04-21T21:17:26Z","receivedAt":"2010-04-21T21:17:26Z","isPatch":false,"sender":{"key":"aghilesk@gmail.com","avatar":null},"body":"\"fatal: The remote end hung up unexpectedly\"\n\nIs that really meaningful ? Or maybe it is a configuration problem\non my side ?\n\n  -- aghiles\n"},{"id":"140062","messageId":"4BCF6E1E.701@gmail.com","threadId":"23557","inReplyTo":"z2o3abd05a91004211417v263d5a0eg497341ddf7bd79a5@mail.gmail.com","subject":"Re: Useless error message?","fromName":"Kim Ebert","fromEmail":"kd7ike@gmail.com","sentAt":"2010-04-21T21:29:02Z","receivedAt":"2010-04-21T21:29:02Z","isPatch":false,"sender":{"key":"kd7ike@gmail.com","avatar":null},"body":"I find that it usually means I didn't set up git-daemon-export-ok. Of \ncourse, that has usually been my experience.\n\nAghiles wrote:\n> \"fatal: The remote end hung up unexpectedly\"\n>\n> Is that really meaningful ? Or maybe it is a configuration problem\n> on my side ?\n>\n>   -- aghiles\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n>   \n"},{"id":"140070","messageId":"20100421221953.GA25348@progeny.tock","threadId":"23557","inReplyTo":"z2o3abd05a91004211417v263d5a0eg497341ddf7bd79a5@mail.gmail.com","subject":"Re: Useless error message?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-21T22:19:54Z","receivedAt":"2010-04-21T22:19:54Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Aghiles wrote:\n\n> \"fatal: The remote end hung up unexpectedly\"\n> \n> Is that really meaningful ? Or maybe it is a configuration problem\n> on my side ?\n\nPlease, fix it. :)\n\nThe problem is this: as far as I can tell, the git protocols are\ndesigned around the success case.  Sometimes if there is an error or\nother interesting event, the servers are kind enough to notify the\nuser “on the side”.  But in the end, all too often, they do not bother\nto inform the client _program_ that a fatal error occured.\n\nWe can’t just throw away this hang-up message because sometimes when\nthe remote host hangs up it really was unexpected.\n\nSo the trick is to make it expected more often.  See the side-band-64k\ncapability in Documentation/technical/protocol-capabilities.txt: the\ngoal is to have fatal error messages for as many failure modes as\npossible.\n\nExamples (I could be missing nuances; I am just trying to convey\nthe idea):\n\nupload-archive:\n - a pipe(), write(), or poll() failure when communicating over\n   the wire or between local processes will result in an unexpected\n   hangup.  I have not checked, but I suspect a SIGPIPE can kill\n   upload-archive, too.\n - On the bright side, all other error conditions are properly\n   handled.  The code for this is very nice and worth imitating.\n\nupload-pack:\n - a missing or shallow repo, HEAD or some other ref pointing to\n   a nonexistent object, early protocol error, or failure to start\n   rev-list or pack-objects will result in an unexpected hangup.\n - errors from rev-list, pack-objects, or the transition of the\n   generated pack are correctly handled.\n\nreceive-pack:\n - all errors result in unexpected hup as far as I can tell.\n\ndaemon:\n - hangs up without an explanation (except to syslog) for invalid\n   or disabled repositories\n - if the underlying service hangs up, hangs up.  If the underlying\n   service writes a message to stderr, writes that message to\n   syslog.  Surely the client is not interested...\n\nIf any other information would help, please let me know.\nJonathan\n"},{"id":"140104","messageId":"7vwrw0573t.fsf@alter.siamese.dyndns.org","threadId":"23557","inReplyTo":"20100421221953.GA25348@progeny.tock","subject":"Re: Useless error message?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-22T06:33:42Z","receivedAt":"2010-04-22T06:33:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> The problem is this: as far as I can tell, the git protocols are\n> designed around the success case.  Sometimes if there is an error or\n> other interesting event, the servers are kind enough to notify the\n> user “on the side”.  But in the end, all too often, they do not bother\n> to inform the client _program_ that a fatal error occured.\n\nThe true story is a bit different.\n\nTo avoid information leak to git-daemon clients, we deliberately choose\nnot to give detailed error messages, so that you cannot tell if an error\nmeans a user \"u\" does not exist or \"u\" does but ~u/repo.git repository\ndoes not exist.\n\n> So the trick is to make it expected more often.  See the side-band-64k\n> capability in Documentation/technical/protocol-capabilities.txt: the\n> goal is to have fatal error messages for as many failure modes as\n> possible.\n\nFor authenticated users (read: services that typically are behind auth) it\nwould be a good thing, but \"as many as possible\" you shouldn't be followed\nblindly.\n"},{"id":"140112","messageId":"20100422094153.GA504@progeny.tock","threadId":"23557","inReplyTo":"7vwrw0573t.fsf@alter.siamese.dyndns.org","subject":"Re: Useless error message?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-22T09:42:16Z","receivedAt":"2010-04-22T09:42:16Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> The true story is a bit different.\n> \n> To avoid information leak to git-daemon clients, we deliberately choose\n> not to give detailed error messages, so that you cannot tell if an error\n> means a user \"u\" does not exist or \"u\" does but ~u/repo.git repository\n> does not exist.\n\nThanks for the clarification.  As I see it, these are two different\nclasses of problem:\n\n1. The git daemon is very quiet, usually for good reason, as you\n   mentioned [1] [2].\n\n2. The git daemon and protocol helpers do not always send the datum “a\n   controlled fatal error occured” by writing some message (any\n   message) to side band 3.\n\nFixing the daemon’s share in both might require setting up a side band\nvery early.  If an RFC patch appears setting up the side band (or an\nexplanation for why that’s not possible), I would be happy to start\nwork building from there.\n\nThat has been the big obstacle for me experimenting with it, more than\nthe information disclosure.  But this is easy to say.  The doing is\nmore important.\n\nThanks again, and sorry for the noise.\nJonathan\n\n[1] I do suspect that in the case of failing enter_repo() or missing\ngit-daemon-export-ok, saying “cannot read the specified repo” would be\nfine.  Most of the time, there is not much value in disclosing a more\ndetailed reason, anyway.\n\n[2] Example fix for a problem in this class:\nhttp://thread.gmane.org/gmane.comp.version-control.git/139029\n"},{"id":"140113","messageId":"4BD01E09.8080504@op5.se","threadId":"23557","inReplyTo":"20100422094153.GA504@progeny.tock","subject":"Re: Useless error message?","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2010-04-22T09:59:37Z","receivedAt":"2010-04-22T09:59:37Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"On 04/22/2010 11:42 AM, Jonathan Nieder wrote:\n> Junio C Hamano wrote:\n> \n>> The true story is a bit different.\n>>\n>> To avoid information leak to git-daemon clients, we deliberately choose\n>> not to give detailed error messages, so that you cannot tell if an error\n>> means a user \"u\" does not exist or \"u\" does but ~u/repo.git repository\n>> does not exist.\n> \n> Thanks for the clarification.  As I see it, these are two different\n> classes of problem:\n> \n> 1. The git daemon is very quiet, usually for good reason, as you\n>     mentioned [1] [2].\n> \n> 2. The git daemon and protocol helpers do not always send the datum “a\n>     controlled fatal error occured” by writing some message (any\n>     message) to side band 3.\n> \n> [1] I do suspect that in the case of failing enter_repo() or missing\n> git-daemon-export-ok, saying “cannot read the specified repo” would be\n> fine.  Most of the time, there is not much value in disclosing a more\n> detailed reason, anyway.\n> \n\nThat would make it possible for random attackers to determine whether\na specific user exists on the system, which is very bad indeed.\n\n> [2] Example fix for a problem in this class:\n> http://thread.gmane.org/gmane.comp.version-control.git/139029\n\nThat's a different problem. We only end up in {send,receive}-pack if\nthe remote user asked for an existing repository, which means he or\nshe is either a very determined guesser or, more likely, already\nknows that the user exists and where he or she keeps git repos. A\npossible issue, to be sure, but definitely a far narrower window\nthan just guessing a username.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"},{"id":"140114","messageId":"20100422101535.GB625@progeny.tock","threadId":"23557","inReplyTo":"4BD01E09.8080504@op5.se","subject":"Re: Useless error message?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-22T10:15:35Z","receivedAt":"2010-04-22T10:15:35Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Andreas Ericsson wrote:\n> On 04/22/2010 11:42 AM, Jonathan Nieder wrote:\n\n>> [1] I do suspect that in the case of failing enter_repo() or missing\n>> git-daemon-export-ok, saying “cannot read the specified repo” would be\n>> fine.  Most of the time, there is not much value in disclosing a more\n>> detailed reason, anyway.\n>\n> That would make it possible for random attackers to determine whether\n> a specific user exists on the system, which is very bad indeed.\n\nI guess I am missing something.  How would\n\n(*) $ git clone git://git.example.com/~u/foo\n    remote: Cannot read the specified repo\n\ntell me whether that user existed on the system?  If the daemon gives\nthe same message for ENOENT, missing git-daemon-export-ok, EPERM, and\nso on so I cannot distinguish the cases, then I just don’t see the\nproblem.\n\nIf the daemon failed for some other reason, like a flaky network, I\nwould see\n\n    $ git clone git://git.example.com/~u/foo\n    fatal: The remote end hung up unexpectedly\n\nSo the extra information could still be helpful, without unwanted\ninformation disclosure.  In the case (*) I learn definitively that the\naddress I specified does not represent a repo I have access to, rather\nthan this being some random, transient unexplained problem.\n\nThanks for the comment.\nJonathan\n"},{"id":"140116","messageId":"4BD0247A.4080103@op5.se","threadId":"23557","inReplyTo":"20100422101535.GB625@progeny.tock","subject":"Re: Useless error message?","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2010-04-22T10:27:06Z","receivedAt":"2010-04-22T10:27:06Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"On 04/22/2010 12:15 PM, Jonathan Nieder wrote:\n> Andreas Ericsson wrote:\n>> On 04/22/2010 11:42 AM, Jonathan Nieder wrote:\n> \n>>> [1] I do suspect that in the case of failing enter_repo() or missing\n>>> git-daemon-export-ok, saying “cannot read the specified repo” would be\n>>> fine.  Most of the time, there is not much value in disclosing a more\n>>> detailed reason, anyway.\n>>\n>> That would make it possible for random attackers to determine whether\n>> a specific user exists on the system, which is very bad indeed.\n> \n> I guess I am missing something.  How would\n> \n> (*) $ git clone git://git.example.com/~u/foo\n>      remote: Cannot read the specified repo\n> \n> tell me whether that user existed on the system?  If the daemon gives\n> the same message for ENOENT, missing git-daemon-export-ok, EPERM, and\n> so on so I cannot distinguish the cases, then I just don’t see the\n> problem.\n> \n> If the daemon failed for some other reason, like a flaky network, I\n> would see\n> \n>      $ git clone git://git.example.com/~u/foo\n>      fatal: The remote end hung up unexpectedly\n> \n> So the extra information could still be helpful, without unwanted\n> information disclosure.  In the case (*) I learn definitively that the\n> address I specified does not represent a repo I have access to, rather\n> than this being some random, transient unexplained problem.\n> \n\nSo that would be the new error message for everything that fails, then?\n\nOne big reason why I'm not bothered with running the git-daemon on a\npublic server is that it's very simple. If something goes wrong, it\ndies without fiddling about.\n\nHow would it benefit you if it said \"fatal: Something went wrong, but\nI didn't crash\" instead of just hanging up? If you have the wrong\nrepo address, you'd still have to check up with whoever gave it to\nyou to get it right. If it *does* crash, you'd still have to get\nhold of the server admin to tell him that it has crashed.\n\nA minor patch to git-fetch, updating the error message with a few\npossible reasons would be far better. I don't care about it myself,\nbut I'm sure such a patch would be a lot easier to get into git.git\nthan something that adds a lot of complexity to the git daemon.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"},{"id":"140117","messageId":"20100422103830.GB701@progeny.tock","threadId":"23557","inReplyTo":"4BD0247A.4080103@op5.se","subject":"Re: Useless error message?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-22T10:38:30Z","receivedAt":"2010-04-22T10:38:30Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Andreas Ericsson wrote:\n> On 04/22/2010 12:15 PM, Jonathan Nieder wrote:\n\n>> (*) $ git clone git://git.example.com/~u/foo\n>>      remote: Cannot read the specified repo\n[...]\n> So that would be the new error message for everything that fails, then?\n\nNo.  Of course, the opposite is the point.  I just mean there should\nbe an error message for all conditions that are lumped together with\nENOENT to avoid information disclosure.  I don’t care much how the\nmessage is phrased.\n\n> If you have the wrong\n> repo address, you'd still have to check up with whoever gave it to\n> you to get it right. If it *does* crash, you'd still have to get\n> hold of the server admin to tell him that it has crashed.\n\nMany things can go wrong other than a missing repo.  For example,\nthere might be objects missing, or high load, or memory corruption.\n\nIn the “wrong address” case, I know who to go to to fix it: ask the\nperson who gave me the address.\n\nIn the “corrupt repository” case, I should go to the repository\nowner.\n\nIn the “server hung up without any good reason” case, I should try to\nnarrow down the problem, perhaps with the help of the server admin or\nmy network administrator.  Is it really so unusual to want to\ndistinguish this from other cases?\n\nWithout code this is all theoretical anyway.  And that’s the real\nproblem: to the uninitiated, it is not easy to write code to try this\nout because the side band is not set up in time.\n\nSigh,\nJonathan\n"},{"id":"140122","messageId":"20100422115625.GJ3563@machine.or.cz","threadId":"23557","inReplyTo":"20100421221953.GA25348@progeny.tock","subject":"Re: Useless error message?","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-04-22T11:56:25Z","receivedAt":"2010-04-22T11:56:25Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Apr 21, 2010 at 05:19:54PM -0500, Jonathan Nieder wrote:\n> Aghiles wrote:\n> \n> > \"fatal: The remote end hung up unexpectedly\"\n> > \n> > Is that really meaningful ? Or maybe it is a configuration problem\n> > on my side ?\n> \n> Please, fix it. :)\n\nI have seen a lot of users who plainly had a lot of trouble even\n_understanding_ the error message - it is phrased in super-dense\nnetworking jargon. I think something like\n\n\t\"fatal: Server terminated the connection for unknown reason\"\n\nmight come a long way (though of course specific error messages would\nstill be far more helpful).\n\n(I assume that the remote end is the server since (i) it is most often\nthe case (ii) it is if you look at it through the client-server optics,\nwhich may not always be the best one, but see (i).)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nhttp://pasky.or.cz/ | \"Ars longa, vita brevis.\" -- Hippocrates\n"},{"id":"140126","messageId":"20100422124453.GA30328@LK-Perkele-V2.elisa-laajakaista.fi","threadId":"23557","inReplyTo":"20100422094153.GA504@progeny.tock","subject":"Re: Useless error message?","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-04-22T12:44:53Z","receivedAt":"2010-04-22T12:44:53Z","isPatch":false,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Thu, Apr 22, 2010 at 04:42:16AM -0500, Jonathan Nieder wrote:\n> \n> Thanks for the clarification.  As I see it, these are two different\n> classes of problem:\n> \n> 1. The git daemon is very quiet, usually for good reason, as you\n>    mentioned [1] [2].\n> \n> 2. The git daemon and protocol helpers do not always send the datum “a\n>    controlled fatal error occured” by writing some message (any\n>    message) to side band 3.\n> \n> Fixing the daemon’s share in both might require setting up a side band\n> very early.  If an RFC patch appears setting up the side band (or an\n> explanation for why that’s not possible), I would be happy to start\n> work building from there.\n\nThere are few subcases of daemon-level errors:\n\n1A) Invalid request\n\nNo feedback is needed. These are protocol violations and well-behaved\nclients don't send these.\n\n1B) Request for invalid repository\n\nThese should have one error. That error can be sent using ERR response\n(already supported).\n\n1C) Request for disabled service\n\nThese too can be reported via ERR. One has to be careful not to create\ninformation leak using these.\n\n1D) Catastrophic network error\n\nOne can't do anything about these.\n\n1E) Relay error\n\nReally shouldn't happen. Due to service state being unknown at time\nof things going wrong, one can't do much about these (what if\nrelay error occurs in middle of packet? pad packet with zeroes?)\n\n\nSo, pretty much the only daemon-level errors with feedback required\nwould be one for invalid repository and disabled service. How about:\n\n\"foo/example: unreadable or anonymous fetching not allowed.\"\n\"foo/example: unreadable or anonymous pushing not allowed.\"\n\"foo/example: unreadable or anonymous snapshotting not allowed.\"\n\"fooserv: requested service unknown.\"\n\nAnd all of these can be sent over ERR. I don't see need for using\nsidebands.\n\n> That has been the big obstacle for me experimenting with it, more than\n> the information disclosure.  But this is easy to say.  The doing is\n> more important.\n\n-Ilari\n"},{"id":"140153","messageId":"l2x3abd05a91004221313s2cb89697i3bcbfbcd6ccf6820@mail.gmail.com","threadId":"23557","inReplyTo":"20100422115625.GJ3563@machine.or.cz","subject":"Re: Useless error message?","fromName":"Aghiles","fromEmail":"aghilesk@gmail.com","sentAt":"2010-04-22T20:13:53Z","receivedAt":"2010-04-22T20:13:53Z","isPatch":false,"sender":{"key":"aghilesk@gmail.com","avatar":null},"body":"Hello,\n\n>\n> I have seen a lot of users who plainly had a lot of trouble even\n> _understanding_ the error message - it is phrased in super-dense\n> networking jargon. I think something like\n>\n>        \"fatal: Server terminated the connection for unknown reason\"\n>\n> might come a long way (though of course specific error messages would\n> still be far more helpful).\n>\n\nI would say that:\n\n  \"fatal: Server terminated the connection.\"\n\nIs best and actually says what it has to stay. \"Unknown reason\" sounds like a\ngodly intervention into the git protocol. Sometimes, the \"hung up\" message is\ngiven as an extra information that actually causes more harm than good:\n\n % git pull hummus\n fatal: 'hummus' does not appear to be a git repository\n fatal: The remote end hung up unexpectedly\n\n  -- aghiles\n"},{"id":"140172","messageId":"20100422222153.GA12000@progeny.tock","threadId":"23557","inReplyTo":"20100422124453.GA30328@LK-Perkele-V2.elisa-laajakaista.fi","subject":"[PATCH] daemon: report inaccessible repositories to user","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-22T22:21:53Z","receivedAt":"2010-04-22T22:21:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This is a follow-up to v1.6.1-rc1~101 (connect.c: add a way for\ngit-daemon to pass an error back to client, 2008-11-01).  Although\nnot all clients support that protocol extension yet, it should be safe\nto start using it anyway, as explained below.\n\nThis patch teaches ‘git daemon’ to let the client know when the\nrequest to access the repository was denied.  Instead of just hanging\nup, now the server lets the client know that access was denied, with a\ncarefully worded error that echoes the request:\n\n  fatal: remote error: /foo/example: unreadable or fetching not allowed.\n  fatal: remote error: /foo/example: unreadable or pushing not allowed.\n  fatal: remote error: /foo/example: unreadable or snapshotting not allowed.\n\nThe failure could be due to one of a few causes.  The message does not\ndistinguish them:\n\n - chdir() failure\n - the protocol was disabled\n - not a git repository\n - not marked for export\n\nNon-admin clients have no reason to care --- all of these situations\nrepresent the same “not a public repository” condition.  Server\nadmins, on the other hand, would care a great deal to know that the\ndaemon does not reveal information about the machine’s configuration\n(such as what directories exist) aside from what requests it is\nconfigured to honor.\n\nThe corresponding safety for protocol helpers used over ssh has been\nin place since v0.99.9k^2~54 (Server-side support for user-relative\npaths, 2005-11-17).\n\nThe error message used does _not_ match the output from backends\n(which is sent to stderr rather than to the client, anyway) when\nenter_repo() fails.  That is fine since ‘git daemon’ already checks\nthat the repository exists by calling enter_repo() itself.\n\nPre-1.6.1 git versions will treat the error request as a breach of\nprotocol.  The result is an acceptable if somewhat funny message.\n\n  fatal: protocol error: expected sha/ref, got 'ERR /foo/example:\n  unreadable or fetching not allowed.'\n\nThanks to Dscho, Ilari, and Andreas for help.  Thanks especially to\nIlari for explaining how this should work.\n\nCc: Johannes Schindelin <johannes.schindelin@gmx.de>\nCc: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\nCc: Andreas Ericsson <ae@op5.se>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nIlari Liusvaara wrote:\n\n> There are few subcases of daemon-level errors:\n[...]\n> So, pretty much the only daemon-level errors with feedback required\n> would be one for invalid repository and disabled service. How about:\n> \n> \"foo/example: unreadable or anonymous fetching not allowed.\"\n> \"foo/example: unreadable or anonymous pushing not allowed.\"\n> \"foo/example: unreadable or anonymous snapshotting not allowed.\"\n> \"fooserv: requested service unknown.\"\n> \n> And all of these can be sent over ERR. I don't see need for using\n> sidebands.\n\nI omitted the qualifier “anonymous” in case we want to reuse these\nmessages for authenticated situations.  I haven’t thought about it\ndeeply, though.\n\nI would also like to write tests.  To begin with, it might be easiest\nto test in inetd mode to avoid allocating a port for it.\n\nAnyway, that shouldn’t hold up giving you a chance to nak the patch. :)\n\nThanks for the pointers.  ERR packets do seem like the right way to\ndo this.\n\nJonathan\n\n daemon.c |   20 ++++++++++++++++----\n 1 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex a90ab10..732a339 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -222,6 +222,7 @@ static char *path_ok(char *directory)\n typedef int (*daemon_service_fn)(void);\n struct daemon_service {\n \tconst char *name;\n+\tconst char *description;\n \tconst char *config_name;\n \tdaemon_service_fn fn;\n \tint enabled;\n@@ -231,6 +232,12 @@ struct daemon_service {\n static struct daemon_service *service_looking_at;\n static int service_enabled;\n \n+static void report_inaccessible(char *dir, struct daemon_service *service)\n+{\n+\tpacket_write(1, \"ERR %s: unreadable or %s not allowed.\\n\",\n+\t             dir, service->description);\n+}\n+\n static int git_daemon_config(const char *var, const char *value, void *cb)\n {\n \tif (!prefixcmp(var, \"daemon.\") &&\n@@ -252,12 +259,15 @@ static int run_service(char *dir, struct daemon_service *service)\n \n \tif (!enabled && !service->overridable) {\n \t\tlogerror(\"'%s': service not enabled.\", service->name);\n+\t\treport_inaccessible(dir, service);\n \t\terrno = EACCES;\n \t\treturn -1;\n \t}\n \n-\tif (!(path = path_ok(dir)))\n+\tif (!(path = path_ok(dir))) {\n+\t\treport_inaccessible(dir, service);\n \t\treturn -1;\n+\t}\n \n \t/*\n \t * Security on the cheap.\n@@ -272,6 +282,7 @@ static int run_service(char *dir, struct daemon_service *service)\n \n \tif (!export_all_trees && access(\"git-daemon-export-ok\", F_OK)) {\n \t\tlogerror(\"'%s': repository not exported.\", path);\n+\t\treport_inaccessible(dir, service);\n \t\terrno = EACCES;\n \t\treturn -1;\n \t}\n@@ -286,6 +297,7 @@ static int run_service(char *dir, struct daemon_service *service)\n \tif (!enabled) {\n \t\tlogerror(\"'%s': service not enabled for '%s'\",\n \t\t\t service->name, path);\n+\t\treport_inaccessible(dir, service);\n \t\terrno = EACCES;\n \t\treturn -1;\n \t}\n@@ -362,9 +374,9 @@ static int receive_pack(void)\n }\n \n static struct daemon_service daemon_service[] = {\n-\t{ \"upload-archive\", \"uploadarch\", upload_archive, 0, 1 },\n-\t{ \"upload-pack\", \"uploadpack\", upload_pack, 1, 1 },\n-\t{ \"receive-pack\", \"receivepack\", receive_pack, 0, 1 },\n+\t{ \"upload-archive\", \"snapshotting\", \"uploadarch\", upload_archive, 0, 1 },\n+\t{ \"upload-pack\", \"fetching\", \"uploadpack\", upload_pack, 1, 1 },\n+\t{ \"receive-pack\", \"pushing\", \"receivepack\", receive_pack, 0, 1 },\n };\n \n static void enable_service(const char *name, int ena)\n-- \n1.7.1.rc2.8.ga54f9\n"}]}