{"thread":{"id":"19303","subject":"git fails with control characters in trunk directory name","startedAt":"2009-05-11T20:08:19Z","lastAt":"2009-05-16T17:49:39Z","messageCount":30,"participants":["Hugo Mildenberger","Alex Riesen","Junio C Hamano","Daniel Barkalow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"113584","messageId":"200905112208.21017.Hugo.Mildenberger@namir.de","threadId":"19303","inReplyTo":null,"subject":"git fails with control characters in trunk directory name","fromName":"Hugo Mildenberger","fromEmail":"hugo.mildenberger@namir.de","sentAt":"2009-05-11T20:08:19Z","receivedAt":"2009-05-11T20:08:19Z","isPatch":false,"sender":{"key":"hugo.mildenberger@namir.de","avatar":null},"body":"Using the mouse to paste a git url from a website into a terminal session in \norder to clone the repository, I recently managed to include invisible \ncontrol characters into the git trunk directory name. \n\nConsequently, I faced all sort of strange behaviour like git pull not working \n(error 2), later on a kernel make which supposedly could not finding a rule \nto create the trunk directory and more such inconsistencies. \nI then reinstalled git, rcs and so on and also tried unsuccessfully several \ngit versions. The next morning I looked into the .git/config file and \nrecognized that the \"url\" key value within the [remote \"origin\"] section \ncontained some control characters: ^J and \\n, as fas as I remember.\n\nWhile this was almost entirely my fault, git could possibly apply a filter, \nreject such a name or at least issue a warning. \n"},{"id":"113613","messageId":"81b0412b0905112351l126a7c5bh902a7c7748d9a77a@mail.gmail.com","threadId":"19303","inReplyTo":"200905112208.21017.Hugo.Mildenberger@namir.de","subject":"Re: git fails with control characters in trunk directory name","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-12T06:51:29Z","receivedAt":"2009-05-12T06:51:29Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/11 Hugo Mildenberger :\n> Using the mouse to paste a git url from a website into a terminal session in\n> order to clone the repository, I recently managed to include invisible\n> control characters into the git trunk directory name.\n\nGit has no \"trunk\". Not in CVS/CVN sense, at least\n\n> Consequently, I faced all sort of strange behaviour like git pull not working\n> (error 2), later on a kernel make which supposedly could not finding a rule\n> to create the trunk directory and more such inconsistencies.\n> I then reinstalled git, rcs and so on and also tried unsuccessfully several\n> git versions. The next morning I looked into the .git/config file and\n> recognized that the \"url\" key value within the [remote \"origin\"] section\n> contained some control characters: ^J and \\n, as fas as I remember.\n\nWhat platform are you on?\nCan you show your .git/config?\n\n> While this was almost entirely my fault, git could possibly apply a filter,\n> reject such a name or at least issue a warning.\n\nMaybe. Or maybe it can just work (well, assuming the user meant to\nuse an url with character you considered \"control\").\n"},{"id":"113624","messageId":"200905121103.00432.Hugo.Mildenberger@namir.de","threadId":"19303","inReplyTo":"81b0412b0905112351l126a7c5bh902a7c7748d9a77a@mail.gmail.com","subject":"Re: git fails with control characters in trunk directory name","fromName":"Hugo Mildenberger","fromEmail":"hugo.mildenberger@namir.de","sentAt":"2009-05-12T09:02:59Z","receivedAt":"2009-05-12T09:02:59Z","isPatch":false,"sender":{"key":"hugo.mildenberger@namir.de","avatar":null},"body":"Hello Alex,\n\n> 2009/5/11 Hugo Mildenberger :\n> > Using the mouse to paste a git url from a website into a terminal session\n> > in order to clone the repository, I recently managed to include invisible\n> > control characters into the git trunk directory name.\n>\n> Git has no \"trunk\". Not in CVS/CVN sense, at least\n>\n> > Consequently, I faced all sort of strange behaviour like git pull not\n> > working (error 2), later on a kernel make which supposedly could not\n> > finding a rule to create the trunk directory and more such\n> > inconsistencies.\n> > I then reinstalled git, rcs and so on and also tried unsuccessfully\n> > several git versions. The next morning I looked into the .git/config file\n> > and recognized that the \"url\" key value within the [remote \"origin\"]\n> > section contained some control characters: ^J and \\n, as fas as I\n> > remember.\n>\n> What platform are you on?\n> Can you show your .git/config?\n>\n> > While this was almost entirely my fault, git could possibly apply a\n> > filter, reject such a name or at least issue a warning.\n>\n> Maybe. Or maybe it can just work (well, assuming the user meant to\n> use an url with character you considered \"control\").\n\nI said it actually did not work well, independent from how anyone prefers to \nclassify characters. My platform is Gentoo-hardened with unicode support and \nan ext3 disk format .  With \"trunk directory\"  I meant the top level \ndirectory which is created when you run git clone on a remote url -- sorry \nfor still not being a native git speaker. I don't have the original setup \nanymore. My _working_ \".git/config\" is now:\n\n[core]\n        repositoryformatversion = 0\n        filemode = true\n        bare = false\n        logallrefupdates = true\n[remote \"origin\"]\n        url = \ngit://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n        fetch = +refs/heads/*:refs/remotes/origin/*\n[branch \"master\"]\n        remote = origin\n        merge = refs/heads/master\n\nThe remote url did contain something like \"^J\\n\" at the end, as did the top \nlevel (the much derided \"trunk\") directory name on disk. I probably got there \nby puting the copied url within quotes on the command line, but today I'm  \nunsure exactly how I arrived there. However, old fashioned as I am, I still \nconsider e.g. a linefeed to be a \"control character\", and inspite of your \nflashing git punditry I still consider this to be an issue.\n"},{"id":"113627","messageId":"81b0412b0905120354p6e1bf517y154601132f0901a3@mail.gmail.com","threadId":"19303","inReplyTo":"200905121103.00432.Hugo.Mildenberger@namir.de","subject":"Re: git fails with control characters in trunk directory name","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-12T10:54:20Z","receivedAt":"2009-05-12T10:54:20Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/12 Hugo Mildenberger :\n>> 2009/5/11 Hugo Mildenberger :\n>> > While this was almost entirely my fault, git could possibly apply a\n>> > filter, reject such a name or at least issue a warning.\n>>\n>> Maybe. Or maybe it can just work (well, assuming the user meant to\n>> use an url with character you considered \"control\").\n>\n> I said it actually did not work well, independent from how anyone prefers to\n> classify characters. My platform is Gentoo-hardened with unicode support and\n> an ext3 disk format .  With \"trunk directory\"  I meant the top level\n> directory which is created when you run git clone on a remote url -- sorry\n> for still not being a native git speaker. I don't have the original setup\n> anymore. My _working_ \".git/config\" is now:\n\nOk, let's assume you picked up '\\r\\n' from that web page\n\n> The remote url did contain something like \"^J\\n\" at the end, as did the top\n> level (the much derided \"trunk\") directory name on disk. I probably got there\n> by puting the copied url within quotes on the command line, but today I'm\n> unsure exactly how I arrived there. However, old fashioned as I am, I still\n> consider e.g. a linefeed to be a \"control character\", and inspite of your\n> flashing git punditry I still consider this to be an issue.\n\nGit tries not to enforce any specific rules where the program itself does\nnot need them. One can easily imagine an automatic system which\ngenerates names of repositories from something like binary data\n(something like BigTable comes to mind) and the less rules the\nunderlying levels impose on it, the simplier the upper levels will be.\n\nOTOH, a warning about commonly used delimiters not fitting a name\ncontext, maybe a good idea. Like \"\\r\\n\\t\", backslash (came up recently\non this list). Such a check and associated warning may be useful for\nrepository names and branches.\n\nStill, it's more of a policy issue and I would make it optional, even\nif enabled by default. Maybe even by defining a regexp which the\nrepo name or branch name must (for hard error) or should (for a warning)\nmatch.\n\nFor your specific case, you can take a look at builtin-clone.c,\njust after the line containing \"guess_dir_name(repo_name\"...\n"},{"id":"113634","messageId":"200905121557.18542.Hugo.Mildenberger@namir.de","threadId":"19303","inReplyTo":"81b0412b0905120354p6e1bf517y154601132f0901a3@mail.gmail.com","subject":"Re: git fails with control characters in trunk directory name","fromName":"Hugo Mildenberger","fromEmail":"hugo.mildenberger@namir.de","sentAt":"2009-05-12T13:57:17Z","receivedAt":"2009-05-12T13:57:17Z","isPatch":false,"sender":{"key":"hugo.mildenberger@namir.de","avatar":null},"body":"> OTOH, a warning about commonly used delimiters not fitting a name\n> context, maybe a good idea. Like \"\\r\\n\\t\", backslash (came up recently\n> on this list). Such a check and associated warning may be useful for\n> repository names and branches.\n>\n> Still, it's more of a policy issue and I would make it optional, even\n> if enabled by default. Maybe even by defining a regexp which the\n> repo name or branch name must (for hard error) or should (for a warning)\n> match.\n>\n> For your specific case, you can take a look at builtin-clone.c,\n> just after the line containing \"guess_dir_name(repo_name\"...\n> --\n\nBut at least the git versions I tried (up to 1.6.3) really do have a problem \nwhen facing a trailing newline in repository names; so one should enforce a \nconvention. Although the situation here is certainly a far-flung, uncommon \none, it could also happen when git was called from scripts.\n\nI looked into guess_dir_name(). A regex call would be easy to fit, but \ncurrently the git binary does not depend on libpcre. Is it generally \nconsidered to be acceptable to add such a dependency? \n\nWhile I like the idea to make use of a configurable regular expression, such \nan expression had to be a command line parameter with a reasonable default \nvalue, because .git/config still would not exist when the value would be \nneeded. \n\nLast not least, I managed to reproduce the problem almost exactly:\n\n1.) hm@localhost git \nclone \"git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n\" \n\t(Note the trailing linefeed)\n\n2.) hm@localhost ~/tmp/bluetooth-testing.git $ make\n\tMakefile:313: /home/hm/tmp/bluetooth-testing.git\n\t/scripts/Kbuild.include: No such file or directory\n\tmake[1]: /home/hm/tmp/bluetooth-testing.git: No such file or directory\n\tmake[1]: *** No rule to make target `/home/hm/tmp/bluetooth-testing.git'.\n\tStop.\n\tmake: *** No rule to make target `include/config/auto.conf', needed by\n\t`include/config/kernel.release'. Stop.\n\n3.) hm@localhost ~/tmp/bluetooth-testing.git $ git pull\n\tfatal: Error in line 2:\n\n4.) \".git/config\" now contains\n\nurl = \ngit://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\\n\n\nI particulary liked the git message \"fatal: Error in line 2:\" ... \n\ngrep says, it would stem from builtin-fmt-merge-msg.c/fmt_merge_msg():\n270 /* get a line */\n271 while (pos < in->len) {\n272 int len;\n273 char *newline, *p = in->buf + pos;\n274\n275 newline = strchr(p, '\\n');\n276 len = newline ? newline - p : strlen(p);\n277 pos += len + !!newline;\n278 i++;\n279 p[len] = 0;\n280 if (handle_line(p))\n281 die (\"Error in line %d: %.*s\", i, len, p);\n282 }\n"},{"id":"113639","messageId":"81b0412b0905120759u15f1ec73k73625a7904515792@mail.gmail.com","threadId":"19303","inReplyTo":"200905121557.18542.Hugo.Mildenberger@namir.de","subject":"Re: git fails with control characters in trunk directory name","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-12T14:59:34Z","receivedAt":"2009-05-12T14:59:34Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/12 Hugo Mildenberger :\n>> OTOH, a warning about commonly used delimiters not fitting a name\n>> context, maybe a good idea. Like \"\\r\\n\\t\", backslash (came up recently\n>> on this list). Such a check and associated warning may be useful for\n>> repository names and branches.\n>>\n>> Still, it's more of a policy issue and I would make it optional, even\n>> if enabled by default. Maybe even by defining a regexp which the\n>> repo name or branch name must (for hard error) or should (for a warning)\n>> match.\n>>\n>> For your specific case, you can take a look at builtin-clone.c,\n>> just after the line containing \"guess_dir_name(repo_name\"...\n>> --\n>\n> But at least the git versions I tried (up to 1.6.3) really do have a problem\n> when facing a trailing newline in repository names; so one should enforce a\n> convention.\n\nThat's what I mean by saying: \"... even if enabled by default\".\nI just want to disable newbie-helping annoyances on my systems.\n\n> I looked into guess_dir_name().\n\nThat's not the right place. The place I meant is right below the call\nto this function (you have to parse the names given in the command-line\ntoo).\n\nThe automatically generated (that's the case with guess_dir_name)\ndirectory name certainly shouldn't contain any unexpected characters.\n\n> A regex call would be easy to fit, but\n> currently the git binary does not depend on libpcre. Is it generally\n> considered to be acceptable to add such a dependency?\n\nNo. And pcre is not the only regex lib in the world. And we prefer\nshell patterns, if any at all.\n\n> While I like the idea to make use of a configurable regular expression, such\n> an expression had to be a command line parameter with a reasonable default\n> value, because .git/config still would not exist when the value would be\n> needed.\n\nThat's where _default_ policy plays its role. \"Default\" like in \"it is compiled\ninto the git executable and needs no configuration present\".\n\n> Last not least, I managed to reproduce the problem almost exactly:\n>\n> 1.) hm@localhost git\n> clone \"git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n> \"\n>        (Note the trailing linefeed)\n\nThat's all the command printed? No \"Initialized empty Git repository\" line?\n\n> 2.) hm@localhost ~/tmp/bluetooth-testing.git $ make\n\nHmm... At this point the clone may have worked (at least partially).\nIt named \"bluetooth-testing.git\", which it shouldn't (but explainable:\nthe repo url suffix is not .git anymore, but \".git\\r\\n\"). But it looks like\nthe post-clone checkout failed (silently? which would be bad):\n\n>        Makefile:313: /home/hm/tmp/bluetooth-testing.git\n>        /scripts/Kbuild.include: No such file or directory\n>        make[1]: /home/hm/tmp/bluetooth-testing.git: No such file or directory\n>        make[1]: *** No rule to make target `/home/hm/tmp/bluetooth-testing.git'.\n>        Stop.\n>        make: *** No rule to make target `include/config/auto.conf', needed by\n>        `include/config/kernel.release'.  Stop.\n\nAssuming the files must be present, of course.\n\n> 3.) hm@localhost ~/tmp/bluetooth-testing.git $ git pull\n>        fatal: Error in line 2:\n>\n> 4.) \".git/config\" now contains\n>\n> url =\n> git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\\n\n>\n> I particulary liked the git message \"fatal: Error in line 2:\" ...\n>\n\nSeparate issue. Will look at it later.\n"},{"id":"113670","messageId":"200905121900.00625.Hugo.Mildenberger@namir.de","threadId":"19303","inReplyTo":"81b0412b0905120759u15f1ec73k73625a7904515792@mail.gmail.com","subject":"Re: git fails with control characters in trunk directory name","fromName":"Hugo Mildenberger","fromEmail":"hugo.mildenberger@namir.de","sentAt":"2009-05-12T16:59:58Z","receivedAt":"2009-05-12T16:59:58Z","isPatch":false,"sender":{"key":"hugo.mildenberger@namir.de","avatar":null},"body":"> > I looked into guess_dir_name().\n> \n> That's not the right place. The place I meant is right below the call\n> to this function (you have to parse the names given in the command-line\n> too).\n> \n> The automatically generated (that's the case with guess_dir_name)\n> directory name certainly shouldn't contain any unexpected characters.\n> \n> > A regex call would be easy to fit, but\n> > currently the git binary does not depend on libpcre. Is it generally\n> > considered to be acceptable to add such a dependency?\n> \n> No. And pcre is not the only regex lib in the world. And we prefer\n> shell patterns, if any at all.\n> \n> > While I like the idea to make use of a configurable regular expression, such\n> > an expression had to be a command line parameter with a reasonable default\n> > value, because .git/config still would not exist when the value would be\n> > needed.\n> \n> That's where _default_ policy plays its role. \"Default\" like in \"it is compiled\n> into the git executable and needs no configuration present\".\n> \n> > Last not least, I managed to reproduce the problem almost exactly:\n> >\n> > 1.) hm@localhost git\n> > clone \"git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n> > \"\n> >        (Note the trailing linefeed)\n> \n> That's all the command printed? No \"Initialized empty Git repository\" line?\n> \n> > 2.) hm@localhost ~/tmp/bluetooth-testing.git $ make\n> \n> Hmm... At this point the clone may have worked (at least partially).\n> It named \"bluetooth-testing.git\", which it shouldn't (but explainable:\n> the repo url suffix is not .git anymore, but \".git\\r\\n\"). But it looks like\n> the post-clone checkout failed (silently? which would be bad):\n> \n> >        Makefile:313: /home/hm/tmp/bluetooth-testing.git\n> >        /scripts/Kbuild.include: No such file or directory\n> >        make[1]: /home/hm/tmp/bluetooth-testing.git: No such file or directory\n> >        make[1]: *** No rule to make target `/home/hm/tmp/bluetooth-testing.git'.\n> >        Stop.\n> >        make: *** No rule to make target `include/config/auto.conf', needed by\n> >        `include/config/kernel.release'.  Stop.\n> \n> Assuming the files must be present, of course.\n> \n> > 3.) hm@localhost ~/tmp/bluetooth-testing.git $ git pull\n> >        fatal: Error in line 2:\n> >\n> > 4.) \".git/config\" now contains\n> >\n> > url =\n> > git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\\n\n> >\n> > I particulary liked the git message \"fatal: Error in line 2:\" ...\n> >\n> \n> Separate issue. Will look at it later.\n> --\n\nYou really want to use shell patterns to match against a string from within a binary? Although git \nalready makes use of regexec from glibc or compat/regex directory in numerous places? \n\nRegarding the procedure above, sorry for keeping it much too short. Here comes a \ncomplete sequence (I will put some comments within square brackets):\n\nhm@localhost /var/tmp $ git --version\ngit version 1.6.0.6\n\nhm@localhost /var/tmp $ git clone \"git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n\"\nInitialized empty Git repository in /mnt/hda1/tmp/bluetooth-testing.git\n/.git/\nremote: Counting objects: 1177836, done.\nremote: Compressing objects: 100% (189467/189467), done.\nremote: Total 1177836 (delta 982785), reused 1176855 (delta 981880)\nReceiving objects: 100% (1177836/1177836), 288.16 MiB | 1288 KiB/s, done.\nResolving deltas: 100% (982785/982785), done.\nChecking out files: 100% (27842/27842), done.\n\nhm@localhost /var/tmp $ ls\nbluetooth-testing.git? [Note that question mark replacing \\n in repository's name]\n\nhm@localhost /var/tmp $ cd blue\t\nbluetooth-testing.git^J/ bluez-utils-2.25-r1.new/\n[I typed cd blue here, and there is the \"^J\" I remembered to have seen elsewhere] \n\nhm@localhost /var/tmp $ cd 'bluetooth-testing.git\n'/ [bash puts the directory name automatically within single quotes in this case]\n\nhm@localhost /var/tmp/bluetooth-testing.git $ make defconfig\n*** Default configuration is based on 'i386_defconfig'\n#\n# configuration written to .config\n#\n\nhm@localhost /var/tmp/bluetooth-testing.git $ make\nmake[1]: /mnt/hda1/tmp/bluetooth-testing.git: No such file or directory\nmake[1]: *** No rule to make target `/mnt/hda1/tmp/bluetooth-testing.git'. Stop.\nmake: *** No rule to make target `include/config/auto.conf', needed by `include/config/kernel.release'. Stop.\n[The problem is simply the odd directory name, while make tries to use the real one]\n\nhm@localhost /var/tmp/bluetooth-testing.git $ git pull\nfatal: Error in line 2:\n"},{"id":"113671","messageId":"81b0412b0905121018lbccda1fvf6c4c19417cdde00@mail.gmail.com","threadId":"19303","inReplyTo":"200905121900.00625.Hugo.Mildenberger@namir.de","subject":"Re: git fails with control characters in trunk directory name","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-12T17:18:33Z","receivedAt":"2009-05-12T17:18:33Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/12 Hugo Mildenberger :\n>> > Last not least, I managed to reproduce the problem almost exactly:\n>> >\n>> > 1.) hm@localhost git\n>> > clone \"git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n>> > \"\n>> >        (Note the trailing linefeed)\n>>\n>> That's all the command printed? No \"Initialized empty Git repository\" line?\n>\n> hm@localhost /var/tmp $ git clone \"git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n> \"\n> Initialized empty Git repository in /mnt/hda1/tmp/bluetooth-testing.git\n> /.git/\n> remote: Counting objects: 1177836, done.\n> remote: Compressing objects: 100% (189467/189467), done.\n> remote: Total 1177836 (delta 982785), reused 1176855 (delta 981880)\n> Receiving objects: 100% (1177836/1177836), 288.16 MiB | 1288 KiB/s, done.\n> Resolving deltas: 100% (982785/982785), done.\n> Checking out files: 100% (27842/27842), done.\n>\n> hm@localhost /var/tmp $ ls\n> bluetooth-testing.git?   [Note that question mark replacing \\n in repository's name]\n>\n\nOk, clone works. Fully and correctly (even if a bit unexpected).\n\n> hm@localhost /var/tmp/bluetooth-testing.git $ make\n> make[1]: /mnt/hda1/tmp/bluetooth-testing.git: No such file or directory\n> make[1]: *** No rule to make target `/mnt/hda1/tmp/bluetooth-testing.git'.  Stop.\n\nIt is just linux build system which does not support compilation in oddly\nnamed directories (which is reasonable, if you ask me).\n\n> hm@localhost /var/tmp/bluetooth-testing.git $ git pull\n> fatal: Error in line 2:\n\nThat's already fixed. Will send the fix in a minute.\n"},{"id":"113672","messageId":"20090512172452.GA32594@blimp.localdomain","threadId":"19303","inReplyTo":"81b0412b0905121018lbccda1fvf6c4c19417cdde00@mail.gmail.com","subject":"[PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-12T17:24:52Z","receivedAt":"2009-05-12T17:24:52Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The fmt-merge-msg does a strong syntax checking of its input and fails\nwith if it is incorrect. The LF character is the only character\nimportant for fmt-merge-msg. As the url in FETCH_HEAD plays only\ninformational role, a quoted representation of the url should be good\nand true enough.\nThe url often comes from either user-editable config or command line,\nso it is reasonable to expect all kinds of characters in it, including\nthe characters which the format of FETCH_HEAD considers special (line\nseparator in this case).\n\nNoticed and reported by Hugo Mildenberger.\n\nSigned-off-by: Alex Riesen \n---\n\nAlex Riesen, Tue, May 12, 2009 19:18:33 +0200:\n> 2009/5/12 Hugo Mildenberger :\n> > hm@localhost /var/tmp/bluetooth-testing.git $ git pull\n> > fatal: Error in line 2:\n> \n> That's already fixed. Will send the fix in a minute.\n\nHere\n\n builtin-fetch.c | 9 +++++++--\n 1 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 3c998ea..eef7091 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -353,12 +353,17 @@ static int store_updated_refs(const char *url, const char *remote_name,\n \t\t\t\t\t\t kind);\n \t\t\tnote_len += sprintf(note + note_len, \"'%s' of \", what);\n \t\t}\n-\t\tnote_len += sprintf(note + note_len, \"%.*s\", url_len, url);\n-\t\tfprintf(fp, \"%s\\t%s\\t%s\\n\",\n+\t\tfprintf(fp, \"%s\\t%s\\t%s\",\n \t\t\tsha1_to_hex(commit ? commit->object.sha1 :\n \t\t\t\t rm->old_sha1),\n \t\t\trm->merge ? \"\" : \"not-for-merge\",\n \t\t\tnote);\n+\t\tfor (i = 0; i < url_len; ++i)\n+\t\t\tif ('\\n' == url[i])\n+\t\t\t\tfputs(\"\\\\n\", fp);\n+\t\t\telse\n+\t\t\t\tfputc(url[i], fp);\n+\t\tfputc('\\n', fp);\n \n \t\tif (ref)\n \t\t\trc |= update_local_ref(ref, what, note);\n-- \n1.6.3.28.ga852b\n"},{"id":"113673","messageId":"20090512174122.GB32594@blimp.localdomain","threadId":"19303","inReplyTo":"200905121900.00625.Hugo.Mildenberger@namir.de","subject":"Re: git fails with control characters in trunk directory name","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-12T17:41:22Z","receivedAt":"2009-05-12T17:41:22Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Hugo Mildenberger, Tue, May 12, 2009 18:59:58 +0200:\n> > > I looked into guess_dir_name().\n> > \n> > That's not the right place. The place I meant is right below the call\n> > to this function (you have to parse the names given in the command-line\n> > too).\n> > \n> > The automatically generated (that's the case with guess_dir_name)\n> > directory name certainly shouldn't contain any unexpected characters.\n> > \n> > > A regex call would be easy to fit, but\n> > > currently the git binary does not depend on libpcre. Is it generally\n> > > considered to be acceptable to add such a dependency?\n> > \n> > No. And pcre is not the only regex lib in the world. And we prefer\n> > shell patterns, if any at all.\n> \n> You really want to use shell patterns to match against a string from within a binary?\n\nIs that a problem? Especially if the matching expression is just\nsomething like a character class?\n\n> Although git already makes use of regexec from glibc or compat/regex\n> directory in numerous places? \n\nYou said pcre, and I wasn't able to look at the source at the time I\nanswered your mail to check if git relies on POSIX regexp. I did\n(and remembered git grep), so no need for pcre and regular exceptions\nare just a line away.\n\n\nP.S. Could you please quote more appropriately? Your discussion is a\nlittle hard to follow.\n"},{"id":"113708","messageId":"7vws8l7w0d.fsf@alter.siamese.dyndns.org","threadId":"19303","inReplyTo":"20090512172452.GA32594@blimp.localdomain","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-12T23:16:02Z","receivedAt":"2009-05-12T23:16:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> +\t\tfor (i = 0; i < url_len; ++i)\n> +\t\t\tif ('\\n' == url[i])\n> +\t\t\t\tfputs(\"\\\\n\", fp);\n> +\t\t\telse\n> +\t\t\t\tfputc(url[i], fp);\n> +\t\tfputc('\\n', fp);\n\nThis ad-hoc quoting feels _very_ wrong. Who is on the reading side and\nhow does it unquote? If it uses quote.c infrastructure, we should be\nquoting using the function from the same library shouldn't we?\n\nIf it is just informational use only, then it might make more sense to\ndrop this ugly \"quoted \\n\" silently. I dunno.\n"},{"id":"113722","messageId":"81b0412b0905122306w4ed41bdiab073a05587fab55@mail.gmail.com","threadId":"19303","inReplyTo":"7vws8l7w0d.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-13T06:06:58Z","receivedAt":"2009-05-13T06:06:58Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/13 Junio C Hamano :\n> Alex Riesen writes:\n>\n>> +             for (i = 0; i < url_len; ++i)\n>> +                     if ('\\n' == url[i])\n>> +                             fputs(\"\\\\n\", fp);\n>> +                     else\n>> +                             fputc(url[i], fp);\n>> +             fputc('\\n', fp);\n>\n> This ad-hoc quoting feels _very_ wrong.  Who is on the reading side and\n> how does it unquote?\n\ngit fmt-merge-msg. It does not unquote. The url is purely informational here.\nOTOH, the \\n shouldn't be in url text at all, so treat it as slightly\nless annoying\nwarning.\n\n> If it is just informational use only, then it might make more sense to\n> drop this ugly \"quoted \\n\" silently.  I dunno.\n\nThat'd mean to loose the information completely. Which is just as bad\nas putting the LF in the url in the first place.\n"},{"id":"113780","messageId":"81b0412b0905130510n61d59f54rf385a89bbb43a530@mail.gmail.com","threadId":"19303","inReplyTo":"200905131340.31509.Hugo.Mildenberger@namir.de","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-13T12:10:32Z","receivedAt":"2009-05-13T12:10:32Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/13 Hugo Mildenberger :\n> Am Mittwoch, 13. Mai 2009 schrieb Alex Riesen \n>>\n>> That'd mean to loose the information completely. Which is just as bad\n>> as putting the LF in the url in the first place.\n>\n> This stray linefeed is not information, but pure contamination. Thus it\n> would be much better to simply strip it off.\n\nNot at the place where the patch changes it. In git clone, maybe (the many\ntimes mentioned guessing function). But then, we have to provide an option\nto leave the names alone, verbatim (which, I think, the non-quessing form\nalready provides. No additional coding necessary).\n\n> And besides from the fact that git apply rejects this patch (fatal: corrupt\n> patch at line 6),\n\nThis is your local problem. Maybe you copied the patch through clipboard\nand it kept the presentation in KMail, or KMail generally corrupts\nmails, I don't\nknow. Just save the mail as is and do \"git am -3\" on it.\n\n> I think it would also not handle the equally wrong repository directory name on disk,\n\nYou think wrong. It does handle it perfectly.\n\nAnd it is not wrong. It is just how it is. Don't spend too much time in MacOS\nand that overgrown DOS. They'll wreck your brain.\n\n> which then possibly leads to subsequent make failures (as it actually happend in\n> the case I described earlier here.)\n\nThe make(1) works in such directories just fine. Fix yours (thinking about\nit now - you wont find a make which does not work in such directories).\n\n> Why not just return to your original idea, which proposed testing the\n> repository name against a regular expression describing a forbidden\n> set of characters (which is \"\\n\", currently) and then terminate with a\n> clear message?\n\nThe idea was not abandoned nor followed upon. It's just I don't need that\nwarning and would switch it off immediately anyway (if it was implemented).\nI just spent a minute thinking about it (that's where regexp idea came from).\nI'm not going to work on it (at least, not right now). I'm even sure you wont\nbe working on it too, now when you have learned what the problem is and\nhow to work around it. And that's while all you needed is to put a two-three\nlines in that guessing function...\n"},{"id":"113782","messageId":"200905131439.33907.Hugo.Mildenberger@namir.de","threadId":"19303","inReplyTo":"81b0412b0905122306w4ed41bdiab073a05587fab55@mail.gmail.com","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Hugo Mildenberger","fromEmail":"hugo.mildenberger@namir.de","sentAt":"2009-05-13T12:39:32Z","receivedAt":"2009-05-13T12:39:32Z","isPatch":true,"sender":{"key":"hugo.mildenberger@namir.de","avatar":null},"body":"Am Mittwoch, 13. Mai 2009 schrieb Alex Riesen \n> 2009/5/13 Junio C Hamano :\n> > Alex Riesen writes:\n> >\n> >> +             for (i = 0; i < url_len; ++i)\n> >> +                     if ('\\n' == url[i])\n> >> +                             fputs(\"\\\\n\", fp);\n> >> +                     else\n> >> +                             fputc(url[i], fp);\n> >> +             fputc('\\n', fp);\n> >\n> > This ad-hoc quoting feels _very_ wrong.  Who is on the reading side and\n> > how does it unquote?\n> \n> git fmt-merge-msg. It does not unquote. The url is purely informational here.\n> OTOH, the \\n shouldn't be in url text at all, so treat it as slightly\n> less annoying\n> warning.\n> \n> > If it is just informational use only, then it might make more sense to\n> > drop this ugly \"quoted \\n\" silently.  I dunno.\n> \n> That'd mean to loose the information completely. Which is just as bad\n> as putting the LF in the url in the first place.\n> --\n\nThis stray linefeed is not information, but pure contamination. Thus it \nwould be much better to simply strip it off. And besides from the fact that \ngit apply rejects this patch (fatal: corrupt patch at line 6), I think it would \nalso not handle the equally wrong repository directory name on disk, which \nthen possibly leads to subsequent make failures (as it actually happend in \nthe case I described earlier here.) Why not just return to your original idea, \nwhich proposed testing the repository name against a regular expression \ndescribing a forbidden set of characters (which is \"\\n\", currently) and then \nterminate with a clear message?\n"},{"id":"113793","messageId":"200905131649.13159.Hugo.Mildenberger@namir.de","threadId":"19303","inReplyTo":"81b0412b0905130510n61d59f54rf385a89bbb43a530@mail.gmail.com","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Hugo Mildenberger","fromEmail":"hugo.mildenberger@namir.de","sentAt":"2009-05-13T14:49:11Z","receivedAt":"2009-05-13T14:49:11Z","isPatch":true,"sender":{"key":"hugo.mildenberger@namir.de","avatar":null},"body":"Am Mittwoch, 13. Mai 2009 schrieb Alex Riesen:\n> 2009/5/13 Hugo Mildenberger :\n> > Am Mittwoch, 13. Mai 2009 schrieb Alex Riesen \n> >>\n> >> That'd mean to loose the information completely. Which is just as bad\n> >> as putting the LF in the url in the first place.\n> >\n> > This stray linefeed is not information, but pure contamination. Thus it\n> > would be much better to simply strip it off.\n> \n> Not at the place where the patch changes it. In git clone, maybe (the many\n> times mentioned guessing function). But then, we have to provide an option\n> to leave the names alone, verbatim (which, I think, the non-quessing form\n> already provides. No additional coding necessary).\n> \n> > And besides from the fact that git apply rejects this patch (fatal: corrupt\n> > patch at line 6),\n> \n> This is your local problem. Maybe you copied the patch through clipboard\n> and it kept the presentation in KMail, or KMail generally corrupts\n> mails, I don't\n> know. Just save the mail as is and do \"git am -3\" on it.\n\nMy apologies, the problem actually was copy & paste from kmail.\n\n> > I think it would also not handle the equally wrong repository directory name on disk,\n> \n> You think wrong. It does handle it perfectly.\n\nI'm thinking right. The stray linefeed is still there:\n\nls -l 'bluetooth-testing.git\n'/ -d\ndrwxr-xr-x 23 hm hm 4096 13. Mai 15:06 bluetooth-testing.git?/\n\n\n> > which then possibly leads to subsequent make failures (as it actually happend in\n> > the case I described earlier here.)\n> \n> The make(1) works in such directories just fine. Fix yours (thinking about\n> it now - you wont find a make which does not work in such directories).\n\nYou aren't saying which make you are using, but GNU make version 3.81 or \nthe linux kernel make system still can't cope with a linefeed contaminated directory \nname:\n\nhm@localhost /var/tmp/bluetooth-testing.git $ make\nmake[1]: /mnt/hda1/tmp/bluetooth-testing.git: No such file or directory\nmake[1]: *** No rule to make target `/mnt/hda1/tmp/bluetooth-testing.git'. Stop.\nmake: *** No rule to make target `include/config/auto.conf', needed by `include/config/kernel.release'. Stop.\n\n> > Why not just return to your original idea, which proposed testing the\n> > repository name against a regular expression describing a forbidden\n> > set of characters (which is \"\\n\", currently) and then terminate with a\n> > clear message?\n> \n> The idea was not abandoned nor followed upon. It's just I don't need that\n> warning and would switch it off immediately anyway (if it was implemented).\n> I just spent a minute thinking about it (that's where regexp idea came from).\n> I'm not going to work on it (at least, not right now). I'm even sure you wont\n> be working on it too, now when you have learned what the problem is and\n> how to work around it. And that's while all you needed is to put a two-three\n> lines in that guessing function...\n\nI reported this problem here after I already knew the cause and how to avoid\nthe problem, because I thought it might be useful fix it generally, especially \nuseful for those who run git within a brushwood of scripts, which I do not. \nIf you personally would need this or not is not of so much interest here. But \nI was interested in discussing a clean way to implement it.\n"},{"id":"113795","messageId":"alpine.LNX.2.00.0905131109240.2147@iabervon.org","threadId":"19303","inReplyTo":"81b0412b0905122306w4ed41bdiab073a05587fab55@mail.gmail.com","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-05-13T15:18:40Z","receivedAt":"2009-05-13T15:18:40Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 13 May 2009, Alex Riesen wrote:\n\n> 2009/5/13 Junio C Hamano :\n> > Alex Riesen writes:\n> >\n> >> +             for (i = 0; i < url_len; ++i)\n> >> +                     if ('\\n' == url[i])\n> >> +                             fputs(\"\\\\n\", fp);\n> >> +                     else\n> >> +                             fputc(url[i], fp);\n> >> +             fputc('\\n', fp);\n> >\n> > This ad-hoc quoting feels _very_ wrong.  Who is on the reading side and\n> > how does it unquote?\n> \n> git fmt-merge-msg. It does not unquote. The url is purely informational here.\n> OTOH, the \\n shouldn't be in url text at all, so treat it as slightly\n> less annoying\n> warning.\n> \n> > If it is just informational use only, then it might make more sense to\n> > drop this ugly \"quoted \\n\" silently.  I dunno.\n> \n> That'd mean to loose the information completely. Which is just as bad\n> as putting the LF in the url in the first place.\n\nLooking back at the original message, it looks like the user included a \nnewline in an argument to clone, and the fetch must have stripped it out \n(or ignored it in some other fashion), because data was retrieved from a \nrepository that doesn't have a newline in its name. Most likely, the \nnewline should just be prohibited in the URL in the config file in the \nfirst place, and we shouldn't be able to get to the point of writing a \nFETCH_HEAD with that value.\n\n\t-Daniel\n*This .sig left intentionally blank*"},{"id":"113800","messageId":"81b0412b0905130909x2ad67407p53ad6f79966c1ea3@mail.gmail.com","threadId":"19303","inReplyTo":"alpine.LNX.2.00.0905131109240.2147@iabervon.org","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-13T16:09:42Z","receivedAt":"2009-05-13T16:09:42Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/13 Daniel Barkalow :\n> On Wed, 13 May 2009, Alex Riesen wrote:\n>> 2009/5/13 Junio C Hamano :\n>>\n>> > If it is just informational use only, then it might make more sense to\n>> > drop this ugly \"quoted \\n\" silently.  I dunno.\n>>\n>> That'd mean to loose the information completely. Which is just as bad\n>> as putting the LF in the url in the first place.\n>\n> Looking back at the original message, it looks like the user included a\n> newline in an argument to clone, and the fetch must have stripped it out\n> (or ignored it in some other fashion), because data was retrieved from a\n> repository that doesn't have a newline in its name.\n\n_That_ looks like a bug to me. We should have asked exactly the name\nwe were given, or notify the user about what we have used for the url\nif we have ignored user's input and decided to use something else.\nPatches follow.\n\n> ... Most likely, the\n> newline should just be prohibited in the URL in the config file in the\n> first place, and we shouldn't be able to get to the point of writing a\n> FETCH_HEAD with that value.\n\nWhat I cannot understand is what's wrong with url containing an LF?\nEspecially if we can handle it, all the tools can handle it, and putting\nLF in a filename is a long-standing UNIX tradition.\n"},{"id":"113805","messageId":"20090513170749.GA26288@blimp.localdomain","threadId":"19303","inReplyTo":"81b0412b0905130909x2ad67407p53ad6f79966c1ea3@mail.gmail.com","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-13T17:07:49Z","receivedAt":"2009-05-13T17:07:49Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Strip trailing spaces off guessed target directory, and replace every\n'control' character in the new directory name with ASCII space (\\x20).\n\nThe user still can have any name by specifying it explicitely after url.\n\nSigned-off-by: Alex Riesen \n---\n\nAlex Riesen, Wed, May 13, 2009 18:09:42 +0200:\n> 2009/5/13 Daniel Barkalow :\n> > On Wed, 13 May 2009, Alex Riesen wrote:\n> >> 2009/5/13 Junio C Hamano :\n> >>\n> >> > If it is just informational use only, then it might make more sense to\n> >> > drop this ugly \"quoted \\n\" silently.  I dunno.\n> >>\n> >> That'd mean to loose the information completely. Which is just as bad\n> >> as putting the LF in the url in the first place.\n> >\n> > Looking back at the original message, it looks like the user included a\n> > newline in an argument to clone, and the fetch must have stripped it out\n> > (or ignored it in some other fashion), because data was retrieved from a\n> > repository that doesn't have a newline in its name.\n> \n> _That_ looks like a bug to me. We should have asked exactly the name\n> we were given, or notify the user about what we have used for the url\n> if we have ignored user's input and decided to use something else.\n> Patches follow.\n> \n\nNow I just have to figure out why the fetch works. Something seem to\nstrip spaces off urls...\n\n builtin-clone.c | 24 ++++++++++++++++++------\n 1 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 880373f..bdbe931 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -104,11 +104,12 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n {\n \tconst char *end = repo + strlen(repo), *start;\n+\tchar *dir;\n \n \t/*\n-\t * Strip trailing slashes and /.git\n+\t * Strip trailing spaces, slashes and /.git\n \t */\n-\twhile (repo < end && is_dir_sep(end[-1]))\n+\twhile (repo < end && (is_dir_sep(end[-1]) || isspace(end[-1])))\n \t\tend--;\n \tif (end - repo > 5 && is_dir_sep(end[-5]) &&\n \t !strncmp(end - 4, \".git\", 4)) {\n@@ -140,10 +141,21 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n \tif (is_bare) {\n \t\tstruct strbuf result = STRBUF_INIT;\n \t\tstrbuf_addf(&result, \"%.*s.git\", (int)(end - start), start);\n-\t\treturn strbuf_detach(&result, 0);\n-\t}\n-\n-\treturn xstrndup(start, end - start);\n+\t\tdir = strbuf_detach(&result, 0);\n+\t} else\n+\t\tdir = xstrndup(start, end - start);\n+\t/* replace all 'control' characters with ascii space */\n+\tfor (start = dir; *start; ++start)\n+\t\tif (*(const unsigned char *)start < 32u)\n+\t\t\tdir[start - dir] = '\\x20';\n+\t/* remove trailing spaces */\n+\tif (dir < start)\n+\t\tfor (end = start; dir < --end; )\n+\t\t\tif (!isspace(*end))\n+\t\t\t\tbreak;\n+\t\t\telse\n+\t\t\t\tdir[end - dir] = '\\0';\n+\treturn dir;\n }\n \n static void strip_trailing_slashes(char *dir)\n-- \n1.6.3.28.ga852b\n"},{"id":"113807","messageId":"alpine.LNX.2.00.0905131212000.2147@iabervon.org","threadId":"19303","inReplyTo":"81b0412b0905130909x2ad67407p53ad6f79966c1ea3@mail.gmail.com","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-05-13T17:12:26Z","receivedAt":"2009-05-13T17:12:26Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 13 May 2009, Alex Riesen wrote:\n\n> 2009/5/13 Daniel Barkalow :\n> > On Wed, 13 May 2009, Alex Riesen wrote:\n> >> 2009/5/13 Junio C Hamano :\n> >>\n> >> > If it is just informational use only, then it might make more sense to\n> >> > drop this ugly \"quoted \\n\" silently.  I dunno.\n> >>\n> >> That'd mean to loose the information completely. Which is just as bad\n> >> as putting the LF in the url in the first place.\n> >\n> > Looking back at the original message, it looks like the user included a\n> > newline in an argument to clone, and the fetch must have stripped it out\n> > (or ignored it in some other fashion), because data was retrieved from a\n> > repository that doesn't have a newline in its name.\n> \n> _That_ looks like a bug to me. We should have asked exactly the name\n> we were given, or notify the user about what we have used for the url\n> if we have ignored user's input and decided to use something else.\n> Patches follow.\n> \n> > ... Most likely, the\n> > newline should just be prohibited in the URL in the config file in the\n> > first place, and we shouldn't be able to get to the point of writing a\n> > FETCH_HEAD with that value.\n> \n> What I cannot understand is what's wrong with url containing an LF?\n> Especially if we can handle it, all the tools can handle it, and putting\n> LF in a filename is a long-standing UNIX tradition.\n\nThe RFC for URLs (which we don't actually follow particularly much) \ndoesn't permit whitespace. Protocols that use URLs directly generally use \nnewlines as control characters. For example, an HTTP request for:\n\nhttp://host/path/with\nnewline/in/it\n\nwould parse as a request for \"http://host/path/with\" with an invalid \nheader line. Normally, to refer to a resource whose name contains a \nnewline, the URL contains \"%0A\" instead.\n\nOf course, our \"url\" config can contain some things that aren't URLs, but \nthis particular case was of the form that's supposed to be a standard URL.\n\n\t-Daniel\n*This .sig left intentionally blank*"},{"id":"113818","messageId":"20090513180853.GB26288@blimp.localdomain","threadId":"19303","inReplyTo":"20090512172452.GA32594@blimp.localdomain","subject":"[PATCH 1/2] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-13T18:08:53Z","receivedAt":"2009-05-13T18:08:53Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The fmt-merge-msg does a strong syntax checking of its input and fails\nwith if it is incorrect. The LF character is the only character\nimportant for fmt-merge-msg. As the url in FETCH_HEAD plays only\ninformational role, a quoted representation of the url should be good\nand true enough.\nThe url often comes from either user-editable config or command line,\nso it is reasonable to expect all kinds of characters in it, including\nthe characters which the format of FETCH_HEAD considers special (line\nseparator in this case).\n\nNoticed and reported by Hugo Mildenberger.\n\nSigned-off-by: Alex Riesen \n---\n\nAlex Riesen, Tue, May 12, 2009 19:24:52 +0200:\n> Alex Riesen, Tue, May 12, 2009 19:18:33 +0200:\n> > 2009/5/12 Hugo Mildenberger :\n> > > hm@localhost /var/tmp/bluetooth-testing.git $ git pull\n> > > fatal: Error in line 2:\n> > \n> > That's already fixed. Will send the fix in a minute.\n> \n> Here\n> \n\nThat one has a bug which breaks t5515: printing an unterminated\nstring ('note').\n\n builtin-fetch.c | 10 ++++++++--\n 1 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 3c998ea..ec75df0 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -353,12 +353,18 @@ static int store_updated_refs(const char *url, const char *remote_name,\n \t\t\t\t\t\t kind);\n \t\t\tnote_len += sprintf(note + note_len, \"'%s' of \", what);\n \t\t}\n-\t\tnote_len += sprintf(note + note_len, \"%.*s\", url_len, url);\n-\t\tfprintf(fp, \"%s\\t%s\\t%s\\n\",\n+\t\tnote[note_len] = '\\0';\n+\t\tfprintf(fp, \"%s\\t%s\\t%s\",\n \t\t\tsha1_to_hex(commit ? commit->object.sha1 :\n \t\t\t\t rm->old_sha1),\n \t\t\trm->merge ? \"\" : \"not-for-merge\",\n \t\t\tnote);\n+\t\tfor (i = 0; i < url_len; ++i)\n+\t\t\tif ('\\n' == url[i])\n+\t\t\t\tfputs(\"\\\\n\", fp);\n+\t\t\telse\n+\t\t\t\tfputc(url[i], fp);\n+\t\tfputc('\\n', fp);\n \n \t\tif (ref)\n \t\t\trc |= update_local_ref(ref, what, note);\n-- \n1.6.3.28.ga852b\n"},{"id":"113819","messageId":"20090513181135.GC26288@blimp.localdomain","threadId":"19303","inReplyTo":"alpine.LNX.2.00.0905131212000.2147@iabervon.org","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-13T18:11:35Z","receivedAt":"2009-05-13T18:11:35Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Daniel Barkalow, Wed, May 13, 2009 19:12:26 +0200:\n> Of course, our \"url\" config can contain some things that aren't URLs, but \n> this particular case was of the form that's supposed to be a standard URL.\n\nThis particular case handled the _url_ just fine. The problem was in the\nautomatically generated name for local directory.\n"},{"id":"114131","messageId":"7veius6eve.fsf@alter.siamese.dyndns.org","threadId":"19303","inReplyTo":"alpine.LNX.2.00.0905131109240.2147@iabervon.org","subject":"Re: [PATCH] Quote LF in urls git fetch saves in FETCH_HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-13T18:23:49Z","receivedAt":"2009-05-13T18:23:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> On Wed, 13 May 2009, Alex Riesen wrote:\n>\n>> 2009/5/13 Junio C Hamano :\n>> > Alex Riesen writes:\n>> >\n>> >> + for (i = 0; i < url_len; ++i)\n>> >> + if ('\\n' == url[i])\n>> >> + fputs(\"\\\\n\", fp);\n>> >> + else\n>> >> + fputc(url[i], fp);\n>> >> + fputc('\\n', fp);\n>> >\n>> > This ad-hoc quoting feels _very_ wrong. Who is on the reading side and\n>> > how does it unquote?\n>> \n>> git fmt-merge-msg. It does not unquote. The url is purely informational here.\n>> OTOH, the \\n shouldn't be in url text at all, so treat it as slightly\n>> less annoying\n>> warning.\n>> \n>> > If it is just informational use only, then it might make more sense to\n>> > drop this ugly \"quoted \\n\" silently. I dunno.\n>> \n>> That'd mean to loose the information completely. Which is just as bad\n>> as putting the LF in the url in the first place.\n>\n> Looking back at the original message, it looks like the user included a \n> newline in an argument to clone, and the fetch must have stripped it out \n> (or ignored it in some other fashion), because data was retrieved from a \n> repository that doesn't have a newline in its name. Most likely, the \n> newline should just be prohibited in the URL in the config file in the \n> first place, and we shouldn't be able to get to the point of writing a \n> FETCH_HEAD with that value.\n\nLet me attempt to summarize the situation.\n\n . FETCH_HEAD is a LF terminated sequence of records; each record\n describes the commit object name, if it is meant to be merged, what URL\n and what ref it came from;\n\n . \"git pull\" reads from FETCH_HEAD to decide which commit objects to pass\n to underlying \"git merge\", and \"git fmt-merge-msg\" reads from\n FETCH_HEAD to decide what message to produce; and\n\n . \"git fetch\" allows a URL with an LF in it and fetches from such a URL\n just fine, but \"git pull\" barfed because \"git fmt-patch\" noticed.\n\nBecause we copy the LF in the URL straight to FETCH_HEAD, it breaks the\nfile format. Alex's proposal is to quote it as \"\\\\n\" to avoid it (I\nsuggested stripping it). Either change will _fix_ the situation in the\nsense that the file format will now be correct, \"git pull\" will extract\nthe right set of commits (as it does not look at the URL at all when\ndeciding which records are used for merge), and \"git fmt-merge-msg\" will\ngenerate correct set of branches and list of incoming commits without\nparse errors, even though the user won't be able to cut and paste the URL\nit records in its output either way (if stripped, it will be \"information\nloss\", but even if it were quoted, it won't be usable because nobody\nunquotes it).\n\nGiven all of the above, I think:\n\n (1) Alex's patch is a good minimum fix to the issue [*1*]. If unfixed,\n people will be able to fetch from but won't be able to pull from a\n URL with a LF in it.\n\n (2) Even though it seems that we do _support_ fetching from such a URL,\n it is not a good thing. People may do all sorts of crazy things, and\n this may be one of these crazy things that we _could_ break the\n backward compatibility to avoid innocent mistakes, by forbidding LF\n in URLs. Nobody could have been using such a URL in real settings\n because of the issue Alex's patch addresses anyway.\n\n (3) But forbidding or warning will be done by new code. Is the cost to\n do such a check (implementation and maintenance of the new code)\n justifiable? Where do we check and when?\n\nSo for now, I'd say I take Alex's patch as-is, and do nothing else.\n\nIf somebody has a compelling example that this kind of mistake is common\nand is hard to figure out why, we can explicitly forbid certain byte\nvalues in the repository URL as a separate step.\n\n\n[Footnote]\n\n*1* The output is purely informational, and especially its URL field is\nnot meant to be used as cut-and-paste source by general public anyway\n(e.g. you may pull using git over SSH but general public may not have an\nSSH access to the repository). I actually was tempted, when I did the\ninitial version of fmt-merge-msg, to shorten an overlong URL with\nellipses, even though I didn't.\n"},{"id":"113840","messageId":"20090513205333.GA21631@blimp.localdomain","threadId":"19303","inReplyTo":"20090513180853.GB26288@blimp.localdomain","subject":"[PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-13T20:53:33Z","receivedAt":"2009-05-13T20:53:33Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Strip trailing spaces off guessed target directory in builtin clone,\nand replace 'control' characters with an ASCII space.\n\nUser still can have any name by specifying it explicitely after url.\n\nSigned-off-by: Alex Riesen \n---\n\nThis should take care of accidental pastings inside shell quotes.\nAt least for the local part of the operation.\nNow I'm looking at the code and think I should have stripped the\nheading whitespace as well. It is much less likely to happen, though.\n\n builtin-clone.c | 24 ++++++++++++++++++------\n 1 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 880373f..bdbe931 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -104,11 +104,12 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n {\n \tconst char *end = repo + strlen(repo), *start;\n+\tchar *dir;\n \n \t/*\n-\t * Strip trailing slashes and /.git\n+\t * Strip trailing spaces, slashes and /.git\n \t */\n-\twhile (repo < end && is_dir_sep(end[-1]))\n+\twhile (repo < end && (is_dir_sep(end[-1]) || isspace(end[-1])))\n \t\tend--;\n \tif (end - repo > 5 && is_dir_sep(end[-5]) &&\n \t !strncmp(end - 4, \".git\", 4)) {\n@@ -140,10 +141,21 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n \tif (is_bare) {\n \t\tstruct strbuf result = STRBUF_INIT;\n \t\tstrbuf_addf(&result, \"%.*s.git\", (int)(end - start), start);\n-\t\treturn strbuf_detach(&result, 0);\n-\t}\n-\n-\treturn xstrndup(start, end - start);\n+\t\tdir = strbuf_detach(&result, 0);\n+\t} else\n+\t\tdir = xstrndup(start, end - start);\n+\t/* replace all 'control' characters with ascii space */\n+\tfor (start = dir; *start; ++start)\n+\t\tif (*(const unsigned char *)start < 32u)\n+\t\t\tdir[start - dir] = '\\x20';\n+\t/* remove trailing spaces */\n+\tif (dir < start)\n+\t\tfor (end = start; dir < --end; )\n+\t\t\tif (!isspace(*end))\n+\t\t\t\tbreak;\n+\t\t\telse\n+\t\t\t\tdir[end - dir] = '\\0';\n+\treturn dir;\n }\n \n static void strip_trailing_slashes(char *dir)\n-- \n1.6.3.28.ga852b\n"},{"id":"113859","messageId":"7vfxf81ppd.fsf@alter.siamese.dyndns.org","threadId":"19303","inReplyTo":"20090513205333.GA21631@blimp.localdomain","subject":"Re: [PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-14T00:41:02Z","receivedAt":"2009-05-14T00:41:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Strip trailing spaces off guessed target directory in builtin clone,\n> and replace 'control' characters with an ASCII space.\n>\n> User still can have any name by specifying it explicitely after url.\n>\n> Signed-off-by: Alex Riesen \n> ---\n>\n> This should take care of accidental pastings inside shell quotes.\n> At least for the local part of the operation.\n> Now I'm looking at the code and think I should have stripped the\n> heading whitespace as well. It is much less likely to happen, though.\n\n> @@ -140,10 +141,21 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n> \tif (is_bare) {\n> \t\tstruct strbuf result = STRBUF_INIT;\n> \t\tstrbuf_addf(&result, \"%.*s.git\", (int)(end - start), start);\n> +\t\tdir = strbuf_detach(&result, 0);\n> +\t} else\n> +\t\tdir = xstrndup(start, end - start);\n> +\t/* replace all 'control' characters with ascii space */\n> +\tfor (start = dir; *start; ++start)\n> +\t\tif (*(const unsigned char *)start < 32u)\n> +\t\t\tdir[start - dir] = '\\x20';\n\nWhat's this strange mixture of 32u and '\\x20'?\n\n> +\t/* remove trailing spaces */\n> +\tif (dir < start)\n> +\t\tfor (end = start; dir < --end; )\n> +\t\t\tif (!isspace(*end))\n> +\t\t\t\tbreak;\n> +\t\t\telse\n> +\t\t\t\tdir[end - dir] = '\\0';\n> +\treturn dir;\n> }\n\nHonestly, I regret having asked if there was a 2/2 ;-)\n\nWhat's the point of this change, now that you have a fix in 1/2? Who are\nyou helping with this patch?\n"},{"id":"113882","messageId":"81b0412b0905132254n5046666t24f3887f1fd4e4d9@mail.gmail.com","threadId":"19303","inReplyTo":"7vfxf81ppd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-14T05:54:34Z","receivedAt":"2009-05-14T05:54:34Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/14 Junio C Hamano :\n> Alex Riesen writes:\n>> +     /* replace all 'control' characters with ascii space */\n>> +     for (start = dir; *start; ++start)\n>> +             if (*(const unsigned char *)start < 32u)\n>> +                     dir[start - dir] = '\\x20';\n>\n> What's this strange mixture of 32u and '\\x20'?\n>\n\nNot sure myself. I probably wanted visibility, and somehow ended\nup using different presentations.\n\n>> +     /* remove trailing spaces */\n>> +     if (dir < start)\n>> +             for (end = start; dir < --end; )\n>> +                     if (!isspace(*end))\n>> +                             break;\n>> +                     else\n>> +                             dir[end - dir] = '\\0';\n>> +     return dir;\n>>  }\n>\n> Honestly, I regret having asked if there was a 2/2 ;-)\n>\n> What's the point of this change, now that you have a fix in 1/2?  Who are\n> you helping with this patch?\n>\n\nWithout this the _automatically_ generated names for cloned repositories\nhave all the whitespace around them. As you cannot sanely depend on\nautomatically\ngenerated names, I thought that making them simpler will make sense.\n\nBut I should complete the patch: remove heading whitespace, and replace\nmultiple spaces and control characters with one space.\n"},{"id":"113890","messageId":"7v8wl0xkcu.fsf@alter.siamese.dyndns.org","threadId":"19303","inReplyTo":"81b0412b0905132254n5046666t24f3887f1fd4e4d9@mail.gmail.com","subject":"Re: [PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-14T06:35:29Z","receivedAt":"2009-05-14T06:35:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n>> What's the point of this change, now that you have a fix in 1/2? Who are\n>> you helping with this patch?\n>\n> Without this the _automatically_ generated names for cloned repositories\n> have all the whitespace around them.\n\nEven if it has whitespace around its name, that's what you got from the\nupstream (a valid source of clone), and wasn't it you who said something\nabout UNIX tradition of allowing LF and others in the filename?\n\nIf clone reports \"ok we created this new repository\" so that the caller\ncan capture it, then the whole process should be able to cope with\nautomatically generated names with or without the patch, shouldn't it?\n\nOr are you trying to help a human user who gives a pathname ridden with\nexcess whitespaces to \"git clone\", and that pathname _happens_ to work as\na valid clone source, creating a new repository whose name is ridden with\nexcess whitespaces the same way as the input pathname? After all, the\nuser deliberately gave them to us, and the repository we cloned from had\nthese excesses in its name (iow, without the excess whitespaces the clone\nitself wouldn't have worked). In such a case, is it really helping him to\nremove these whitespaces as excesses?\n\nI do not understand the point of this patch and that is why I asked who\nyou are trying to help.\n"},{"id":"113908","messageId":"81b0412b0905140133l78def06kae0cb4798feaa4a9@mail.gmail.com","threadId":"19303","inReplyTo":"81b0412b0905132254n5046666t24f3887f1fd4e4d9@mail.gmail.com","subject":"Re: [PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-14T08:33:02Z","receivedAt":"2009-05-14T08:33:02Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Strip leading and trailing spaces off guessed target directory, and\nreplace sequences of whitespace and 'control' characters with one\nspace character.\n\nUser still can have any name by specifying it explicitely after url.\n\nSigned-off-by: Alex Riesen \n---\n\n2009/5/14 Alex Riesen :\n> But I should complete the patch: remove heading whitespace, and replace\n> multiple spaces and control characters with one space.\n\nDone.\n\n builtin-clone.c | 34 +++++++++++++++++++++++++++++-----\n 1 files changed, 29 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 880373f..d068b7e 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -104,11 +104,12 @@ static char *get_repo_path(const char *repo, int\n*is_bundle)\n static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n {\n \tconst char *end = repo + strlen(repo), *start;\n+\tchar *dir;\n\n \t/*\n-\t * Strip trailing slashes and /.git\n+\t * Strip trailing spaces, slashes and /.git\n \t */\n-\twhile (repo < end && is_dir_sep(end[-1]))\n+\twhile (repo < end && (is_dir_sep(end[-1]) || isspace(end[-1])))\n \t\tend--;\n \tif (end - repo > 5 && is_dir_sep(end[-5]) &&\n \t !strncmp(end - 4, \".git\", 4)) {\n@@ -140,10 +141,33 @@ static char *guess_dir_name(const char *repo,\nint is_bundle, int is_bare)\n \tif (is_bare) {\n \t\tstruct strbuf result = STRBUF_INIT;\n \t\tstrbuf_addf(&result, \"%.*s.git\", (int)(end - start), start);\n-\t\treturn strbuf_detach(&result, 0);\n+\t\tdir = strbuf_detach(&result, 0);\n+\t} else\n+\t\tdir = xstrndup(start, end - start);\n+\t/*\n+\t * Replace sequences of 'control' characters and whitespace\n+\t * with one ascii space, remove leading and trailing spaces.\n+\t */\n+\tif (*dir) {\n+\t\tchar *out = dir;\n+\t\tint prev_space = 1 /* strip leading whitespace */;\n+\t\tfor (end = dir; *end; ++end) {\n+\t\t\tchar ch = *end;\n+\t\t\tif ((unsigned char)ch < '\\x20')\n+\t\t\t\tch = '\\x20';\n+\t\t\tif (isspace(ch)) {\n+\t\t\t\tif (prev_space)\n+\t\t\t\t\tcontinue;\n+\t\t\t\tprev_space = 1;\n+\t\t\t} else\n+\t\t\t\tprev_space = 0;\n+\t\t\t*out++ = ch;\n+\t\t}\n+\t\t*out = '\\0';\n+\t\tif (out > dir && prev_space)\n+\t\t\tout[-1] = '\\0';\n \t}\n-\n-\treturn xstrndup(start, end - start);\n+\treturn dir;\n }\n\n static void strip_trailing_slashes(char *dir)\n-- \n1.6.3.1.37.g5d96e\n\n\nFrom 6d6075130afbdd35260e3e33c099f57d9faebe98 Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen \nDate: Wed, 13 May 2009 18:32:06 +0200\nSubject: [PATCH 2/2] Improve the naming of guessed target repository for git clone\n\nStrip leading and trailing spaces off guessed target directory, and\nreplace sequences of whitespace and 'control' characters with one\nspace character.\n\nUser still can have any name by specifying it explicitely after url.\n\nSigned-off-by: Alex Riesen \n---\n builtin-clone.c | 34 +++++++++++++++++++++++++++++-----\n 1 files changed, 29 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 880373f..d068b7e 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -104,11 +104,12 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n {\n \tconst char *end = repo + strlen(repo), *start;\n+\tchar *dir;\n \n \t/*\n-\t * Strip trailing slashes and /.git\n+\t * Strip trailing spaces, slashes and /.git\n \t */\n-\twhile (repo < end && is_dir_sep(end[-1]))\n+\twhile (repo < end && (is_dir_sep(end[-1]) || isspace(end[-1])))\n \t\tend--;\n \tif (end - repo > 5 && is_dir_sep(end[-5]) &&\n \t !strncmp(end - 4, \".git\", 4)) {\n@@ -140,10 +141,33 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)\n \tif (is_bare) {\n \t\tstruct strbuf result = STRBUF_INIT;\n \t\tstrbuf_addf(&result, \"%.*s.git\", (int)(end - start), start);\n-\t\treturn strbuf_detach(&result, 0);\n+\t\tdir = strbuf_detach(&result, 0);\n+\t} else\n+\t\tdir = xstrndup(start, end - start);\n+\t/*\n+\t * Replace sequences of 'control' characters and whitespace\n+\t * with one ascii space, remove leading and trailing spaces.\n+\t */\n+\tif (*dir) {\n+\t\tchar *out = dir;\n+\t\tint prev_space = 1 /* strip leading whitespace */;\n+\t\tfor (end = dir; *end; ++end) {\n+\t\t\tchar ch = *end;\n+\t\t\tif ((unsigned char)ch < '\\x20')\n+\t\t\t\tch = '\\x20';\n+\t\t\tif (isspace(ch)) {\n+\t\t\t\tif (prev_space)\n+\t\t\t\t\tcontinue;\n+\t\t\t\tprev_space = 1;\n+\t\t\t} else\n+\t\t\t\tprev_space = 0;\n+\t\t\t*out++ = ch;\n+\t\t}\n+\t\t*out = '\\0';\n+\t\tif (out > dir && prev_space)\n+\t\t\tout[-1] = '\\0';\n \t}\n-\n-\treturn xstrndup(start, end - start);\n+\treturn dir;\n }\n \n static void strip_trailing_slashes(char *dir)\n-- \n1.6.3.1.37.g5d96e\n\n"},{"id":"113909","messageId":"81b0412b0905140145i4093e742x9f8c762e2bf32624@mail.gmail.com","threadId":"19303","inReplyTo":"7v8wl0xkcu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-14T08:45:11Z","receivedAt":"2009-05-14T08:45:11Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/14 Junio C Hamano :\n> Alex Riesen writes:\n>\n>>> What's the point of this change, now that you have a fix in 1/2?  Who are\n>>> you helping with this patch?\n>>\n>> Without this the _automatically_ generated names for cloned repositories\n>> have all the whitespace around them.\n>\n> Even if it has whitespace around its name, that's what you got from the\n> upstream (a valid source of clone), and wasn't it you who said something\n> about UNIX tradition of allowing LF and others in the filename?\n\nYes, when user explicitely asked a program about that. This here\n(clone with only URL as argument) is not the case, I think.\n\n> If clone reports \"ok we created this new repository\" so that the caller\n> can capture it, then the whole process should be able to cope with\n> automatically generated names with or without the patch, shouldn't it?\n\nNo, don't think so. You're not always able to capture the output of git clone\n(Windows again), and BTW - init-db output is not designed to be captured\nunambiguously.\n\n> Or are you trying to help a human user who gives a pathname ridden with\n> excess whitespaces to \"git clone\", and that pathname _happens_ to work as\n> a valid clone source, creating a new repository whose name is ridden with\n> excess whitespaces the same way as the input pathname?\n\nNot really. I just try to make the _generated_ output, which the user cannot\npredict anyway (nor does the user care much about it) to be less\nproblematic. Yes, I did say that LF-anything in UNIX filenames is a normal\nthing. That does not mean that such names are so very convenient to use.\nThey do cause problems, even if just through scrambling terminal output.\nThey are \"inconvenient\". If our users don't expect precise output anyway,\nwe can be a little more adhering to usual practices in choosing names.\n\n> ... After all, the\n> user deliberately gave them to us, and the repository we cloned from had\n> these excesses in its name (iow, without the excess whitespaces the clone\n> itself wouldn't have worked).  In such a case, is it really helping him to\n> remove these whitespaces as excesses?\n\nI think yes. Otherwise the strict form of git clone could have been used,\nwhich involves absolutely no guessing and mangling.\n"},{"id":"113925","messageId":"200905141450.13057.Hugo.Mildenberger@namir.de","threadId":"19303","inReplyTo":"81b0412b0905140145i4093e742x9f8c762e2bf32624@mail.gmail.com","subject":"Re: [PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Hugo Mildenberger","fromEmail":"hugo.mildenberger@namir.de","sentAt":"2009-05-14T12:50:10Z","receivedAt":"2009-05-14T12:50:10Z","isPatch":true,"sender":{"key":"hugo.mildenberger@namir.de","avatar":null},"body":"Am Donnerstag, 14. Mai 2009 schrieb Alex Riesen:\n> 2009/5/14 Junio C Hamano :\n> > Alex Riesen writes:\n> >\n> >>> What's the point of this change, now that you have a fix in 1/2?  Who are\n> >>> you helping with this patch?\n> >>\n> >> Without this the _automatically_ generated names for cloned repositories\n> >> have all the whitespace around them.\n> >\n> > Even if it has whitespace around its name, that's what you got from the\n> > upstream (a valid source of clone), and wasn't it you who said something\n> > about UNIX tradition of allowing LF and others in the filename?\n> \n> Yes, when user explicitely asked a program about that. This here\n> (clone with only URL as argument) is not the case, I think.\n> \n> > If clone reports \"ok we created this new repository\" so that the caller\n> > can capture it, then the whole process should be able to cope with\n> > automatically generated names with or without the patch, shouldn't it?\n> \n> No, don't think so. You're not always able to capture the output of git clone\n> (Windows again), and BTW - init-db output is not designed to be captured\n> unambiguously.\n> \n> > Or are you trying to help a human user who gives a pathname ridden with\n> > excess whitespaces to \"git clone\", and that pathname _happens_ to work as\n> > a valid clone source, creating a new repository whose name is ridden with\n> > excess whitespaces the same way as the input pathname?\n> \n> Not really. I just try to make the _generated_ output, which the user cannot\n> predict anyway (nor does the user care much about it) to be less\n> problematic. Yes, I did say that LF-anything in UNIX filenames is a normal\n> thing. That does not mean that such names are so very convenient to use.\n> They do cause problems, even if just through scrambling terminal output.\n> They are \"inconvenient\". If our users don't expect precise output anyway,\n> we can be a little more adhering to usual practices in choosing names.\n> \n> > ... After all, the\n> > user deliberately gave them to us, and the repository we cloned from had\n> > these excesses in its name (iow, without the excess whitespaces the clone\n> > itself wouldn't have worked).  In such a case, is it really helping him to\n> > remove these whitespaces as excesses?\n> \n> I think yes. Otherwise the strict form of git clone could have been used,\n> which involves absolutely no guessing and mangling.\n> \nThis discussion began with the problem I encountered during a git clone \noperation applied to an url which accidentally included a linefeed. It partly \nwent through although the remote side did not include that character. \nAfter Alex Riesen fixed the git part by escaping linefeeds, the linefeed \nleft over in the top level directory name still caused a linux kernel \nmake to fail (which really is not problem of git or GNU make itself). \nElsewhere in this thread, Daniel Barkalow proposed to apply the HTTP \nRFC to the whole url. If accepted, this would cope with the problem in a \nstandard way. \n"},{"id":"114108","messageId":"7vbpptndjg.fsf@alter.siamese.dyndns.org","threadId":"19303","inReplyTo":"81b0412b0905140133l78def06kae0cb4798feaa4a9@mail.gmail.com","subject":"Re: [PATCH 2/2] Improve the naming of guessed target repository for git clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-16T17:49:39Z","receivedAt":"2009-05-16T17:49:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Strip leading and trailing spaces off guessed target directory, and\n> replace sequences of whitespace and 'control' characters with one\n> space character.\n>\n> User still can have any name by specifying it explicitely after url.\n>\n> Signed-off-by: Alex Riesen \n\nThanks, will queue.\n"}]}