{"thread":{"id":"28722","subject":"[RFC 00/13] Checking full vs. partial refnames","startedAt":"2011-10-19T20:55:03Z","lastAt":"2011-10-19T21:59:19Z","messageCount":17,"participants":["mhagger@alum.mit.edu","Junio C Hamano","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"178013","messageId":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":null,"subject":"[RFC 00/13] Checking full vs. partial refnames","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:03Z","receivedAt":"2011-10-19T20:55:03Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThere are many places where it is necessary to determine whether a\nrefname is a complete, valid, top-level reference name in the \"refs/\"\ntree, one of the special refnames like \"HEAD\" or \"FETCH_HEAD\", or\nwhether it is potentially a valid fragment of a refname that can be\nDWIMed into a true reference.  Until now such checks have been\nincomplete and/or scattered around.\n\nThe first three patches in this series beef up check_refname_format()\n(and adds another static function, parse_refname_prefix()) with the\nability to make such checks when the REFNAME_FULL flag is used.\n\nThe fourth patch removes the checking of refnames passed to\nadd_extra_ref(), allowing the function to tolerate oddities like\n\"refs/tags/3.1.1.1^{}\".\n\nThe rest of the patches consist of wild-assed guesses about some\ncallers of check_refname_format() that I suppose can use the\nREFNAME_FULL option, thereby tightening up what they accept.  About\nall I can say is that the test suite passes with these patches\napplied.  But recent experience indicates that the test suite has a\nlot of holes.  Therefore, it would be great if experts would look over\nthese suggestions.\n\nThere are many other callers of check_refname_format() that I haven't\ntouched, because I'm not even brave enough to make wild-assed guesses\nabout them.  (Since I left them without the REFNAME_FULL option, they\nrather allow too many references through than too few.)  It would be\ngreat if a more experienced developer would look at the other callers\nand decide which need to be changed.\n\nBTW, this patch series does *not* fix the specific problem that Junio\nmentioned (that \"git update-ref tmp/junk HEAD\" does not reject the\nbogus refname), nor probably many others.  The gruelling work is not\nthis patch series; it is the effort of tracking down all of the code\npaths that might try to pass bogus refnames to the refs API.\n\nThis patch series applies on top of jc/check-ref-format-fixup, and\nalso rebases smoothly to the current \"next\".\n\nMichael Haggerty (13):\n  check_refname_component(): iterate via index rather than via pointer\n  parse_refname_prefix(): new function\n  Teach check_refname_format() to check full refnames\n  add_ref(): move the call of check_refname_format() to callers\n  receive-pack::update(): use check_refname_format(..., REFNAME_FULL)\n  strbuf_check_branch_ref(): use check_refname_format(...,\n    REFNAME_FULL)\n  one_local_ref(): use check_refname_format(..., REFNAME_FULL)\n  expand_namespace(): the refname is full, so use REFNAME_FULL option\n  new_branch(): verify that new branch name is a valid full refname\n  strbuf_check_tag_ref(): the refname is full, so use REFNAME_FULL\n    option\n  replace_object(): the refname is full, so use REFNAME_FULL option\n  resolve_ref: use check_refname_format(..., REFNAME_FULL)\n  filter_refs(): the refname is full, so use REFNAME_FULL option\n\n Documentation/git-check-ref-format.txt |   14 ++++\n builtin/check-ref-format.c             |    4 +\n builtin/fetch-pack.c                   |    2 +-\n builtin/receive-pack.c                 |    2 +-\n builtin/replace.c                      |    2 +-\n builtin/tag.c                          |    2 +-\n environment.c                          |    2 +-\n fast-import.c                          |    2 +-\n refs.c                                 |  124 ++++++++++++++++++++------------\n refs.h                                 |   12 ++-\n remote.c                               |    2 +-\n sha1_name.c                            |    2 +-\n t/t1402-check-ref-format.sh            |   31 ++++++++\n 13 files changed, 143 insertions(+), 58 deletions(-)\n\n-- \n1.7.7\n"},{"id":"178022","messageId":"1319057716-28094-2-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 01/13] check_refname_component(): iterate via index rather than via pointer","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:04Z","receivedAt":"2011-10-19T20:55:04Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThis will make later changes easier.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   14 +++++++-------\n 1 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0d74d70..3b41252 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -912,11 +912,11 @@ static inline int bad_ref_char(int ch)\n  */\n static int check_refname_component(const char *ref, int flags)\n {\n-\tconst char *cp;\n+\tint i, len = strlen(ref);\n \tchar last = '\\0';\n \n-\tfor (cp = ref; ; cp++) {\n-\t\tchar ch = *cp;\n+\tfor (i = 0; i < len; i++) {\n+\t\tchar ch = ref[i];\n \t\tif (ch == '\\0' || ch == '/')\n \t\t\tbreak;\n \t\tif (bad_ref_char(ch))\n@@ -927,7 +927,7 @@ static int check_refname_component(const char *ref, int flags)\n \t\t\treturn -1; /* Refname contains \"@{\". */\n \t\tlast = ch;\n \t}\n-\tif (cp == ref)\n+\tif (i == 0)\n \t\treturn -1; /* Component has zero length. */\n \tif (ref[0] == '.') {\n \t\tif (!(flags & REFNAME_DOT_COMPONENT))\n@@ -936,12 +936,12 @@ static int check_refname_component(const char *ref, int flags)\n \t\t * Even if leading dots are allowed, don't allow \".\"\n \t\t * as a component (\"..\" is prevented by a rule above).\n \t\t */\n-\t\tif (ref[1] == '\\0')\n+\t\tif (i == 1)\n \t\t\treturn -1; /* Component equals \".\". */\n \t}\n-\tif (cp - ref >= 5 && !memcmp(cp - 5, \".lock\", 5))\n+\tif (i >= 5 && !memcmp(&ref[i - 5], \".lock\", 5))\n \t\treturn -1; /* Refname ends with \".lock\". */\n-\treturn cp - ref;\n+\treturn i;\n }\n \n int check_refname_format(const char *ref, int flags)\n-- \n1.7.7\n"},{"id":"178015","messageId":"1319057716-28094-3-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 02/13] parse_refname_prefix(): new function","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:05Z","receivedAt":"2011-10-19T20:55:05Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nAdd a function parse_refname_prefix() that can read a possible refname\nfrom the front of a string.  This is like check_refname_format(),\nexcept:\n\n* It accepts (string, len) parameters and can therefore handle\n  non-NUL-terminated strings.\n\n* It returns the length of the part of the string that was parsed,\n  allowing it to be used for refnames that are followed by additional\n  characters.\n\nRe-implement check_refname_format() using the new function.\n\nRename function check_refname_component() to parse_refname_component()\nfor consistency.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   64 ++++++++++++++++++++++++++++++++++++++++++++--------------------\n 1 files changed, 44 insertions(+), 20 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 3b41252..387da83 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -907,24 +907,30 @@ static inline int bad_ref_char(int ch)\n \n /*\n  * Try to read one refname component from the front of ref.  Return\n- * the length of the component found, or -1 if the component is not\n- * legal.\n+ * the length of the component found, or -1 if the start of the string\n+ * cannot be interpreted as a component of a legal refname.\n  */\n-static int check_refname_component(const char *ref, int flags)\n+static int parse_refname_component(const char *ref, int len, int flags)\n {\n-\tint i, len = strlen(ref);\n+\tint i;\n \tchar last = '\\0';\n \n \tfor (i = 0; i < len; i++) {\n \t\tchar ch = ref[i];\n-\t\tif (ch == '\\0' || ch == '/')\n-\t\t\tbreak;\n+\t\tif (ch == '/')\n+\t\t\tbreak; /* Component terminated by \"..\". */\n \t\tif (bad_ref_char(ch))\n-\t\t\treturn -1; /* Illegal character in refname. */\n-\t\tif (last == '.' && ch == '.')\n-\t\t\treturn -1; /* Refname contains \"..\". */\n-\t\tif (last == '@' && ch == '{')\n-\t\t\treturn -1; /* Refname contains \"@{\". */\n+\t\t\tbreak; /* Component terminated by illegal character. */\n+\t\tif (last == '.' && ch == '.') {\n+\t\t\t/* Component terminated by \"..\". */\n+\t\t\ti--;\n+\t\t\tbreak;\n+\t\t}\n+\t\tif (last == '@' && ch == '{') {\n+\t\t\t/* Component terminated by \"@{\". */\n+\t\t\ti--;\n+\t\t\tbreak;\n+\t\t}\n \t\tlast = ch;\n \t}\n \tif (i == 0)\n@@ -944,17 +950,26 @@ static int check_refname_component(const char *ref, int flags)\n \treturn i;\n }\n \n-int check_refname_format(const char *ref, int flags)\n+/*\n+ * Try to interpret the beginning of a string as a refname.  Return\n+ * the length of the part of the string that could constitute a valid\n+ * refname, or -1 if the start of the string cannot possibly be\n+ * interpreted as a refname.  flags has the same interpretation as for\n+ * check_refname_format().\n+ */\n+static int parse_refname_prefix(const char *ref, int len, int flags)\n {\n \tint component_len, component_count = 0;\n+\tconst char *p = ref;\n+\tint valid_len;\n \n \twhile (1) {\n-\t\t/* We are at the start of a path component. */\n-\t\tcomponent_len = check_refname_component(ref, flags);\n+\t\t/* p is at the start of a path component. */\n+\t\tcomponent_len = parse_refname_component(p, len, flags);\n \t\tif (component_len < 0) {\n \t\t\tif ((flags & REFNAME_REFSPEC_PATTERN) &&\n-\t\t\t\t\tref[0] == '*' &&\n-\t\t\t\t\t(ref[1] == '\\0' || ref[1] == '/')) {\n+\t\t\t\t\tlen && p[0] == '*' &&\n+\t\t\t\t\t(len == 1 || p[1] == '/')) {\n \t\t\t\t/* Accept one wildcard as a full refname component. */\n \t\t\t\tflags &= ~REFNAME_REFSPEC_PATTERN;\n \t\t\t\tcomponent_len = 1;\n@@ -963,17 +978,26 @@ int check_refname_format(const char *ref, int flags)\n \t\t\t}\n \t\t}\n \t\tcomponent_count++;\n-\t\tif (ref[component_len] == '\\0')\n+\t\tif (component_len == len || p[component_len] != '/')\n \t\t\tbreak;\n \t\t/* Skip to next component. */\n-\t\tref += component_len + 1;\n+\t\tp += component_len + 1;\n+\t\tlen -= component_len + 1;\n \t}\n \n-\tif (ref[component_len - 1] == '.')\n+\tvalid_len = p + component_len - ref;\n+\n+\tif (ref[valid_len - 1] == '.')\n \t\treturn -1; /* Refname ends with '.'. */\n \tif (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)\n \t\treturn -1; /* Refname has only one component. */\n-\treturn 0;\n+\treturn valid_len;\n+}\n+\n+int check_refname_format(const char *ref, int flags)\n+{\n+\tint len = strlen(ref);\n+\treturn parse_refname_prefix(ref, len, flags) == len ? 0 : -1;\n }\n \n const char *prettify_refname(const char *name)\n-- \n1.7.7\n"},{"id":"178021","messageId":"1319057716-28094-4-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 03/13] Teach check_refname_format() to check full refnames","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:06Z","receivedAt":"2011-10-19T20:55:06Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nA full refname has to start with \"refs/\" or consist entirely of capital\nletters and underscores.  Add an option REFNAME_FULL that causes\ncheck_refname_format() and parse_refname_prefix() to verify that the\nrefname is a valid full refname.\n\nAdd a \"--full\" option to \"git check-ref-format\" that can be used to\naccess the new option.  Use this to add some tests involving \"git\ncheck-ref-format --full\".\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n Documentation/git-check-ref-format.txt |   14 +++++++++++\n builtin/check-ref-format.c             |    4 +++\n refs.c                                 |   39 ++++++++++++++++++-------------\n refs.h                                 |   12 ++++++---\n t/t1402-check-ref-format.sh            |   31 +++++++++++++++++++++++++\n 5 files changed, 80 insertions(+), 20 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex 103e7b1..9a8aafb 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -27,6 +27,11 @@ if refs are packed by `git gc`).\n \n git imposes the following rules on how references are named:\n \n+. They must either start with `refs/` (e.g., `refs/heads/master`) or\n+  consist entirely of capital letters and underscores (e.g., `HEAD` or\n+  `MERGE_HEAD`).  This rule is enforced if the `--full` option is\n+  used.\n+\n . They can include slash `/` for hierarchical (directory)\n   grouping, but no slash-separated component can begin with a\n   dot `.` or end with the sequence `.lock`.\n@@ -83,6 +88,15 @@ typed the branch name.\n \n OPTIONS\n -------\n+--full::\n+--no-full::\n+\tControls whether the refname must represent a full refname\n+\t(i.e., starting with `refs/`).  If `--full` and\n+\t`--allow-onelevel` are both specified, then special top-level\n+\trefnames consisting of capital letters and underscored (like\n+\t`HEAD` or `MERGE_HEAD`) are also allowed.  The default is\n+\t`--no-full`.\n+\n --allow-onelevel::\n --no-allow-onelevel::\n \tControls whether one-level refnames are accepted (i.e.,\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex 28a7320..a73626d 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -64,6 +64,10 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \tfor (i = 1; i < argc && argv[i][0] == '-'; i++) {\n \t\tif (!strcmp(argv[i], \"--normalize\") || !strcmp(argv[i], \"--print\"))\n \t\t\tnormalize = 1;\n+\t\telse if (!strcmp(argv[i], \"--full\"))\n+\t\t\tflags |= REFNAME_FULL;\n+\t\telse if (!strcmp(argv[i], \"--no-full\"))\n+\t\t\tflags &= ~REFNAME_FULL;\n \t\telse if (!strcmp(argv[i], \"--allow-onelevel\"))\n \t\t\tflags |= REFNAME_ALLOW_ONELEVEL;\n \t\telse if (!strcmp(argv[i], \"--no-allow-onelevel\"))\ndiff --git a/refs.c b/refs.c\nindex 387da83..8299e51 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -980,6 +980,11 @@ static int parse_refname_prefix(const char *ref, int len, int flags)\n \t\tcomponent_count++;\n \t\tif (component_len == len || p[component_len] != '/')\n \t\t\tbreak;\n+\t\tif (component_count == 1 && (flags & REFNAME_FULL)) {\n+\t\t\t/* That was the first of multiple components */\n+\t\t\tif (component_len != 4 || strncmp(p, \"refs\", 4))\n+\t\t\t\treturn -1;\n+\t\t}\n \t\t/* Skip to next component. */\n \t\tp += component_len + 1;\n \t\tlen -= component_len + 1;\n@@ -989,8 +994,22 @@ static int parse_refname_prefix(const char *ref, int len, int flags)\n \n \tif (ref[valid_len - 1] == '.')\n \t\treturn -1; /* Refname ends with '.'. */\n-\tif (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)\n-\t\treturn -1; /* Refname has only one component. */\n+\n+\tif (component_count == 1) {\n+\t\tif (!(flags & REFNAME_ALLOW_ONELEVEL))\n+\t\t\treturn -1;\n+\n+\t\tif (flags & REFNAME_FULL) {\n+\t\t\t/* Make sure that the refname is ALL_CAPS. */\n+\t\t\tint i;\n+\t\t\tfor (i = 0; i < component_len; i++) {\n+\t\t\t\tchar ch = p[i];\n+\t\t\t\tif (ch != '_' && (ch < 'A' || 'Z' < ch))\n+\t\t\t\t\treturn -1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \treturn valid_len;\n }\n \n@@ -1028,20 +1047,8 @@ const char *ref_fetch_rules[] = {\n \n static int refname_ok_at_root_level(const char *str, int len)\n {\n-\tif (len >= 5 && !memcmp(str, \"refs/\", 5))\n-\t\treturn 1;\n-\n-\twhile (len--) {\n-\t\tchar ch = *str++;\n-\n-\t\t/*\n-\t\t * Only accept likes of .git/HEAD, .git/MERGE_HEAD at\n-\t\t * the root level as a ref.\n-\t\t */\n-\t\tif (ch != '_' && (ch < 'A' || 'Z' < ch))\n-\t\t\treturn 0;\n-\t}\n-\treturn 1;\n+\treturn parse_refname_prefix(str, len,\n+\t\t\t\t    REFNAME_ALLOW_ONELEVEL|REFNAME_FULL) == len;\n }\n \n int refname_match(const char *abbrev_name, const char *full_name, const char **rules)\ndiff --git a/refs.h b/refs.h\nindex 0229c57..7707cda 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -97,9 +97,10 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi\n  */\n extern int for_each_reflog(each_ref_fn, void *);\n \n-#define REFNAME_ALLOW_ONELEVEL 1\n-#define REFNAME_REFSPEC_PATTERN 2\n-#define REFNAME_DOT_COMPONENT 4\n+#define REFNAME_ALLOW_ONELEVEL 01\n+#define REFNAME_REFSPEC_PATTERN 02\n+#define REFNAME_DOT_COMPONENT 04\n+#define REFNAME_FULL 010\n \n /*\n  * Return 0 iff ref has the correct format for a refname according to\n@@ -110,7 +111,10 @@ extern int for_each_reflog(each_ref_fn, void *);\n  * components.  No leading or repeated slashes are accepted.  If\n  * REFNAME_DOT_COMPONENT is set in flags, then allow refname\n  * components to start with \".\" (but not a whole component equal to\n- * \".\" or \"..\").\n+ * \".\" or \"..\").  If REFNAME_FULL is set in flags, then additionally\n+ * verify that the refname is a valid full refname--that it either\n+ * starts with \"refs/\" or that it consists of only capital letters and\n+ * underscores (like \"HEAD\" or \"MERGE_HEAD\").\n  */\n extern int check_refname_format(const char *ref, int flags);\n \ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex 1ae4d87..2f8a48e 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -70,6 +70,7 @@ invalid_ref \"$ref\" --refspec-pattern\n valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n invalid_ref \"$ref\" --normalize\n valid_ref \"$ref\" '--allow-onelevel --normalize'\n+invalid_ref \"$ref\" '--full'\n \n ref='foo/bar'\n valid_ref \"$ref\"\n@@ -77,12 +78,19 @@ valid_ref \"$ref\" --allow-onelevel\n valid_ref \"$ref\" --refspec-pattern\n valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n valid_ref \"$ref\" --normalize\n+invalid_ref \"$ref\" '--full'\n \n ref='foo/*'\n invalid_ref \"$ref\"\n invalid_ref \"$ref\" --allow-onelevel\n valid_ref \"$ref\" --refspec-pattern\n valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+invalid_ref \"$ref\" '--full'\n+invalid_ref \"$ref\" '--refspec-pattern --full'\n+\n+ref='refs/*'\n+invalid_ref \"$ref\" '--full'\n+valid_ref \"$ref\" '--refspec-pattern --full'\n \n ref='*/foo'\n invalid_ref \"$ref\"\n@@ -91,18 +99,23 @@ valid_ref \"$ref\" --refspec-pattern\n valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n invalid_ref \"$ref\" --normalize\n valid_ref \"$ref\" '--refspec-pattern --normalize'\n+invalid_ref \"$ref\" '--full'\n+invalid_ref \"$ref\" '--refspec-pattern --full'\n \n ref='foo/*/bar'\n invalid_ref \"$ref\"\n invalid_ref \"$ref\" --allow-onelevel\n valid_ref \"$ref\" --refspec-pattern\n valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+invalid_ref \"$ref\" '--full'\n \n ref='*'\n invalid_ref \"$ref\"\n invalid_ref \"$ref\" --allow-onelevel\n invalid_ref \"$ref\" --refspec-pattern\n valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+invalid_ref \"$ref\" '--full'\n+invalid_ref \"$ref\" '--refspec-pattern --full'\n \n ref='foo/*/*'\n invalid_ref \"$ref\" --refspec-pattern\n@@ -126,6 +139,24 @@ valid_ref NOT_MINGW \"$ref\" '--allow-onelevel --normalize'\n invalid_ref NOT_MINGW \"$ref\" '--refspec-pattern --normalize'\n valid_ref NOT_MINGW \"$ref\" '--refspec-pattern --allow-onelevel --normalize'\n \n+ref='HEAD'\n+invalid_ref \"$ref\"\n+valid_ref \"$ref\" --allow-onelevel\n+invalid_ref \"$ref\" --full\n+valid_ref \"$ref\" '--allow-onelevel --full'\n+\n+valid_ref FETCH_HEAD '--allow-onelevel --full'\n+valid_ref refs/foo --full\n+invalid_ref HEAD/ '--allow-onelevel --full'\n+valid_ref HEAD/ '--allow-onelevel --full --normalize'\n+invalid_ref refs '--full'\n+invalid_ref refs '--allow-onelevel --full'\n+invalid_ref refs/ '--allow-onelevel --full'\n+invalid_ref HEAD/foo '--full'\n+invalid_ref head '--allow-onelevel --full'\n+valid_ref 'refs/foo/*' '--refspec-pattern --full'\n+valid_ref 'refs/*/foo' '--refspec-pattern --full'\n+\n test_expect_success \"check-ref-format --branch @{-1}\" '\n \tT=$(git write-tree) &&\n \tsha1=$(echo A | git commit-tree $T) &&\n-- \n1.7.7\n"},{"id":"178017","messageId":"1319057716-28094-5-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 04/13] add_ref(): move the call of check_refname_format() to callers","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:07Z","receivedAt":"2011-10-19T20:55:07Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nDo not call check_refname_format() in add_ref(); instead change its\ncallers to do the check.  (In fact, don't do any checking in\nadd_extra_ref(), because that function handles bizarre things like\n\"refs/tags/3.1.1.1^{}\".)\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nI'm still not clear on how extra_refs are used.  Are they generated\nfrom local refs or are they generated from remote refs?  If the\nlatter, then it is probably irresponsible not to do *some* sanity\nchecking in add_extra_ref() to prevent any chance of refnames like\n\"../../../etc/passwd\".\n\n refs.c |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 8299e51..a40dfa5 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -60,8 +60,6 @@ static void add_ref(const char *name, const unsigned char *sha1,\n \tentry = xmalloc(sizeof(struct ref_entry) + len);\n \thashcpy(entry->sha1, sha1);\n \thashclr(entry->peeled);\n-\tif (check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))\n-\t\tdie(\"Reference has invalid format: '%s'\", name);\n \tmemcpy(entry->name, name, len);\n \tentry->flag = flag;\n \tif (new_entry)\n@@ -232,6 +230,8 @@ static void read_packed_refs(FILE *f, struct ref_array *array)\n \n \t\tname = parse_ref_line(refline, sha1);\n \t\tif (name) {\n+\t\t\tif (check_refname_format(name, REFNAME_FULL))\n+\t\t\t\tdie(\"Reference has invalid format: '%s'\", name);\n \t\t\tadd_ref(name, sha1, flag, array, &last);\n \t\t\tcontinue;\n \t\t}\n@@ -336,6 +336,8 @@ static void get_ref_dir(const char *submodule, const char *base,\n \t\t\t\t\thashclr(sha1);\n \t\t\t\t\tflag |= REF_BROKEN;\n \t\t\t\t}\n+\t\t\tif (check_refname_format(ref, REFNAME_FULL))\n+\t\t\t\tdie(\"Reference has invalid format: '%s'\", ref);\n \t\t\tadd_ref(ref, sha1, flag, array, NULL);\n \t\t}\n \t\tfree(ref);\n-- \n1.7.7\n"},{"id":"178014","messageId":"1319057716-28094-6-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 05/13] receive-pack::update(): use check_refname_format(..., REFNAME_FULL)","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:08Z","receivedAt":"2011-10-19T20:55:08Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nReplace local code with a use of check_refname_format()'s new\nREFNAME_FULL option.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/receive-pack.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 261b610..508451b 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -401,7 +401,7 @@ static const char *update(struct command *cmd)\n \tstruct ref_lock *lock;\n \n \t/* only refs/... are allowed */\n-\tif (prefixcmp(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n+\tif (check_refname_format(name, REFNAME_FULL)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n \t\treturn \"funny refname\";\n \t}\n-- \n1.7.7\n"},{"id":"178016","messageId":"1319057716-28094-7-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 06/13] strbuf_check_branch_ref(): use check_refname_format(..., REFNAME_FULL)","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:09Z","receivedAt":"2011-10-19T20:55:09Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThe reference name should be a full one, so adjust the check\naccordingly.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_name.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 03ffc2c..335da37 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -883,7 +883,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n \tif (name[0] == '-')\n \t\treturn -1;\n \tstrbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n-\treturn check_refname_format(sb->buf, 0);\n+\treturn check_refname_format(sb->buf, REFNAME_FULL);\n }\n \n /*\n-- \n1.7.7\n"},{"id":"178019","messageId":"1319057716-28094-8-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 07/13] one_local_ref(): use check_refname_format(..., REFNAME_FULL)","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:10Z","receivedAt":"2011-10-19T20:55:10Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nInstead of manually skipping over \"refs/\", let check_refname_format()\nhandle the whole refname.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n remote.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex e52aa9b..30ee6d8 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1595,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla\n \tint len;\n \n \t/* we already know it starts with refs/ to get here */\n-\tif (check_refname_format(refname + 5, 0))\n+\tif (check_refname_format(refname, REFNAME_FULL))\n \t\treturn 0;\n \n \tlen = strlen(refname) + 1;\n-- \n1.7.7\n"},{"id":"178020","messageId":"1319057716-28094-9-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 08/13] expand_namespace(): the refname is full, so use REFNAME_FULL option","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:11Z","receivedAt":"2011-10-19T20:55:11Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n environment.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 0bee6a7..a6d6f04 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -107,7 +107,7 @@ static char *expand_namespace(const char *raw_namespace)\n \t\tif (strcmp((*c)->buf, \"/\") != 0)\n \t\t\tstrbuf_addf(&buf, \"refs/namespaces/%s\", (*c)->buf);\n \tstrbuf_list_free(components);\n-\tif (check_refname_format(buf.buf, 0))\n+\tif (check_refname_format(buf.buf, REFNAME_FULL))\n \t\tdie(\"bad git namespace path \\\"%s\\\"\", raw_namespace);\n \tstrbuf_addch(&buf, '/');\n \treturn strbuf_detach(&buf, NULL);\n-- \n1.7.7\n"},{"id":"178018","messageId":"1319057716-28094-10-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 09/13] new_branch(): verify that new branch name is a valid full refname","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:12Z","receivedAt":"2011-10-19T20:55:12Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nIs it possible to omit the REFNAME_ALLOW_ONELEVEL option from this\ncall?\n\n fast-import.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 8d8ea3c..51cf898 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -722,7 +722,7 @@ static struct branch *new_branch(const char *name)\n \n \tif (b)\n \t\tdie(\"Invalid attempt to create duplicate branch: %s\", name);\n-\tif (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))\n+\tif (check_refname_format(name, REFNAME_FULL|REFNAME_ALLOW_ONELEVEL))\n \t\tdie(\"Branch name doesn't conform to GIT standards: %s\", name);\n \n \tb = pool_calloc(1, sizeof(struct branch));\n-- \n1.7.7\n"},{"id":"178023","messageId":"1319057716-28094-11-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 10/13] strbuf_check_tag_ref(): the refname is full, so use REFNAME_FULL option","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:13Z","receivedAt":"2011-10-19T20:55:13Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/tag.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 9b6fd95..abf3cb0 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -412,7 +412,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n \tstrbuf_reset(sb);\n \tstrbuf_addf(sb, \"refs/tags/%s\", name);\n \n-\treturn check_refname_format(sb->buf, 0);\n+\treturn check_refname_format(sb->buf, REFNAME_FULL);\n }\n \n int cmd_tag(int argc, const char **argv, const char *prefix)\n-- \n1.7.7\n"},{"id":"178024","messageId":"1319057716-28094-12-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 11/13] replace_object(): the refname is full, so use REFNAME_FULL option","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:14Z","receivedAt":"2011-10-19T20:55:14Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/replace.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 517fa10..f5d4f77 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,\n \t\t     \"refs/replace/%s\",\n \t\t     sha1_to_hex(object)) > sizeof(ref) - 1)\n \t\tdie(\"replace ref name too long: %.*s...\", 50, ref);\n-\tif (check_refname_format(ref, 0))\n+\tif (check_refname_format(ref, REFNAME_FULL))\n \t\tdie(\"'%s' is not a valid ref name.\", ref);\n \n \tif (!resolve_ref(ref, prev, 1, NULL))\n-- \n1.7.7\n"},{"id":"178026","messageId":"1319057716-28094-13-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 12/13] resolve_ref: use check_refname_format(..., REFNAME_FULL)","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:15Z","receivedAt":"2011-10-19T20:55:15Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n...instead of own inline code.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a40dfa5..67df3a6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -548,8 +548,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *\n \t\t\tif (len < 0)\n \t\t\t\treturn NULL;\n \t\t\tbuffer[len] = 0;\n-\t\t\tif (!prefixcmp(buffer, \"refs/\") &&\n-\t\t\t\t\t!check_refname_format(buffer, 0)) {\n+\t\t\tif (!check_refname_format(buffer, REFNAME_FULL)) {\n \t\t\t\tstrcpy(ref_buffer, buffer);\n \t\t\t\tref = ref_buffer;\n \t\t\t\tif (flag)\n-- \n1.7.7\n"},{"id":"178025","messageId":"1319057716-28094-14-git-send-email-mhagger@alum.mit.edu","threadId":"28722","inReplyTo":"1319057716-28094-1-git-send-email-mhagger@alum.mit.edu","subject":"[RFC 13/13] filter_refs(): the refname is full, so use REFNAME_FULL option","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T20:55:16Z","receivedAt":"2011-10-19T20:55:16Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/fetch-pack.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex c6bc8eb..d72aa44 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -546,7 +546,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n \tfor (ref = *refs; ref; ref = next) {\n \t\tnext = ref->next;\n \t\tif (!memcmp(ref->name, \"refs/\", 5) &&\n-\t\t    check_refname_format(ref->name + 5, 0))\n+\t\t    check_refname_format(ref->name, REFNAME_FULL))\n \t\t\t; /* trash */\n \t\telse if (args.fetch_all &&\n \t\t\t (!args.depth || prefixcmp(ref->name, \"refs/tags/\") )) {\n-- \n1.7.7\n"},{"id":"178045","messageId":"7vsjmobqim.fsf@alter.siamese.dyndns.org","threadId":"28722","inReplyTo":"1319057716-28094-5-git-send-email-mhagger@alum.mit.edu","subject":"Re: [RFC 04/13] add_ref(): move the call of check_refname_format() to callers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-19T21:49:53Z","receivedAt":"2011-10-19T21:49:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"mhagger@alum.mit.edu writes:\n\n> I'm still not clear on how extra_refs are used.  Are they generated\n> from local refs or are they generated from remote refs?  If the\n> latter, then it is probably irresponsible not to do *some* sanity\n> checking in add_extra_ref() to prevent any chance of refnames like\n> \"../../../etc/passwd\".\n\nNo, add_extra_ref() already tells us what their values are, these are\nnever used to actually read from filesystem. Their refname field has\nalmost no value other than for debugging and we probably shouldn't even\ninsist on uniqueness among extra refs or for that matter collision with\nthe real refs. As I mentioned in an earlier message, their only raison\nd'être is to be found by for_each_ref() that feeds revision machinery with\nup to which commits we know we have complete histories for. A sample call\nchain looks like this:\n\n - cmd_clone()\n  - setup_reference()\n   - add_one_reference()\n     - add_extra_ref()\n       adds refs in other repositories we borrow from as \"extra\"\n  - transport_fetch_refs()\n   - fetch_refs_via_pack()\n    - fetch_pack()\n     - do_fetch_pack()\n      - find_common()\n       - for_each_ref(rev_list_insert_ref)\n\nThat way find_common() thinks histories leading to these extra refs are\nalready complete on our end (i.e. we have all the necessary objects), and\nby subtracting that from what we are asking from the other end, we can\nreduce the amount of history that needs to be transferred.\n"},{"id":"178049","messageId":"7vobxcbqeu.fsf@alter.siamese.dyndns.org","threadId":"28722","inReplyTo":"1319057716-28094-10-git-send-email-mhagger@alum.mit.edu","subject":"Re: [RFC 09/13] new_branch(): verify that new branch name is a valid full refname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-19T21:52:09Z","receivedAt":"2011-10-19T21:52:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"mhagger@alum.mit.edu writes:\n\n> From: Michael Haggerty <mhagger@alum.mit.edu>\n>\n>\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n>\n> Is it possible to omit the REFNAME_ALLOW_ONELEVEL option from this\n> call?\n\nI _think_ it takes an unadorned branch name, so the most prudent would be\nto check the result of prefixing \"refs/heads/\" to *name with REFNAME_FULL.\n\n>  fast-import.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/fast-import.c b/fast-import.c\n> index 8d8ea3c..51cf898 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -722,7 +722,7 @@ static struct branch *new_branch(const char *name)\n>  \n>  \tif (b)\n>  \t\tdie(\"Invalid attempt to create duplicate branch: %s\", name);\n> -\tif (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))\n> +\tif (check_refname_format(name, REFNAME_FULL|REFNAME_ALLOW_ONELEVEL))\n>  \t\tdie(\"Branch name doesn't conform to GIT standards: %s\", name);\n>  \n>  \tb = pool_calloc(1, sizeof(struct branch));\n"},{"id":"178051","messageId":"4E9F4837.5070108@alum.mit.edu","threadId":"28722","inReplyTo":"7vsjmobqim.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC 04/13] add_ref(): move the call of check_refname_format() to callers","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-10-19T21:59:19Z","receivedAt":"2011-10-19T21:59:19Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 10/19/2011 11:49 PM, Junio C Hamano wrote:\n> mhagger@alum.mit.edu writes:\n>> I'm still not clear on how extra_refs are used.  Are they generated\n>> from local refs or are they generated from remote refs?  If the\n>> latter, then it is probably irresponsible not to do *some* sanity\n>> checking in add_extra_ref() to prevent any chance of refnames like\n>> \"../../../etc/passwd\".\n> \n> No, add_extra_ref() already tells us what their values are, these are\n> never used to actually read from filesystem. Their refname field has\n> almost no value other than for debugging and we probably shouldn't even\n> insist on uniqueness among extra refs or for that matter collision with\n> the real refs. [...]\n\nThanks for the explanation.  I'm inspired to separate them a little bit\nmore from \"real\" refs because they are such a special case.  For\nexample, maybe it would make sense to add a function\nfor_each_extra_ref() to avoid having to mix them with real refs in the\niteration.  OTOH not important AFAICS.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}