{"thread":{"id":"31350","subject":"bug: when applying binary diffs, -p0 is ignored","startedAt":"2012-08-24T22:09:09Z","lastAt":"2012-08-25T05:25:53Z","messageCount":2,"participants":["Colin McCabe","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"197799","messageId":"CA+qbEUOw9zR1_kdXU=mB8_KADq2NRT_qxSvGT+SfS55dUM8j6A@mail.gmail.com","threadId":"31350","inReplyTo":null,"subject":"bug: when applying binary diffs, -p0 is ignored","fromName":"Colin McCabe","fromEmail":"cmccabe@alumni.cmu.edu","sentAt":"2012-08-24T22:09:09Z","receivedAt":"2012-08-24T22:09:09Z","isPatch":false,"sender":{"key":"cmccabe@alumni.cmu.edu","avatar":null},"body":"I found a bug in git's handling of binary diff segments.\n\nWhen applying binary diffs using -p0, the prefix (or --strip) argument\nis ignored.\n\nFor example, try this:\ngit apply -p0 --binary <<'EOF'\ndiff --git a/init.tar.gz a/init.tar.gz\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..\n 386b94f511a17a8a3d62eb6cec14694cb9b9b51d\nGIT binary patch\nliteral 118\nzcmb2|=3qGT-8_JS`RzGFp(Y0bmIKvX{ySXzs`-OhSfm*K#a}SG%CY!eN_LjnjGuP-\nzFHDep|-nAGgFaXfQAU0L+LoA^-pY\n\nliteral 0\nHcmV?d00001\n\nEOF\n\nIt will create init.tar.gz, but in the root of the repository, not in a/\n\n(Sorry if my mailer wraps the long index line)\n\ncheers,\nColin McCabe\n"},{"id":"197809","messageId":"7v393by23i.fsf@alter.siamese.dyndns.org","threadId":"31350","inReplyTo":"CA+qbEUOw9zR1_kdXU=mB8_KADq2NRT_qxSvGT+SfS55dUM8j6A@mail.gmail.com","subject":"Re: bug: when applying binary diffs, -p0 is ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-25T05:25:53Z","receivedAt":"2012-08-25T05:25:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Colin McCabe writes:\n\n> I found a bug in git's handling of binary diff segments.\n>\n> When applying binary diffs using -p0, the prefix (or --strip) argument\n> is ignored.\n\nThanks for a report. An ancient bug well spotted.\n\nPerhaps this will help.\n\n-- >8 --\nSubject: apply: compute patch->def_name correctly under -p0\n\nBack when \"git apply\" was written, we made sure that the user can\nskip more than the default number of path components (i.e. 1) by\ngiving \"-p\", but the logic for doing so was built around the\nnotion of \"we skip N slashes and stop\". This obviously does not\nwork well when running under -p0 where we do not want to skip any,\nbut still want to skip SP/HT that separates the pathnames of\npreimage and postimage and want to reject absolute pathnames.\n\nStop using \"stop_at_slash()\", and instead introduce a new helper\n\"skip_tree_prefix()\" with similar logic but works correctly even for\nthe -p0 case.\n\nThis is an ancient bug, but has been masked for a long time because\nmost of the patches are text and have other clues to tell us the\nname of the preimage and the postimage.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 55 ++++++++++++++++++++++++++++++-------------------------\n 1 file changed, 30 insertions(+), 25 deletions(-)\n\ndiff --git i/builtin/apply.c w/builtin/apply.c\nindex 3bf71dc..4a511b3 100644\n--- i/builtin/apply.c\n+++ w/builtin/apply.c\n@@ -1095,15 +1095,23 @@ static int gitdiff_unrecognized(const char *line, struct patch *patch)\n \treturn -1;\n }\n \n-static const char *stop_at_slash(const char *line, int llen)\n+/*\n+ * Skip p_value leading components from \"line\"; as we do not accept\n+ * absolute paths, return NULL in that case.\n+ */\n+static const char *skip_tree_prefix(const char *line, int llen)\n {\n-\tint nslash = p_value;\n+\tint nslash;\n \tint i;\n \n+\tif (!p_value)\n+\t\treturn (llen && line[0] == '/') ? NULL : line;\n+\n+\tnslash = p_value;\n \tfor (i = 0; i < llen; i++) {\n \t\tint ch = line[i];\n \t\tif (ch == '/' && --nslash <= 0)\n-\t\t\treturn &line[i];\n+\t\t\treturn (i == 0) ? NULL : &line[i + 1];\n \t}\n \treturn NULL;\n }\n@@ -1133,12 +1141,11 @@ static char *git_header_name(const char *line, int llen)\n \t\tif (unquote_c_style(&first, line, &second))\n \t\t\tgoto free_and_fail1;\n \n-\t\t/* advance to the first slash */\n-\t\tcp = stop_at_slash(first.buf, first.len);\n-\t\t/* we do not accept absolute paths */\n-\t\tif (!cp || cp == first.buf)\n+\t\t/* strip the a/b prefix including trailing slash */\n+\t\tcp = skip_tree_prefix(first.buf, first.len);\n+\t\tif (!cp)\n \t\t\tgoto free_and_fail1;\n-\t\tstrbuf_remove(&first, 0, cp + 1 - first.buf);\n+\t\tstrbuf_remove(&first, 0, cp - first.buf);\n \n \t\t/*\n \t\t * second points at one past closing dq of name.\n@@ -1152,22 +1159,21 @@ static char *git_header_name(const char *line, int llen)\n \t\tif (*second == '\"') {\n \t\t\tif (unquote_c_style(&sp, second, NULL))\n \t\t\t\tgoto free_and_fail1;\n-\t\t\tcp = stop_at_slash(sp.buf, sp.len);\n-\t\t\tif (!cp || cp == sp.buf)\n+\t\t\tcp = skip_tree_prefix(sp.buf, sp.len);\n+\t\t\tif (!cp)\n \t\t\t\tgoto free_and_fail1;\n \t\t\t/* They must match, otherwise ignore */\n-\t\t\tif (strcmp(cp + 1, first.buf))\n+\t\t\tif (strcmp(cp, first.buf))\n \t\t\t\tgoto free_and_fail1;\n \t\t\tstrbuf_release(&sp);\n \t\t\treturn strbuf_detach(&first, NULL);\n \t\t}\n \n \t\t/* unquoted second */\n-\t\tcp = stop_at_slash(second, line + llen - second);\n-\t\tif (!cp || cp == second)\n+\t\tcp = skip_tree_prefix(second, line + llen - second);\n+\t\tif (!cp)\n \t\t\tgoto free_and_fail1;\n-\t\tcp++;\n-\t\tif (line + llen - cp != first.len + 1 ||\n+\t\tif (line + llen - cp != first.len ||\n \t\t memcmp(first.buf, cp, first.len))\n \t\t\tgoto free_and_fail1;\n \t\treturn strbuf_detach(&first, NULL);\n@@ -1179,10 +1185,9 @@ static char *git_header_name(const char *line, int llen)\n \t}\n \n \t/* unquoted first name */\n-\tname = stop_at_slash(line, llen);\n-\tif (!name || name == line)\n+\tname = skip_tree_prefix(line, llen);\n+\tif (!name)\n \t\treturn NULL;\n-\tname++;\n \n \t/*\n \t * since the first name is unquoted, a dq if exists must be\n@@ -1196,10 +1201,9 @@ static char *git_header_name(const char *line, int llen)\n \t\t\tif (unquote_c_style(&sp, second, NULL))\n \t\t\t\tgoto free_and_fail2;\n \n-\t\t\tnp = stop_at_slash(sp.buf, sp.len);\n-\t\t\tif (!np || np == sp.buf)\n+\t\t\tnp = skip_tree_prefix(sp.buf, sp.len);\n+\t\t\tif (!np)\n \t\t\t\tgoto free_and_fail2;\n-\t\t\tnp++;\n \n \t\t\tlen = sp.buf + sp.len - np;\n \t\t\tif (len < second - name &&\n@@ -1231,13 +1235,14 @@ static char *git_header_name(const char *line, int llen)\n \t\tcase '\\n':\n \t\t\treturn NULL;\n \t\tcase '\\t': case ' ':\n-\t\t\tsecond = stop_at_slash(name + len, line_len - len);\n+\t\t\tif (!name[len + 1])\n+\t\t\t\treturn NULL;\n+\t\t\tsecond = skip_tree_prefix(name + len + 1,\n+\t\t\t\t\t\t line_len - (len + 1));\n \t\t\tif (!second)\n \t\t\t\treturn NULL;\n-\t\t\tsecond++;\n-\t\t\tif (second[len] == '\\n' && !strncmp(name, second, len)) {\n+\t\t\tif (second[len] == '\\n' && !strncmp(name, second, len))\n \t\t\t\treturn xmemdupz(name, len);\n-\t\t\t}\n \t\t}\n \t}\n }\n"}]}