{"thread":{"id":"32980","subject":"Crashes while trying to show tag objects with bad timestamps","startedAt":"2013-02-22T22:30:28Z","lastAt":"2013-02-25T19:33:28Z","messageCount":16,"participants":["Mantas Mikulėnas","Jeff King","Junio C Hamano","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"210064","messageId":"kg8ri2$vjb$1@ger.gmane.org","threadId":"32980","inReplyTo":null,"subject":"Crashes while trying to show tag objects with bad timestamps","fromName":"Mantas Mikulėnas","fromEmail":"grawity@gmail.com","sentAt":"2013-02-22T22:30:28Z","receivedAt":"2013-02-22T22:30:28Z","isPatch":false,"sender":{"key":"grawity@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31021?v=4"},"body":"When messing around with various repositories, I noticed that git 1.8\n(currently using 1.8.2.rc0.22.gb3600c3) has problems parsing tag objects\nthat have invalid timestamps.\n\nTimes in tag objects appear to be kept as Unix timestamps, but I didn't\nrealize this at first, and ran something roughly equivalent to:\n git cat-file -p $tagname | git hash-object -w -t tag --stdin\ncreating a tag object the \"tagger\" line containing formatted time\ninstead of a Unix timestamp.\n\nGit doesn't handle the resulting tag objects nicely at all. For example,\nrunning `git cat-file -p` on the new object outputs a really odd\ntimestamp \"Thu Jun Thu Jan 1 00:16:09 1970 +0016\" (I'm guessing it\nparses the year as Unix time), and `git show` outright crashes\n(backtrace included below.)\n\nI would have expected both commands to print a \"tag object corrupt\"\nmessage, or maybe even a more specific \"bad timestamp in tagger line\"...\n\nTo reproduce:\n\nprintf '%s\\n' \\\n 'object 4b825dc642cb6eb9a060e54bf8d69288fbee4904' 'type tree' \\\n 'tag test' 'tagger User Thu Jun 9 16:44:04 2005 +0000' \\\n '' 'Test tag' | git hash-object -w -t tag --stdin | xargs git show\n\n\n> #0 0x00007f42560bb5f3 in ____strtoull_l_internal () from /usr/lib/libc.so.6\n> No symbol table info available.\n> #1 0x00000000004b4c81 in pp_user_info (pp=pp@entry=0x7fff3c30f1a0, \n> what=what@entry=0x50c13b \"Tagger\", sb=sb@entry=0x7fff3c30f140, \n> line=0xc267c7 \"Jilles Tjoelker Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\", encoding=0x507e20 \"UTF-8\") at pretty.c:431\n> name = {alloc = 24, len = 15, buf = 0xc24690 \"Jilles Tjoelker\"}\n> mail = {alloc = 24, len = 15, buf = 0xc24750 \"jilles@stack.nl\"}\n> ident = {\n> name_begin = 0xc267c7 \"Jilles Tjoelker Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\", \n> name_end = 0xc267d6 \" Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\", \n> mail_begin = 0xc267d8 \"jilles@stack.nl> Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\", mail_end = 0xc267e7 \"> Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\", \n> date_begin = 0x0, date_end = 0x0, tz_begin = 0x0, tz_end = 0x0}\n> linelen = \n> line_end = \n> date = \n> mailbuf = 0xc267d8 \"jilles@stack.nl> Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\"\n> namebuf = 0xc267c7 \"Jilles Tjoelker Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\"\n> namelen = 33\n> maillen = 15\n> max_length = 78\n> time = \n> tz = \n> #2 0x0000000000439af5 in show_tagger (buf=, len=, \n> rev=) at builtin/log.c:400\n> pp = {fmt = CMIT_FMT_MEDIUM, abbrev = 0, subject = 0x0, after_subject = 0x0, \n> preserve_subject = 0, date_mode = DATE_NORMAL, date_mode_explicit = 0, \n> need_8bit_cte = 0, notes_message = 0x0, reflog_info = 0x0, \n> output_encoding = 0x0, mailmap = 0x0, color = 0}\n> out = {alloc = 0, len = 0, buf = 0x7a8188 \"\"}\n> #3 show_tag_object (rev=0x7fff3c30f1f0, \n> sha1=0xc2be44 \"\\230\\211\\275\\331\\365Q\\306z\\017\\071d\\331\\035\\062\\247a\\347~M8P\", ) at builtin/log.c:427\n> new_offset = 151\n> type = OBJ_TAG\n> buf = 0xc26770 \"object ffa28d13e40e03bd367d0219c7eb516be0f180d2\\ntype commit\\ntag hyperion-1.0rc1\\ntagger Jilles Tjoelker Thu Jun 9 16:44:04 2005 +0000\\n\\nTag 1.0rc1.\\n\\n\"\n> size = 165\n> offset = \n\n\n-- \nMantas Mikulėnas \n"},{"id":"210067","messageId":"20130222224655.GB21579@sigill.intra.peff.net","threadId":"32980","inReplyTo":"kg8ri2$vjb$1@ger.gmane.org","subject":"Re: Crashes while trying to show tag objects with bad timestamps","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-22T22:46:55Z","receivedAt":"2013-02-22T22:46:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 23, 2013 at 12:30:28AM +0200, Mantas Mikulėnas wrote:\n\n> When messing around with various repositories, I noticed that git 1.8\n> (currently using 1.8.2.rc0.22.gb3600c3) has problems parsing tag objects\n> that have invalid timestamps.\n> \n> Times in tag objects appear to be kept as Unix timestamps, but I didn't\n> realize this at first, and ran something roughly equivalent to:\n> git cat-file -p $tagname | git hash-object -w -t tag --stdin\n> creating a tag object the \"tagger\" line containing formatted time\n> instead of a Unix timestamp.\n\nThanks, that makes it easy to replicate. It looks like it is not just\ntags, but rather the pp_user_info function does not realize that\nsplit_ident may return NULL for the date field if it is unparseable.\nSomething like this stops the crash and just gives a bogus date:\n\ndiff --git a/pretty.c b/pretty.c\nindex eae57ad..9688857 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -428,8 +428,16 @@ void pp_user_info(const struct pretty_print_context *pp,\n \tstrbuf_add(&name, namebuf, namelen);\n \n \tnamelen = name.len + mail.len + 3; /* ' ' + '<' + '>' */\n-\ttime = strtoul(ident.date_begin, &date, 10);\n-\ttz = strtol(date, NULL, 10);\n+\n+\tif (ident.date_begin) {\n+\t\ttime = strtoul(ident.date_begin, &date, 10);\n+\t\ttz = strtol(date, NULL, 10);\n+\t}\n+\telse {\n+\t\t/* ident line had malformed date */\n+\t\ttime = 0;\n+\t\ttz = 0;\n+\t}\n \n \tif (pp->fmt == CMIT_FMT_EMAIL) {\n \t\tstrbuf_addstr(sb, \"From: \");\n\nI guess we should probably issue a warning, too. Also disappointingly,\ngit-fsck does not seem to detect this breakage at all.\n\n> Git doesn't handle the resulting tag objects nicely at all. For example,\n> running `git cat-file -p` on the new object outputs a really odd\n> timestamp \"Thu Jun Thu Jan 1 00:16:09 1970 +0016\" (I'm guessing it\n> parses the year as Unix time), and `git show` outright crashes\n> (backtrace included below.)\n\nIf \"cat-file -p\" is not using the usual pretty-print routines, it\nprobably should. I'll take a look.\n\n-Peff\n"},{"id":"210068","messageId":"7vy5egark3.fsf@alter.siamese.dyndns.org","threadId":"32980","inReplyTo":"20130222224655.GB21579@sigill.intra.peff.net","subject":"Re: Crashes while trying to show tag objects with bad timestamps","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-22T22:53:48Z","receivedAt":"2013-02-22T22:53:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I guess we should probably issue a warning, too. Also disappointingly,\n> git-fsck does not seem to detect this breakage at all.\n\nYes for the warning, and no for disappointing. IIRC, in the very\nearly implementations allowed tag object without dates.\n\nI _think_ we can start tightening fsck, though.\n"},{"id":"210069","messageId":"20130222230132.GB4514@google.com","threadId":"32980","inReplyTo":"kg8ri2$vjb$1@ger.gmane.org","subject":"[RFC/PATCH] hash-object doc: \"git hash-object -w\" can write invalid objects","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-02-22T23:01:32Z","receivedAt":"2013-02-22T23:01:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When using \"hash-object -w\" to create non-blob objects, it is\ngenerally a good policy to run \"git fsck\" afterward to make sure the\nresulting object is valid. Add a warning to the manpage.\n\nWhile it at, gently nudge the user of \"hash-object -w\" toward\nhigher-level interfaces for creating or modifying trees, commits, and\ntags.\n\nReported-by: Mantas Mikulėnas \nSigned-off-by: Jonathan Nieder \n---\nHi Mantas,\n\nMantas Mikulėnas wrote:\n\n> When messing around with various repositories, I noticed that git 1.8\n> (currently using 1.8.2.rc0.22.gb3600c3) has problems parsing tag objects\n> that have invalid timestamps.\n[...]\n> Git doesn't handle the resulting tag objects nicely at all. For example,\n> running `git cat-file -p` on the new object outputs a really odd\n> timestamp \"Thu Jun Thu Jan 1 00:16:09 1970 +0016\" (I'm guessing it\n> parses the year as Unix time),\n\nThe usual rule is that with invalid objects (e.g. as detected by \"git\nfsck\"), any non-crash result is acceptable. Garbage in, garbage out.\n\n> and `git show` outright crashes\n> (backtrace included below.)\n\nProbably worth fixing.\n\nI notice that git-hash-object(1) doesn't contain any reference to\ngit-fsck(1). How about something like this, to start?\n\nPerhaps by default hash-object should automatically fsck the objects\nit is asked to create.\n\nThanks,\nJonathan\n\n Documentation/git-hash-object.txt | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/Documentation/git-hash-object.txt b/Documentation/git-hash-object.txt\nindex 02c1f12..8ed8c6e 100644\n--- a/Documentation/git-hash-object.txt\n+++ b/Documentation/git-hash-object.txt\n@@ -30,6 +30,8 @@ OPTIONS\n \n -w::\n \tActually write the object into the object database.\n+\tThis does not check that the resulting object is valid;\n+\tfor that, see linkgit:git-fsck[1].\n \n --stdin::\n \tRead the object from standard input instead of from a file.\n@@ -53,6 +55,14 @@ OPTIONS\n \tconversion. If the file is read from standard input then this\n \tis always implied, unless the --path option is given.\n \n+SEE ALSO\n+--------\n+linkgit:git-mktree[1],\n+linkgit:git-commit-tree[1],\n+linkgit:git-tag[1],\n+linkgit:git-filter-branch[1],\n+sha1sum(1)\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\n-- \n1.8.1.4\n"},{"id":"210070","messageId":"20130222230418.GC21579@sigill.intra.peff.net","threadId":"32980","inReplyTo":"7vy5egark3.fsf@alter.siamese.dyndns.org","subject":"Re: Crashes while trying to show tag objects with bad timestamps","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-22T23:04:18Z","receivedAt":"2013-02-22T23:04:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2013 at 02:53:48PM -0800, Junio C Hamano wrote:\n\n> > I guess we should probably issue a warning, too. Also disappointingly,\n> > git-fsck does not seem to detect this breakage at all.\n> \n> Yes for the warning, \n\nUnfortunately, a good warning is harder than I had hoped. At the point\nwhere we notice the problem, pp_user_info, we have very little context.\nWe can say only something like:\n\n warning: malformed date in ident 'Jeff King BOGUS'\n\nbut we cannot say in which object, or even that it was a \"tagger\" line\n(and in some cases we do not even have an object, as in\nmake_cover_letter).\n\n> and no for disappointing. IIRC, in the very early implementations\n> allowed tag object without dates.\n> \n> I _think_ we can start tightening fsck, though.\n\nThen I think it would make sense to allow the very specific no-date tag,\nbut not allow arbitrary crud. I wonder if there's an example in the\nkernel or in git.git.\n\nI also took a look at parsing routine of \"cat-file -p\". It's totally\nhand-rolled, separate from what \"git show\" does, and is not build on the\npretty-print code at all. I wonder, though, if it actually makes sense\nto munge the date there. The commit-object pretty-printer for cat-file\njust shows the object intact. It seems weirdly inconsistent that we\nwould munge tags just to rewrite the date. If you want a real\npretty-printer, you should be using porcelain like \"show\".\n\nIt would be a regression, of course, for people relying on \"cat-file -p\"\nto have consistent output. But I am very tempted to call it a bug, and\ntempted to call \"cat-file -p\" inside a script a bad thing (you cannot,\nafter all, tell what object type you have; you should figure out the\ntype you expect and then use \"cat-file \" to make sure you\nget the right one).\n\n-Peff\n"},{"id":"210071","messageId":"7vtxp4aqxj.fsf@alter.siamese.dyndns.org","threadId":"32980","inReplyTo":"20130222230132.GB4514@google.com","subject":"Re: [RFC/PATCH] hash-object doc: \"git hash-object -w\" can write invalid objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-22T23:07:20Z","receivedAt":"2013-02-22T23:07:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Perhaps by default hash-object should automatically fsck the objects\n> it is asked to create.\n\nYes, and let the experimentors to override when they are trying to\ninvent a new object type, finished a reader but not a writer (that\nis why they are exprimenting with hash-object) nor updated fsck,\nwith an explicit command line option to \"hash-objects\".\n\nThen we do not have to say \"-w by default can create an invalid\nobject\" in its documentation. In a sense, allowing to create any\ngarbage (by the definition of then-current fsck and the rest of the\nGit) is the raison d'etre of the command.\n\nThanks.\n"},{"id":"210072","messageId":"20130222230910.GD21579@sigill.intra.peff.net","threadId":"32980","inReplyTo":"20130222230132.GB4514@google.com","subject":"Re: [RFC/PATCH] hash-object doc: \"git hash-object -w\" can write invalid objects","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-22T23:09:10Z","receivedAt":"2013-02-22T23:09:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2013 at 03:01:32PM -0800, Jonathan Nieder wrote:\n\n> > Git doesn't handle the resulting tag objects nicely at all. For example,\n> > running `git cat-file -p` on the new object outputs a really odd\n> > timestamp \"Thu Jun Thu Jan 1 00:16:09 1970 +0016\" (I'm guessing it\n> > parses the year as Unix time),\n> \n> The usual rule is that with invalid objects (e.g. as detected by \"git\n> fsck\"), any non-crash result is acceptable. Garbage in, garbage out.\n\nAgreed, though I think a more consistent garbage would be good (e.g.,\ntime=0, tz=0).\n\n> I notice that git-hash-object(1) doesn't contain any reference to\n> git-fsck(1). How about something like this, to start?\n\nI think it's a good change. Though note that this problem is not\ndiscovered by fsck (which I think we should also change).\n\n> Perhaps by default hash-object should automatically fsck the objects\n> it is asked to create.\n\nNot unreasonable. In this case, we also have git-mktag. It would be nice\nif we could simply run the input through a type-specific sanity checker\n(optional, I hope; I use hash-object often to craft test cases like this\n:) ). The same need came up a month or two ago in a discussion of how to\nuse \"git replace\" safely. But I guess fsck after-the-fact is just\nanother form of the same solution.\n\n-Peff\n"},{"id":"210074","messageId":"CAPWNY8UMkxvLPk2TxCz+BAat1sNXitjhv=yqcdY0yZ1OLjgd0w@mail.gmail.com","threadId":"32980","inReplyTo":"20130222230418.GC21579@sigill.intra.peff.net","subject":"Re: Crashes while trying to show tag objects with bad timestamps","fromName":"Mantas Mikulėnas","fromEmail":"grawity@gmail.com","sentAt":"2013-02-22T23:14:40Z","receivedAt":"2013-02-22T23:14:40Z","isPatch":false,"sender":{"key":"grawity@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31021?v=4"},"body":"On Sat, Feb 23, 2013 at 1:04 AM, Jeff King wrote:\n> On Fri, Feb 22, 2013 at 02:53:48PM -0800, Junio C Hamano wrote:\n>> and no for disappointing. IIRC, in the very early implementations\n>> allowed tag object without dates.\n>>\n>> I _think_ we can start tightening fsck, though.\n>\n> Then I think it would make sense to allow the very specific no-date tag,\n> but not allow arbitrary crud. I wonder if there's an example in the\n> kernel or in git.git.\n\nI couldn't find any such examples. However, I did find several tags\nwith no \"tagger\" line at all: git.git has \"v0.99\" and linux.git has\nmany such tags starting with \"v2.6.11\" ending with \"v2.6.13-rc3\".\n\nIt seems that `git cat-file -p` doesn't like such tags too – if there\nis no \"tagger\", it doesn't display *any* header lines. More bugs?\n\n--\nMantas Mikulėnas\n"},{"id":"210075","messageId":"7vppzsaqc5.fsf@alter.siamese.dyndns.org","threadId":"32980","inReplyTo":"20130222230418.GC21579@sigill.intra.peff.net","subject":"Re: Crashes while trying to show tag objects with bad timestamps","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-22T23:20:10Z","receivedAt":"2013-02-22T23:20:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Feb 22, 2013 at 02:53:48PM -0800, Junio C Hamano wrote:\n>\n>> > I guess we should probably issue a warning, too. Also disappointingly,\n>> > git-fsck does not seem to detect this breakage at all.\n>> \n>> Yes for the warning, \n>\n> Unfortunately, a good warning is harder than I had hoped. At the point\n> where we notice the problem, pp_user_info, we have very little context.\n> We can say only something like:\n>\n> warning: malformed date in ident 'Jeff King BOGUS'\n>\n> but we cannot say in which object, or even that it was a \"tagger\" line\n> (and in some cases we do not even have an object, as in\n> make_cover_letter).\n\nAs pp_user_info() is called from very few places, I do not think it\nis unreasonable to add an output parameter (i.e. \"unsigned *\") to\nlet the caller know that we made a best guess given malformed input\nand handle the error in the caller. The make_cover_letter() caller\nmay look like:\n\n\tpp_user_info(&pp, NULL, &sb, committer, encoding, &errors);\n if (errors & PP_CORRUPT_DATE)\n\t\twarning(\"unparsable datestamp in '%s'\", committer);\n\nalthough it is unlikely to see this error in practice, given that\ncommitter is coming from git_committer_info(0) and would have the\ncurrent timestamp.\n\n> I also took a look at parsing routine of \"cat-file -p\". It's totally\n> hand-rolled, separate from what \"git show\" does, and is not build on the\n> pretty-print code at all. I wonder, though, if it actually makes sense\n> to munge the date there. The commit-object pretty-printer for cat-file\n> just shows the object intact. It seems weirdly inconsistent that we\n> would munge tags just to rewrite the date. If you want a real\n> pretty-printer, you should be using porcelain like \"show\".\n\nThe whole \"cat-file -p\" is a historical wart, aka poor-man's\n\"show\". I do not even consider it a part of the plumbing. It is a\nfair game for Porcelainisque improvement ;-)\n"},{"id":"210260","messageId":"20130225182101.GA13912@sigill.intra.peff.net","threadId":"32980","inReplyTo":"CAPWNY8UMkxvLPk2TxCz+BAat1sNXitjhv=yqcdY0yZ1OLjgd0w@mail.gmail.com","subject":"Re: Crashes while trying to show tag objects with bad timestamps","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-25T18:21:01Z","receivedAt":"2013-02-25T18:21:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 23, 2013 at 01:14:40AM +0200, Mantas Mikulėnas wrote:\n\n> > Then I think it would make sense to allow the very specific no-date tag,\n> > but not allow arbitrary crud. I wonder if there's an example in the\n> > kernel or in git.git.\n> \n> I couldn't find any such examples. However, I did find several tags\n> with no \"tagger\" line at all: git.git has \"v0.99\" and linux.git has\n> many such tags starting with \"v2.6.11\" ending with \"v2.6.13-rc3\".\n\nYes, I think Junio was mis-remembering the exact condition. It looks\nlike we added tagger lines in c818566 ([PATCH] Update tags to record who\nmade them, 2005-07-14), which pulls the identity straight from \"git var\nGIT_COMMITTER_IDENT\". I double-checked to be sure that we included the\ndate stamp at that time, and we did.\n\nWhen parsing such a tag, we put a \"0\" in the date field of the \"struct\ntag\", and I suspect that is what caused the memory confusion.\n\nSo I think we are fine to fsck tagger lines as we do ordinary\nauthor/committer ident lines; the only exception is that we should not\ncomplain if they do not exist.\n\n> It seems that `git cat-file -p` doesn't like such tags too – if there\n> is no \"tagger\", it doesn't display *any* header lines. More bugs?\n\nYeah, I think we should just rid of that parser entirely. It is very\ninconsistent with the pretty-printer used by \"git show\", as well as the\none used by \"git for-each-ref\", not to mention parse_tag (ugh, how many\ntag parsers do we have?).\n\n-Peff\n"},{"id":"210262","messageId":"20130225183009.GB13912@sigill.intra.peff.net","threadId":"32980","inReplyTo":"7vppzsaqc5.fsf@alter.siamese.dyndns.org","subject":"Re: Crashes while trying to show tag objects with bad timestamps","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-25T18:30:09Z","receivedAt":"2013-02-25T18:30:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2013 at 03:20:10PM -0800, Junio C Hamano wrote:\n\n> As pp_user_info() is called from very few places, I do not think it\n> is unreasonable to add an output parameter (i.e. \"unsigned *\") to\n> let the caller know that we made a best guess given malformed input\n> and handle the error in the caller. The make_cover_letter() caller\n> may look like:\n> \n> \tpp_user_info(&pp, NULL, &sb, committer, encoding, &errors);\n> if (errors & PP_CORRUPT_DATE)\n> \t\twarning(\"unparsable datestamp in '%s'\", committer);\n> \n> although it is unlikely to see this error in practice, given that\n> committer is coming from git_committer_info(0) and would have the\n> current timestamp.\n\nSadly that is not quite enough for the object-parsing cases (which are\nthe ones we _really_ want to add context to, because they are buried\ninside other pp_* calls. Probably adding an object context field (or an\nerror return) to the pretty-print context would make sense. But I don't\nrelish the thought of annotating each pretty-print caller.\n\nI think we're OK to be silent and just react in an appropriate way;\nhaving looked over the other callers of split_ident_line, we already do\nso in some places. See my patch 1 below for details.\n\nOnce fsck is taught to note this, then the warning is a lot less\nimportant (my patch 3 below).\n\n> The whole \"cat-file -p\" is a historical wart, aka poor-man's\n> \"show\". I do not even consider it a part of the plumbing. It is a\n> fair game for Porcelainisque improvement ;-)\n\nGood, that's how I feel, too. See my patch 4. :)\n\nHere are the patches I'd like to do:\n\n [1/4]: handle malformed dates in ident lines\n [2/4]: skip_prefix: return a non-const pointer\n [3/4]: fsck: check \"tagger\" lines\n [4/4]: cat-file: print tags raw for \"cat-file -p\"\n\nThe first one is solid, and should probably go to maint and/or the -rc\ntrack, as it fixes a segfault on bogus input. It's hopefully a\nno-brainer, as the existing behavior is obviously unacceptable. We may\nchange our mind later about exactly what to print for such bogus input,\nbut whatever we print in such a case is just trying to be nice to the\nuser, and anybody who depends on our particular handling of malformed\nobjects is crazy.\n\nThe rest can wait, as they are about improving output when fed bogus\ninput, or tightening fsck. Moreover, they have some problems which make\nthem not suitable for applying yet. I'll give details in each patch.\n\n-Peff\n"},{"id":"210265","messageId":"20130225183815.GA14438@sigill.intra.peff.net","threadId":"32980","inReplyTo":"20130225183009.GB13912@sigill.intra.peff.net","subject":"[PATCH 1/4] handle malformed dates in ident lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-25T18:38:15Z","receivedAt":"2013-02-25T18:38:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The split_ident_line function is used by many code paths to\nfind the name, mail, and date fields of an identity line.\nIt will return failure when the output is completely\nunparseable, but may return success (along with a NULL date\nfield) if the date is empty or malformed. Callers that care\nabout the date need to handle this situation explicitly, or\nthey will end up segfaulting as they feed the NULL date to\nstrtoul or similar.\n\nThere are basically three options:\n\n 1. Reject the ident line entirely (i.e., do the same thing\n we would do if the name or email were missing). We\n already use this strategy in git-commit's\n determine_author_info.\n\n 2. Process the name and email, but refuse to work on any\n date portions. format_person_part does this, and a\n commit with such an ident would yield an empty string\n for \"--format=%at\".\n\n 3. Proceed with some obviously bogus sentinel value for\n the time (e.g., the start of the epoch).\n\nOnly two of the existing callers read the date but do not\nhandle this case at all: pp_user_info and git-blame's\nget_ac_line. This patch modifies both to use option (3). The\nhope is that this is friendly (we still produce some output)\nbut the epoch start date will give the user a clue that the\ndate is not valid.\n\nSigned-off-by: Jeff King \n---\nHaving written that, I feel like the case for doing (3) is somewhat\nflimsy. I don't know that it really matters that much either way, but\nI'd be just as happy to do any of the others.\n\nIn fact, I really wonder if split_ident_line shouldn't simply be\nreturning error in such a case. The comment above it claims that reflogs\ndo not have such a timestamp, but they do. I suspect it is a weird\ninteraction of the reflog walker on format_person_part, but I haven't\nchecked. I wonder if we can simply fix the reflog code to pass a string\nwith the date in the usual way (since we _should_ have it at some\npoint). If not, then we can perhaps make things safer for other callers\nwith a wrapper like:\n\n /* safe default version */\n int split_ident_line(...)\n {\n /* calls less safe but more flexible version; i.e., the\n * existing one */\n if (split_ident_line_date_optional(...) < 0)\n return -1;\n if (!ident.date_begin)\n return -1;\n return 0;\n }\n\n builtin/blame.c | 13 +++++++++----\n pretty.c | 15 ++++++++++++---\n 2 files changed, 21 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 86100e9..2edff25 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1375,10 +1375,15 @@ static void get_ac_line(const char *inbuf, const char *what,\n \tmaillen = ident.mail_end - ident.mail_begin;\n \tmailbuf = ident.mail_begin;\n \n-\t*time = strtoul(ident.date_begin, NULL, 10);\n-\n-\tlen = ident.tz_end - ident.tz_begin;\n-\tstrbuf_add(tz, ident.tz_begin, len);\n+\tif (ident.date_begin) {\n+\t\t*time = strtoul(ident.date_begin, NULL, 10);\n+\t\tlen = ident.tz_end - ident.tz_begin;\n+\t\tstrbuf_add(tz, ident.tz_begin, len);\n+\t}\n+\telse {\n+\t\t*time = 0;\n+\t\tstrbuf_addstr(tz, \"-0000\");\n+\t}\n \n \t/*\n \t * Now, convert both name and e-mail using mailmap\ndiff --git a/pretty.c b/pretty.c\nindex eae57ad..4b19908 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -391,7 +391,7 @@ void pp_user_info(const struct pretty_print_context *pp,\n \tstruct strbuf mail;\n \tstruct ident_split ident;\n \tint linelen;\n-\tchar *line_end, *date;\n+\tchar *line_end;\n \tconst char *mailbuf, *namebuf;\n \tsize_t namelen, maillen;\n \tint max_length = 78; /* per rfc2822 */\n@@ -428,8 +428,17 @@ void pp_user_info(const struct pretty_print_context *pp,\n \tstrbuf_add(&name, namebuf, namelen);\n \n \tnamelen = name.len + mail.len + 3; /* ' ' + '<' + '>' */\n-\ttime = strtoul(ident.date_begin, &date, 10);\n-\ttz = strtol(date, NULL, 10);\n+\n+\tif (ident.date_begin) {\n+\t\tchar *date;\n+\t\ttime = strtoul(ident.date_begin, &date, 10);\n+\t\ttz = strtol(date, NULL, 10);\n+\t}\n+\telse {\n+\t\t/* ident has missing or malformed date */\n+\t\ttime = 0;\n+\t\ttz = 0;\n+\t}\n \n \tif (pp->fmt == CMIT_FMT_EMAIL) {\n \t\tstrbuf_addstr(sb, \"From: \");\n-- \n1.8.1.4.4.g265d2fa\n"},{"id":"210266","messageId":"20130225183948.GB14438@sigill.intra.peff.net","threadId":"32980","inReplyTo":"20130225183009.GB13912@sigill.intra.peff.net","subject":"[PATCH/RFC 2/4] skip_prefix: return a non-const pointer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-25T18:39:48Z","receivedAt":"2013-02-25T18:39:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The const rules in C are such that one cannot write a\nfunction that takes a const or non-const pointer and returns\na pointer that matches the input in const-ness. Instead, you\nmust take a const pointer (because you are promising not to\nmodify it), and then either return a const pointer (which is\nsafer, but annoying to callers who originally had a\nnon-const pointer) or a non-const pointer (less safe, as you\nmay accidentally drop constness, but less annoying).\n\nThis is a well-known problem, and the standard string\nfunctions like strchr take the \"less annoying\" approach.\nLet's mimic them. Even though this is technically less safe,\nskip_prefix tends to be used alongside standard string\nmanipulation functions already, so it is not really\nintroducing a new problem.\n\nSigned-off-by: Jeff King \n---\nI have mixed feelings on this. It _is_ less safe, and this is a known\nbug in the C standard. Still, it seems like the more idiomatic C thing\nto do.\n\nMy main motivation is to avoid a bunch of casts in the next patch.\n\n builtin/commit.c | 2 +-\n git-compat-util.h | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 3348aa1..bb6890b 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -895,7 +895,7 @@ static int template_untouched(struct strbuf *sb)\n \t\treturn 0;\n \n \tstripspace(&tmpl, cleanup_mode == CLEANUP_ALL);\n-\tstart = (char *)skip_prefix(sb->buf, tmpl.buf);\n+\tstart = skip_prefix(sb->buf, tmpl.buf);\n \tif (!start)\n \t\tstart = sb->buf;\n \tstrbuf_release(&tmpl);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b7eaaa9..56c066b 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -320,10 +320,10 @@ static inline const char *skip_prefix(const char *str, const char *prefix)\n extern int prefixcmp(const char *str, const char *prefix);\n extern int suffixcmp(const char *str, const char *suffix);\n \n-static inline const char *skip_prefix(const char *str, const char *prefix)\n+static inline char *skip_prefix(const char *str, const char *prefix)\n {\n \tsize_t len = strlen(prefix);\n-\treturn strncmp(str, prefix, len) ? NULL : str + len;\n+\treturn strncmp(str, prefix, len) ? NULL : (char *)(str + len);\n }\n \n #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n-- \n1.8.1.4.4.g265d2fa\n"},{"id":"210268","messageId":"20130225184617.GC14438@sigill.intra.peff.net","threadId":"32980","inReplyTo":"20130225183009.GB13912@sigill.intra.peff.net","subject":"[PATCH 3/4] fsck: check \"tagger\" lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-25T18:46:17Z","receivedAt":"2013-02-25T18:46:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The fsck_tag function does not check very much about tags at\nall; it just makes sure that we were able to load the\npointed-to object during the parse_tag phase. This does\ncheck some basic things (the \"object\" line is OK, and the\npointed-to object exists with the expected type).\n\nWe did not, however, check the \"tagger\" line at all; if they\nexist, we should feed them to fsck_ident (and it is OK if\nthey do not, as early versions of git did not include them).\n\nThis patch runs through the whole tag object during\nfsck_tag, similar to what we do in fsck_commit. Some of\nthese checks are technically redundant with just checking\nthat parse_tag filled in the \"tag->tagged\" field. However:\n\n 1. We have to parse through those lines anyway to get to\n the tagger line, so we need to sanity check our\n parsing.\n\n 2. We can give more specific errors (e.g., report a\n malformed \"object\" line).\n\n 3. Previously we depended on implementation details of\n parse_tag for our fsck (e.g., that it would never fill\n in \"tagged\" if the types did not match). Now our\n exhaustive checks are in one place, which makes it\n easier to verify exactly what fsck is checking.\n\nSigned-off-by: Jeff King \n---\nUnfortunately, this causes t1050 to fail in an interesting way. It runs\n\"index-pack --strict\" while setting GIT_DIR=nonexistent. As a result,\nwhen we try to read the tag object from disk, we can't find it. We\ndon't run into the same problem verifying commits and trees, because\nthose objects leave the raw object data in their \"buffer\" field.\n\nI'm tempted to call what that test is doing insane, but I wonder if\nthere is another corner case with running \"index-pack --strict\" as part\nof an incoming push or fetch. I haven't investigated that yet.\n\n fsck.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t1450-fsck.sh | 43 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 104 insertions(+), 1 deletion(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex 99c0497..20d55c4 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -340,15 +340,75 @@ static int fsck_tag(struct tag *tag, fsck_error error_func)\n \treturn 0;\n }\n \n-static int fsck_tag(struct tag *tag, fsck_error error_func)\n+static int fsck_tag_buffer(char *buf, struct tag *tag, fsck_error error_func)\n {\n \tstruct object *tagged = tag->tagged;\n+\tunsigned char sha1[20];\n+\tchar *eol;\n+\n+\tbuf = skip_prefix(buf, \"object \");\n+\tif (!buf)\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"invalid format - expected 'object' line\");\n+\tif (get_sha1_hex(buf, sha1) || buf[40] != '\\n')\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"invalid 'object' line format - bad sha1\");\n+\tbuf += 41;\n \n+\t/*\n+\t * We already called parse_tag, so we don't have to bother looking up\n+\t * the sha1 again.\n+\t */\n \tif (!tagged)\n \t\treturn error_func(&tag->object, FSCK_ERROR, \"could not load tagged object\");\n+\n+\tbuf = skip_prefix(buf, \"type \");\n+\tif (!buf)\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"invalid format - expected 'type' line\");\n+\teol = strchr(buf, '\\n');\n+\tif (!eol)\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"invalid format - truncation at 'type' line\");\n+\t*eol = '\\0';\n+\tif (type_from_string(buf) != tagged->type)\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"'type' line does not match type of tagged object\");\n+\t*eol = '\\n';\n+\n+\tbuf = skip_prefix(eol + 1, \"tag \");\n+\tif (!buf)\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"invalid format - expected 'tag' line\");\n+\teol = strchr(buf, '\\n');\n+\tif (!eol)\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"invalid format - truncation at 'tag' line\");\n+\n+\t/*\n+\t * A missing tagger is OK, as very old versions of git did not produce\n+\t * such a line. But if we do have it, we should verify its contents.\n+\t */\n+\tbuf = skip_prefix(eol + 1, \"tagger \");\n+\tif (buf) {\n+\t\tint err = fsck_ident(&buf, &tag->object, error_func);\n+\t\tif (err)\n+\t\t\treturn err;\n+\t}\n+\n \treturn 0;\n }\n \n+static int fsck_tag(struct tag *tag, fsck_error error_func)\n+{\n+\tchar *buf;\n+\tunsigned long size;\n+\tenum object_type type;\n+\tint err;\n+\n+\tbuf = read_sha1_file(tag->object.sha1, &type, &size);\n+\tif (!buf)\n+\t\treturn error_func(&tag->object, FSCK_ERROR, \"could not read tag object\");\n+\n+\terr = fsck_tag_buffer(buf, tag, error_func);\n+\n+\tfree(buf);\n+\treturn err;\n+}\n+\n int fsck_object(struct object *obj, int strict, fsck_error error_func)\n {\n \tif (!obj)\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex d730734..3a3bce6 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -180,6 +180,49 @@ test_expect_success 'tag pointing to something else than its type' '\n \ttest_must_fail git fsck --tags\n '\n \n+test_expect_success 'tag with missing tagger is OK' '\n+\tsha=$(echo blob | git hash-object -w --stdin) &&\n+\ttest_when_finished \"remove_object $sha\" &&\n+\tcat >good-tag <<-EOF &&\n+\tobject $sha\n+\ttype blob\n+\ttag missing-tagger-ok\n+\n+\tThis is totally fine.\n+\tEOF\n+\n+\ttag=$(git hash-object -t tag -w --stdin out 2>&1 &&\n+\t>expect &&\n+\ttest_cmp expect out\n+'\n+\n+test_expect_success 'tag with bogus tagger is not OK' '\n+\tsha=$(echo blob | git hash-object -w --stdin) &&\n+\ttest_when_finished \"remove_object $sha\" &&\n+\tcat >wrong-tag <<-EOF &&\n+\tobject $sha\n+\ttype blob\n+\ttag bogus-tagger\n+\ttagger T A Gger Mon Feb 25 12:32:51 2013 -0500\n+\n+\tThat date is bogus (it should be an epoch + timezone). Any bogus ident\n+\tline should trigger, but this was chosen to match a breakage seen in\n+\tthe wild.\n+\tEOF\n+\n+\ttag=$(git hash-object -t tag -w --stdin out 2>&1 &&\n+\tcat out &&\n+\tgrep \"error in tag $tag.* - bad date\" out\n+'\n+\n test_expect_success 'cleaned up' '\n \tgit fsck >actual 2>&1 &&\n \ttest_cmp empty actual\n-- \n1.8.1.4.4.g265d2fa\n"},{"id":"210269","messageId":"20130225185058.GD14438@sigill.intra.peff.net","threadId":"32980","inReplyTo":"20130225183009.GB13912@sigill.intra.peff.net","subject":"[PATCH 4/4] cat-file: print tags raw for \"cat-file -p\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-25T18:50:58Z","receivedAt":"2013-02-25T18:50:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When \"cat-file -p\" prints commits, it shows them in their\nraw format, since git's format is already human-readable.\nFor tags, however, we print the whole thing raw except for\none thing: we convert the timestamp on the tagger line into a\nhuman-readable date.\n\nThis dates all the way back to a0f15fa (Pretty-print tagger\ndates, 2006-03-01). At that time there was no way to\npretty-print a tag. These days \"git show\" does this already,\nand is the normal tool for showing a pretty-printed output\n(\"cat-file tag $tag\" remains the preferred method for\nshowing porcelain output).\n\nLet's drop this. It makes us more consistent with cat-file's\ncommit pretty-printer, and it means we can drop a whole\nbunch of hand-rolled tag parsing code (which happened to\nbehave inconsistently with the tag pretty-printing code\nelsewhere).\n\nNote that \"git verify-tag\" and \"git tag -v\" depend on\n\"cat-file -p\" to show the tag. This means they will start\nshowing the raw timestamp. We may want to adjust them to\nuse the pretty-printing code from \"git show\".\n\nSigned-off-by: Jeff King \n---\nI don't use \"git tag -v\" much, so I'm not sure what is sane there. But\nthis seems like it would be a regression for people who want to check\nthe human-readable date given by GPG against the date in the tag object.\n\nI still think dropping this hand-rolled parsing is a good thing. The\nmost sane thing to me would be to move the parsing from \"git show\" into\nthe pretty-print code, then have both it and \"verify-tag\" use it.\nProbably \"for-each-ref\" could stand to use it as well, as it has its own\nhome-grown parser.\n\n builtin/cat-file.c | 71 -----------------------------------------------------\n t/t1006-cat-file.sh | 5 +---\n 2 files changed, 1 insertion(+), 75 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 00528dd..b195edf 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -16,73 +16,6 @@\n #define BATCH 1\n #define BATCH_CHECK 2\n \n-static void pprint_tag(const unsigned char *sha1, const char *buf, unsigned long size)\n-{\n-\t/* the parser in tag.c is useless here. */\n-\tconst char *endp = buf + size;\n-\tconst char *cp = buf;\n-\n-\twhile (cp < endp) {\n-\t\tchar c = *cp++;\n-\t\tif (c != '\\n')\n-\t\t\tcontinue;\n-\t\tif (7 <= endp - cp && !memcmp(\"tagger \", cp, 7)) {\n-\t\t\tconst char *tagger = cp;\n-\n-\t\t\t/* Found the tagger line. Copy out the contents\n-\t\t\t * of the buffer so far.\n-\t\t\t */\n-\t\t\twrite_or_die(1, buf, cp - buf);\n-\n-\t\t\t/*\n-\t\t\t * Do something intelligent, like pretty-printing\n-\t\t\t * the date.\n-\t\t\t */\n-\t\t\twhile (cp < endp) {\n-\t\t\t\tif (*cp++ == '\\n') {\n-\t\t\t\t\t/* tagger to cp is a line\n-\t\t\t\t\t * that has ident and time.\n-\t\t\t\t\t */\n-\t\t\t\t\tconst char *sp = tagger;\n-\t\t\t\t\tchar *ep;\n-\t\t\t\t\tunsigned long date;\n-\t\t\t\t\tlong tz;\n-\t\t\t\t\twhile (sp < cp && *sp != '>')\n-\t\t\t\t\t\tsp++;\n-\t\t\t\t\tif (sp == cp) {\n-\t\t\t\t\t\t/* give up */\n-\t\t\t\t\t\twrite_or_die(1, tagger,\n-\t\t\t\t\t\t\t cp - tagger);\n-\t\t\t\t\t\tbreak;\n-\t\t\t\t\t}\n-\t\t\t\t\twhile (sp < cp &&\n-\t\t\t\t\t !('0' <= *sp && *sp <= '9'))\n-\t\t\t\t\t\tsp++;\n-\t\t\t\t\twrite_or_die(1, tagger, sp - tagger);\n-\t\t\t\t\tdate = strtoul(sp, &ep, 10);\n-\t\t\t\t\ttz = strtol(ep, NULL, 10);\n-\t\t\t\t\tsp = show_date(date, tz, 0);\n-\t\t\t\t\twrite_or_die(1, sp, strlen(sp));\n-\t\t\t\t\txwrite(1, \"\\n\", 1);\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (cp < endp && *cp == '\\n')\n-\t\t\t/* end of header */\n-\t\t\tbreak;\n-\t}\n-\t/* At this point, we have copied out the header up to the end of\n-\t * the tagger line and cp points at one past \\n. It could be the\n-\t * next header line after the tagger line, or it could be another\n-\t * \\n that marks the end of the headers. We need to copy out the\n-\t * remainder as is.\n-\t */\n-\tif (cp < endp)\n-\t\twrite_or_die(1, cp, endp - cp);\n-}\n-\n static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n {\n \tunsigned char sha1[20];\n@@ -133,10 +66,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n \t\tbuf = read_sha1_file(sha1, &type, &size);\n \t\tif (!buf)\n \t\t\tdie(\"Cannot read object %s\", obj_name);\n-\t\tif (type == OBJ_TAG) {\n-\t\t\tpprint_tag(sha1, buf, size);\n-\t\t\treturn 0;\n-\t\t}\n \n \t\t/* otherwise just spit out the data */\n \t\tbreak;\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex d8b7f2f..da4ffbb 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -135,14 +135,11 @@ tag_size=$(strlen \"$tag_content\")\n tag_content=\"$tag_header_without_timestamp 0000000000 +0000\n \n $tag_description\"\n-tag_pretty_content=\"$tag_header_without_timestamp Thu Jan 1 00:00:00 1970 +0000\n-\n-$tag_description\"\n \n tag_sha1=$(echo_without_newline \"$tag_content\" | git mktag)\n tag_size=$(strlen \"$tag_content\")\n \n-run_tests 'tag' $tag_sha1 $tag_size \"$tag_content\" \"$tag_pretty_content\" 1\n+run_tests 'tag' $tag_sha1 $tag_size \"$tag_content\" \"$tag_content\" 1\n \n test_expect_success \\\n \"Reach a blob from a tag pointing to it\" \\\n-- \n1.8.1.4.4.g265d2fa\n"},{"id":"210282","messageId":"CAPWNY8V=OT_Rt0GfhrmEhf_bqqMhCSR4EDCiX0FOr8EZSPkAUA@mail.gmail.com","threadId":"32980","inReplyTo":"20130225185058.GD14438@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] cat-file: print tags raw for \"cat-file -p\"","fromName":"Mantas Mikulėnas","fromEmail":"grawity@gmail.com","sentAt":"2013-02-25T19:33:28Z","receivedAt":"2013-02-25T19:33:28Z","isPatch":true,"sender":{"key":"grawity@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31021?v=4"},"body":"On Mon, Feb 25, 2013 at 8:50 PM, Jeff King wrote:\n> Note that \"git verify-tag\" and \"git tag -v\" depend on\n> \"cat-file -p\" to show the tag. This means they will start\n> showing the raw timestamp. We may want to adjust them to\n> use the pretty-printing code from \"git show\".\n>\n> Signed-off-by: Jeff King \n> ---\n> I don't use \"git tag -v\" much, so I'm not sure what is sane there. But\n> this seems like it would be a regression for people who want to check\n> the human-readable date given by GPG against the date in the tag object.\n\nPersonally, I've found it quite confusing that commits (incl. merged\ntags) can be verified with `git show --show-signature`, but for tags I\nmust use `git tag -v`... took me a while to find the latter.\n\n\n\n(`git show --verify` might be even better, but that's just me.)\n\n-- \nMantas Mikulėnas \n"}]}