{"thread":{"id":"30573","subject":"git rev-list %an, %ae, %at bug in v1.7.10.1 and beyond","startedAt":"2012-05-21T08:01:50Z","lastAt":"2012-05-22T06:13:28Z","messageCount":6,"participants":["Bryan Turner","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"191789","messageId":"CAGyf7-G3nNTTP1bKdd9HLKEn-8+LoxCeY2R08x9gKZwS0L_N6g@mail.gmail.com","threadId":"30573","inReplyTo":null,"subject":"git rev-list %an, %ae, %at bug in v1.7.10.1 and beyond","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2012-05-21T08:01:50Z","receivedAt":"2012-05-21T08:01:50Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"Hello all,\n\nI believe I've found an issue in git v1.7.10.1 and v1.7.10.2 (and\nmaster) where the output of git rev-list has changed for some commits.\nMost commits do not appear to trigger the issue; it may be related to\nUnicode characters being used in the author name. I've run a git\nbisect and it appears the bug was introduced in\n4b340cfab9c7a18e39bc531d6a6ffaffdf95f62d.\n\nBuilt from master (as well as using the 1.7.10.1 and 1.7.10.2 release\ntags), I get output like this:\n================================ Output =====================================\naphrael:qa-resources.git bturner$ git rev-list\n--format=\"%H|%h|%P|%p|%an|%ae|%at%n%B%n@@object_end@@\" -1\n5c1ccdec5f84aa149a4978f729fdda70769f942f\ncommit 5c1ccdec5f84aa149a4978f729fdda70769f942f\n5c1ccdec5f84aa149a4978f729fdda70769f942f|5c1ccde|02c78bc39ac6192623bf080e3e2ac892a4f5764c|02c78bc|||\ncommit with unicode name\n\n@@object_end@@\n================================ End ========================================\n\nNote that the author name, e-mail and timestamp values are all missing\n(the three |'s in a row at the end).\n\nBuilt from 0dbe6592ccbd1a394a69a52074e3729d546fe952, the parent of\n4b340cf, and in previous versions of git (1.7.10 and earlier), I got\noutput like this:\n================================ Output =====================================\naphrael:qa-resources.git bturner$ git rev-list\n--format=\"%H|%h|%P|%p|%an|%ae|%at%n%B%n@@object_end@@\" -1\n5c1ccdec5f84aa149a4978f729fdda70769f942f\ncommit 5c1ccdec5f84aa149a4978f729fdda70769f942f\n5c1ccdec5f84aa149a4978f729fdda70769f942f|5c1ccde|02c78bc39ac6192623bf080e3e2ac892a4f5764c|02c78bc|a|farmas@atlassian.com|1327876222\ncommit with unicode name\n\n@@object_end@@\n================================ End ========================================\n\nNote that the author name, e-mail and timestamp are all present. The\n\"a\" appears as ASCII here, but it's actually a UTF-16LE character (the\nterminal on the Mac is being \"helpful\").\n\nTo try and verify whether the difference is a bug or a bugfix (because\nI wasn't certain whether perhaps the output from earlier versions was\nthe result of a bug which was fixed in 1.7.10.1 and on), I compared\nthe git rev-list output with git cat-file -p (again, built from\nmaster):\n================================ Output =====================================\naphrael:qa-resources.git bturner$ git cat-file -p\n5c1ccdec5f84aa149a4978f729fdda70769f942f\ntree dd173cb70baaac07bdf405f4e3db110e7fafd180\nparent 02c78bc39ac6192623bf080e3e2ac892a4f5764c\nauthor a <farmas@atlassian.com> 1327876222 +1100\ncommitter a <farmas@atlassian.com> 1327876222 +1100\n\ncommit with unicode name\n================================ End ========================================\n\nThe git cat-file output is consistent across versions of git where I'm\nseeing the rev-list issue and versions where I'm not.\n\nI would be happy to provide a zip file containing the repository with\nthe commit shown in all the output above, if that will facilitate\ntesting/fixing the issue. Just let me know where to put it. I lack the\nC chops to provide a patch myself; sorry about that.\n\nBest regards,\nBryan Turner\n"},{"id":"191869","messageId":"20120522043221.GA6859@sigill.intra.peff.net","threadId":"30573","inReplyTo":"CAGyf7-G3nNTTP1bKdd9HLKEn-8+LoxCeY2R08x9gKZwS0L_N6g@mail.gmail.com","subject":"Re: git rev-list %an, %ae, %at bug in v1.7.10.1 and beyond","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-22T04:32:21Z","receivedAt":"2012-05-22T04:32:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 21, 2012 at 06:01:50PM +1000, Bryan Turner wrote:\n\n> Note that the author name, e-mail and timestamp values are all missing\n> (the three |'s in a row at the end).\n> [...]\n> Built from 0dbe6592ccbd1a394a69a52074e3729d546fe952, the parent of\n> 4b340cf, and in previous versions of git (1.7.10 and earlier), I got\n> output like this:\n> [...]\n> Note that the author name, e-mail and timestamp are all present. The\n> \"a\" appears as ASCII here, but it's actually a UTF-16LE character (the\n> terminal on the Mac is being \"helpful\").\n\nI'm not too surprised this is broken (in fact, I'm surprised it ever\nreally worked). UTF-16, especially representing pure ascii characters,\nwill have embedded NULs. Most of the code assumes that things like names\nand emails are NUL-terminated and ascii-compatible (so ascii, or some\nascii-superset encoding like utf8, iso8859-1, etc). You can store a\ncommit message (and name) in utf-16 if you tell git that you are doing\nso, but we should be re-encoding it before handling it.\n\n> ================================ Output =====================================\n> aphrael:qa-resources.git bturner$ git cat-file -p\n> 5c1ccdec5f84aa149a4978f729fdda70769f942f\n> tree dd173cb70baaac07bdf405f4e3db110e7fafd180\n> parent 02c78bc39ac6192623bf080e3e2ac892a4f5764c\n> author a <farmas@atlassian.com> 1327876222 +1100\n> committer a <farmas@atlassian.com> 1327876222 +1100\n> \n> commit with unicode name\n> ================================ End ========================================\n\nThere's no encoding header, so having a utf-16 character there is wrong.\nHow did you make such a commit in the first place, though? I believe\nthat git-commit treats the name as a string and would terminate on a NUL\n(or am I wrong in thinking that this \"a\" is really U+0061, and is\nactually some other unicode character that _looks_ like \"a\", but doesn't\ncontain a NUL?).\n\nDid you create it by piping straight to git-hash-object? What does\npiping the above through \"xxd\" or \"cat -A\" show?\n\n-Peff\n"},{"id":"191870","messageId":"20120522043509.GB6859@sigill.intra.peff.net","threadId":"30573","inReplyTo":"20120522043221.GA6859@sigill.intra.peff.net","subject":"Re: git rev-list %an, %ae, %at bug in v1.7.10.1 and beyond","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-22T04:35:09Z","receivedAt":"2012-05-22T04:35:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 22, 2012 at 12:32:21AM -0400, Jeff King wrote:\n\n> I'm not too surprised this is broken (in fact, I'm surprised it ever\n> really worked). UTF-16, especially representing pure ascii characters,\n> will have embedded NULs. Most of the code assumes that things like names\n> and emails are NUL-terminated and ascii-compatible (so ascii, or some\n> ascii-superset encoding like utf8, iso8859-1, etc). You can store a\n> commit message (and name) in utf-16 if you tell git that you are doing\n> so, but we should be re-encoding it before handling it.\n\nActually, I take that back. I think storing in utf-16 would probably\nfail. We need to use ascii to even read the headers to get to the\nencoding header to realize that it is in utf-16. So I believe we really\ndo only support ascii-superset encodings.\n\n-Peff\n"},{"id":"191873","messageId":"CAGyf7-Gj8YynENrnkZMT4WSEDtYpmUL5qaS4TEuvfaa_YcXxOw@mail.gmail.com","threadId":"30573","inReplyTo":"20120522043509.GB6859@sigill.intra.peff.net","subject":"Re: git rev-list %an, %ae, %at bug in v1.7.10.1 and beyond","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2012-05-22T05:13:09Z","receivedAt":"2012-05-22T05:13:09Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"Peff,\n\nThanks for taking the time to respond! I appreciate it.\n\nI went and spoke to the tester who made the original commit, and it\nsounds like what he did was open .git/config with a text editor and\nupdate the user.name directly. He then saved the file. His operating\nsystem's code page was UTF-16LE, so I believe the file was saved in\nthat format. He then did the commit.\n\nThat sort of behavior is pretty far outside the scope of anything git\ncan realistically be expected to handle (and I told the tester that as\nwell). That said, I'm interested that it \"suddenly\" broke, and also\nthat cat-file still appears to not think there's any problem.\n\nI piped the output through xxd, for cat-file and rev-list. The\ncat-file output is the same in both versions of git, and looks like\nthis:\nauri:qa-resources.git bturner$ git cat-file -p\n5c1ccdec5f84aa149a4978f729fdda70769f942f | xxd\n0000000: 7472 6565 2064 6431 3733 6362 3730 6261  tree dd173cb70ba\n0000010: 6161 6330 3762 6466 3430 3566 3465 3364  aac07bdf405f4e3d\n0000020: 6231 3130 6537 6661 6664 3138 300a 7061  b110e7fafd180.pa\n0000030: 7265 6e74 2030 3263 3738 6263 3339 6163  rent 02c78bc39ac\n0000040: 3631 3932 3632 3362 6630 3830 6533 6532  6192623bf080e3e2\n0000050: 6163 3839 3261 3466 3537 3634 630a 6175  ac892a4f5764c.au\n0000060: 7468 6f72 2061 203c 6661 726d 6173 4061  thor a <farmas@a\n0000070: 746c 6173 7369 616e 2e63 6f6d 3e20 3133  tlassian.com> 13\n0000080: 3237 3837 3632 3232 202b 3131 3030 0a63  27876222 +1100.c\n0000090: 6f6d 6d69 7474 6572 2061 203c 6661 726d  ommitter a <farm\n00000a0: 6173 4061 746c 6173 7369 616e 2e63 6f6d  as@atlassian.com\n00000b0: 3e20 3133 3237 3837 3632 3232 202b 3131  > 1327876222 +11\n00000c0: 3030 0a0a 636f 6d6d 6974 2077 6974 6820  00..commit with\n00000d0: 756e 6963 6f64 6520 6e61 6d65 0a         unicode name.\n\nIn 1.7.9.5, rev-list looks like this:\nauri:qa-resources.git bturner$ git rev-list\n--format=\"%H|%h|%P|%p|%an|%ae|%at%n%B%n@@object_end@@\" -1\n5c1ccdec5f84aa149a4978f729fdda70769f942f | xxd\n0000000: 636f 6d6d 6974 2035 6331 6363 6465 6335  commit 5c1ccdec5\n0000010: 6638 3461 6131 3439 6134 3937 3866 3732  f84aa149a4978f72\n0000020: 3966 6464 6137 3037 3639 6639 3432 660a  9fdda70769f942f.\n0000030: 3563 3163 6364 6563 3566 3834 6161 3134  5c1ccdec5f84aa14\n0000040: 3961 3439 3738 6637 3239 6664 6461 3730  9a4978f729fdda70\n0000050: 3736 3966 3934 3266 7c35 6331 6363 6465  769f942f|5c1ccde\n0000060: 7c30 3263 3738 6263 3339 6163 3631 3932  |02c78bc39ac6192\n0000070: 3632 3362 6630 3830 6533 6532 6163 3839  623bf080e3e2ac89\n0000080: 3261 3466 3537 3634 637c 3032 6337 3862  2a4f5764c|02c78b\n0000090: 637c 617c 6661 726d 6173 4061 746c 6173  c|a|farmas@atlas\n00000a0: 7369 616e 2e63 6f6d 7c31 3332 3738 3736  sian.com|1327876\n00000b0: 3232 320a 636f 6d6d 6974 2077 6974 6820  222.commit with\n00000c0: 756e 6963 6f64 6520 6e61 6d65 0a0a 4040  unicode name..@@\n00000d0: 6f62 6a65 6374 5f65 6e64 4040 0a         object_end@@.\n\nIn 1.7.10.2, it looks like this:\nauri:qa-resources.git bturner$ git rev-list\n--format=\"%H|%h|%P|%p|%an|%ae|%at%n%B%n@@object_end@@\" -1\n5c1ccdec5f84aa149a4978f729fdda70769f942f | xxd\n0000000: 636f 6d6d 6974 2035 6331 6363 6465 6335  commit 5c1ccdec5\n0000010: 6638 3461 6131 3439 6134 3937 3866 3732  f84aa149a4978f72\n0000020: 3966 6464 6137 3037 3639 6639 3432 660a  9fdda70769f942f.\n0000030: 3563 3163 6364 6563 3566 3834 6161 3134  5c1ccdec5f84aa14\n0000040: 3961 3439 3738 6637 3239 6664 6461 3730  9a4978f729fdda70\n0000050: 3736 3966 3934 3266 7c35 6331 6363 6465  769f942f|5c1ccde\n0000060: 7c30 3263 3738 6263 3339 6163 3631 3932  |02c78bc39ac6192\n0000070: 3632 3362 6630 3830 6533 6532 6163 3839  623bf080e3e2ac89\n0000080: 3261 3466 3537 3634 637c 3032 6337 3862  2a4f5764c|02c78b\n0000090: 637c 7c7c 0a63 6f6d 6d69 7420 7769 7468  c|||.commit with\n00000a0: 2075 6e69 636f 6465 206e 616d 650a 0a40   unicode name..@\n00000b0: 406f 626a 6563 745f 656e 6440 400a       @object_end@@.\n\nNone of the output indicates a NUL is present. I'm wondering if the\nold code that rev-list was running (and maybe the code cat-file\nappears to still be running) might be ignoring leading NULs until it\nfinds the first character, or something similar. If that were the\ncase, had the tester used a name like \"ab\" (or anything more than one\ncharacter), perhaps the commit would have caused unexpected behavior\nall along.\n\nI don't know how to dump the binary for the commit object itself; that\nmay give us better information on what the author segment actually\ncontains. Unfortunately, the repository has been packed, so there is\nno loose object for the commit anymore.\n\nIf there's any other information I can provide, please let me know.\n\nBest regards,\nBryan Turner\n\nOn 22 May 2012 14:35, Jeff King <peff@peff.net> wrote:\n> On Tue, May 22, 2012 at 12:32:21AM -0400, Jeff King wrote:\n>\n>> I'm not too surprised this is broken (in fact, I'm surprised it ever\n>> really worked). UTF-16, especially representing pure ascii characters,\n>> will have embedded NULs. Most of the code assumes that things like names\n>> and emails are NUL-terminated and ascii-compatible (so ascii, or some\n>> ascii-superset encoding like utf8, iso8859-1, etc). You can store a\n>> commit message (and name) in utf-16 if you tell git that you are doing\n>> so, but we should be re-encoding it before handling it.\n>\n> Actually, I take that back. I think storing in utf-16 would probably\n> fail. We need to use ascii to even read the headers to get to the\n> encoding header to realize that it is in utf-16. So I believe we really\n> do only support ascii-superset encodings.\n>\n> -Peff\n"},{"id":"191878","messageId":"20120522055811.GA10597@sigill.intra.peff.net","threadId":"30573","inReplyTo":"CAGyf7-Gj8YynENrnkZMT4WSEDtYpmUL5qaS4TEuvfaa_YcXxOw@mail.gmail.com","subject":"Re: git rev-list %an, %ae, %at bug in v1.7.10.1 and beyond","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-22T05:58:11Z","receivedAt":"2012-05-22T05:58:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 22, 2012 at 03:13:09PM +1000, Bryan Turner wrote:\n\n> I went and spoke to the tester who made the original commit, and it\n> sounds like what he did was open .git/config with a text editor and\n> update the user.name directly. He then saved the file. His operating\n> system's code page was UTF-16LE, so I believe the file was saved in\n> that format. He then did the commit.\n\nThat doesn't make sense. Git will not read a config file in utf16\n(again, because of the NULs). Nor can you store NULs with \"git config\nuser.name ...\". So I'm not sure how utf16 might have made it there.\n\n> That sort of behavior is pretty far outside the scope of anything git\n> can realistically be expected to handle (and I told the tester that as\n> well). That said, I'm interested that it \"suddenly\" broke, and also\n> that cat-file still appears to not think there's any problem.\n\nYeah, I'm worried that it's indicative of a bug that might affect other\ncases.\n\n> I piped the output through xxd, for cat-file and rev-list. The\n> cat-file output is the same in both versions of git, and looks like\n> this:\n> auri:qa-resources.git bturner$ git cat-file -p\n> 5c1ccdec5f84aa149a4978f729fdda70769f942f | xxd\n> 0000000: 7472 6565 2064 6431 3733 6362 3730 6261  tree dd173cb70ba\n> 0000010: 6161 6330 3762 6466 3430 3566 3465 3364  aac07bdf405f4e3d\n> 0000020: 6231 3130 6537 6661 6664 3138 300a 7061  b110e7fafd180.pa\n> 0000030: 7265 6e74 2030 3263 3738 6263 3339 6163  rent 02c78bc39ac\n> 0000040: 3631 3932 3632 3362 6630 3830 6533 6532  6192623bf080e3e2\n> 0000050: 6163 3839 3261 3466 3537 3634 630a 6175  ac892a4f5764c.au\n> 0000060: 7468 6f72 2061 203c 6661 726d 6173 4061  thor a <farmas@a\n> 0000070: 746c 6173 7369 616e 2e63 6f6d 3e20 3133  tlassian.com> 13\n> 0000080: 3237 3837 3632 3232 202b 3131 3030 0a63  27876222 +1100.c\n> 0000090: 6f6d 6d69 7474 6572 2061 203c 6661 726d  ommitter a <farm\n> 00000a0: 6173 4061 746c 6173 7369 616e 2e63 6f6d  as@atlassian.com\n> 00000b0: 3e20 3133 3237 3837 3632 3232 202b 3131  > 1327876222 +11\n> 00000c0: 3030 0a0a 636f 6d6d 6974 2077 6974 6820  00..commit with\n> 00000d0: 756e 6963 6f64 6520 6e61 6d65 0a         unicode name.\n\nBut I don't see any unicode at all here. The author and committer names\nare just ascii 'a'. However, that seems to be the real problem. If I\ncreate a commit with a single-letter name, I see some weirdness:\n\n  $ git init\n  $ echo content >foo && git add foo\n  $ GIT_AUTHOR_NAME=a git commit -m msg\n   Author:  <>\n   1 file changed, 1 insertion(+)\n   create mode 100644 foo\n\nUh oh, that's odd. And worse:\n\n  $ git log -1 --format='|%an <%ae>|'\n  | <>|\n\nBut in v1.7.10:\n\n  $ git log -1 --format='|%an <%ae>|'\n  |a <peff@peff.net>|\n\nSo there is definitely a bug. The unicode thing is a red herring, and if\nthere was any unicode at some point, git stripped it out when making the\ncommit. The real regression seems to be in single-character names.\n\nI'll see if I can find the bug.\n\n-Peff\n"},{"id":"191882","messageId":"20120522061328.GA17896@sigill.intra.peff.net","threadId":"30573","inReplyTo":"20120522055811.GA10597@sigill.intra.peff.net","subject":"Re: git rev-list %an, %ae, %at bug in v1.7.10.1 and beyond","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-22T06:13:28Z","receivedAt":"2012-05-22T06:13:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 22, 2012 at 01:58:11AM -0400, Jeff King wrote:\n\n>   $ git init\n>   $ echo content >foo && git add foo\n>   $ GIT_AUTHOR_NAME=a git commit -m msg\n>    Author:  <>\n>    1 file changed, 1 insertion(+)\n>    create mode 100644 foo\n> \n> Uh oh, that's odd. And worse:\n> \n>   $ git log -1 --format='|%an <%ae>|'\n>   | <>|\n> \n> So there is definitely a bug. The unicode thing is a red herring, and if\n> there was any unicode at some point, git stripped it out when making the\n> commit. The real regression seems to be in single-character names.\n> \n> I'll see if I can find the bug.\n\nBoth are caused by an off-by-one error in split_ident_line. I just\nposted a patch.\n\n-Peff\n"}]}