{"thread":{"id":"49871","subject":"pathspec: problems with too long command line","startedAt":"2018-11-21T12:37:03Z","lastAt":"2018-11-21T20:56:15Z","messageCount":4,"participants":["Marc Strapetz","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"363821","messageId":"c3be6eff-365b-96b8-16d2-0528612fc1fc@syntevo.com","threadId":"49871","inReplyTo":null,"subject":"pathspec: problems with too long command line","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-11-21T09:23:34Z","receivedAt":"2018-11-21T12:37:03Z","isPatch":false,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":" From our GUI client we are invoking git operations on a possibly large \nset of files. This may result in pathspecs which are exceeding the \nmaximum command line length, especially on Windows [1] and OSX [2]. To \nworkaround this problem we are currently splitting up such operations by \ninvoking multiple git commands. This works well for some commands (like \nadd), but doesn't work well for others (like commit).\n\nA possible solution could be to add another patchspec magic word which \nwill read paths from a file instead of command line. A similar approach \ncan be found in Mercurial with its \"listfile:\" pattern [3].\n\nDoes that sound reasonable? If so, we should be able to provide a \ncorresponding patch.\n\n-Marc\n\n[1] https://blogs.msdn.microsoft.com/oldnewthing/20031210-00/?p=41553/\n[2] https://serverfault.com/questions/69430\n[3] https://www.mercurial-scm.org/repo/hg/help/patterns\n\n\n"},{"id":"363822","messageId":"20181121132152.GA8246@sigill.intra.peff.net","threadId":"49871","inReplyTo":"c3be6eff-365b-96b8-16d2-0528612fc1fc@syntevo.com","subject":"Re: pathspec: problems with too long command line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-21T13:21:52Z","receivedAt":"2018-11-21T13:21:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 21, 2018 at 10:23:34AM +0100, Marc Strapetz wrote:\n\n> From our GUI client we are invoking git operations on a possibly large set\n> of files. This may result in pathspecs which are exceeding the maximum\n> command line length, especially on Windows [1] and OSX [2]. To workaround\n> this problem we are currently splitting up such operations by invoking\n> multiple git commands. This works well for some commands (like add), but\n> doesn't work well for others (like commit).\n> \n> A possible solution could be to add another patchspec magic word which will\n> read paths from a file instead of command line. A similar approach can be\n> found in Mercurial with its \"listfile:\" pattern [3].\n> \n> Does that sound reasonable? If so, we should be able to provide a\n> corresponding patch.\n\nQuite a few commands take --stdin, which can be used to send pathspecs\n(and often other stuff) without size limits. I don't think either\n\"commit\" or \"add\" does, but that might be another route.\n\nI'm slightly nervous at a pathspec that starts reading arbitrary files,\nbecause I suspect there may be interesting ways to abuse it for services\nwhich expose Git. E.g., if I have a web service which can show the\nhistory of a file, I might take a $file parameter from the client and\nrun \"git rev-list -- $file\" (handling shell quoting, of course). That's\nOK now, but with the proposed pathspec magic, a malicious user could ask\nfor \":(from-file=/etc/passwd)\" or whatever.\n\nI dunno. Maybe that is overly paranoid, and certainly servers like that\nare a subset of users. And perhaps such servers should be specifying\nGIT_LITERAL_PATHSPECS=1 anyway.\n\n-Peff\n"},{"id":"363824","messageId":"xmqq7eh67dqq.fsf@gitster-ct.c.googlers.com","threadId":"49871","inReplyTo":"20181121132152.GA8246@sigill.intra.peff.net","subject":"Re: pathspec: problems with too long command line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-21T13:37:49Z","receivedAt":"2018-11-21T13:37:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Nov 21, 2018 at 10:23:34AM +0100, Marc Strapetz wrote:\n>\n>> From our GUI client we are invoking git operations on a possibly large set\n>> of files. ...\n>> command line length, especially on Windows [1] and OSX [2]. To workaround\n>> this problem we are currently splitting up such operations by invoking\n>> multiple git commands. This works well for some commands (like add), but\n>> doesn't work well for others (like commit).\n\n> Quite a few commands take --stdin, which can be used to send pathspecs\n> (and often other stuff) without size limits. I don't think either\n> \"commit\" or \"add\" does, but that might be another route.\n\nA GUI client, like your server, should not be using end-user facing\nPorcelain commands like \"add\" and \"commit\" anyway.  In the standard\n\"update-index\" followed by \"write-tree\" followed-by \"commit-tree\"\nfollowed by \"update-ref\" sequence, the only thing that needs to take\npathspec is the update-index step, and it already does take --stdin.\n\nIn any case, I share your gut feeling that this should not be a\nmagic pathspec, but should instead be \"--stdin[-paths]\", for command\nline parsing's sanity.  Catchng random strings that begin with\ndouble dash as fishy is much simpler and more robust than having to\ntell if a string that is a risky or a benign magic pathspec.\n\n\n\n"},{"id":"363846","messageId":"8db810d2-5424-8833-c22f-5d9dfd77e3c5@syntevo.com","threadId":"49871","inReplyTo":"xmqq7eh67dqq.fsf@gitster-ct.c.googlers.com","subject":"Re: pathspec: problems with too long command line","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-11-21T20:56:11Z","receivedAt":"2018-11-21T20:56:15Z","isPatch":false,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":"On 21.11.2018 14:37, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n>> On Wed, Nov 21, 2018 at 10:23:34AM +0100, Marc Strapetz wrote:\n>>\n>>>  From our GUI client we are invoking git operations on a possibly large set\n>>> of files. ...\n>>> command line length, especially on Windows [1] and OSX [2]. To workaround\n>>> this problem we are currently splitting up such operations by invoking\n>>> multiple git commands. This works well for some commands (like add), but\n>>> doesn't work well for others (like commit).\n> \n>> Quite a few commands take --stdin, which can be used to send pathspecs\n>> (and often other stuff) without size limits. I don't think either\n>> \"commit\" or \"add\" does, but that might be another route.\n> \n> A GUI client, like your server, should not be using end-user facing\n> Porcelain commands like \"add\" and \"commit\" anyway.  In the standard\n> \"update-index\" followed by \"write-tree\" followed-by \"commit-tree\"\n> followed by \"update-ref\" sequence, the only thing that needs to take\n> pathspec is the update-index step, and it already does take --stdin.\n\nIn our case it's a desktop client. We didn't consider using plumbing \ncommands in general but only in cases where no appropriate high level \ncommands exist. One reason for this decision was definitely a lack of \nour understanding in the beginning (which is no excuse anymore :). \nAnother reason is that our users have quite frequently requested to see \ninvoked Git commands, for their own understanding and learning. I think \nthis argument remains valid. A third reason is to reduce process \ninvocations. Although we are quite experienced Git users now, one final \nreason may be that it's still desirable to rely on additional validation \nin high level commands.\n\nSummed up, I would prefer to find a solution which allows to stick with \n\"git add\"s, \"git commit\"s, \"git checkout\"s, ... and providing \n--stdin-paths alternatively to <pathspec> would be a good solution from \na GUI client developer's perspective. I'm probably too biased to see \nwhether it will be beneficial to standalone Git, too?\n\n>> I'm slightly nervous at a pathspec that starts reading arbitrary files,\n>> because I suspect there may be interesting ways to abuse it for services\n>> which expose Git. E.g., if I have a web service which can show the\n>> history of a file, I might take a $file parameter from the client and\n>> run \"git rev-list -- $file\" (handling shell quoting, of course). That's\n>> OK now, but with the proposed pathspec magic, a malicious user could ask\n>> for \":(from-file=/etc/passwd)\" or whatever.\n> \n> In any case, I share your gut feeling that this should not be a\n> magic pathspec, but should instead be \"--stdin[-paths]\", for command\n> line parsing's sanity.  Catchng random strings that begin with\n> double dash as fishy is much simpler and more robust than having to\n> tell if a string that is a risky or a benign magic pathspec.\n\nThese are interesting points. Then --stdin[-paths] is definitely the \nbetter choice.\n\n-Marc\n"}]}