{"thread":{"id":"36852","subject":"[PATCH v3] receive-pack: optionally deny case clone refs","startedAt":"2014-06-06T00:52:34Z","lastAt":"2014-06-06T03:01:15Z","messageCount":3,"participants":["David Turner","Duy Nguyen"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"243440","messageId":"1402015954-5320-1-git-send-email-dturner@twitter.com","threadId":"36852","inReplyTo":null,"subject":"[PATCH v3] receive-pack: optionally deny case clone refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-06-06T00:52:34Z","receivedAt":"2014-06-06T00:52:34Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"It is possible to have two refs which are the same but for case.\nThis works great on the case-sensitive filesystems, but not so well on\ncase-insensitive filesystems.  It is fairly typical to have\ncase-insensitive clients (Macs, say) with a case-sensitive server\n(GNU/Linux).\n\nShould a user attempt to pull on a Mac when there are case clone refs\nwith differing contents, they'll get an error message containing\nsomething like \"Ref refs/remotes/origin/lower is at\n[sha-of-lowercase-ref] but expected [sha-of-uppercase-ref]....\n(unable to update local ref)\"\n\nWith a case-insensitive git server, if a branch called capital-M\nMaster (that differs from lowercase-m-master) is pushed, nobody else\ncan push to (lowercase-m) master until the branch is removed.\n\nCreate the option receive.denycaseclonerefs, which checks pushed\nrefs to ensure that they are not case clones of an existing\nref.  This setting is turned on by default if core.ignorecase is\nset, but not otherwise.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n Documentation/config.txt           |  6 +++++\n Documentation/git-push.txt         |  5 +++--\n Documentation/glossary-content.txt |  5 +++++\n builtin/receive-pack.c             | 27 +++++++++++++++++++++-\n t/t5400-send-pack.sh               | 46 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 86 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1932e9b..b24b117 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2053,6 +2053,12 @@ receive.unpackLimit::\n \tespecially on slow filesystems.  If not set, the value of\n \t`transfer.unpackLimit` is used instead.\n \n+receive.denyCaseCloneRefs::\n+\tIf set to true, git-receive-pack will deny a ref update that creates\n+\ta ref which is the same but for case as an existing ref.  This is\n+\tuseful when clients are on a case-insensitive filesystem, which\n+\twill cause errors when given refs which differ only in case.\n+\n receive.denyDeletes::\n \tIf set to true, git-receive-pack will deny a ref update that deletes\n \tthe ref. Use this to prevent such a ref deletion via a push.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 21cd455..c92c3a6 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -323,8 +323,9 @@ remote rejected::\n \tof the following safety options in effect:\n \t`receive.denyCurrentBranch` (for pushes to the checked out\n \tbranch), `receive.denyNonFastForwards` (for forced\n-\tnon-fast-forward updates), `receive.denyDeletes` or\n-\t`receive.denyDeleteCurrent`.  See linkgit:git-config[1].\n+\tnon-fast-forward updates), `receive.denyDeletes`,\n+\t`receive.denyCaseCloneRefs` or `receive.denyDeleteCurrent`.\n+\tSee linkgit:git-config[1].\n \n remote failure::\n \tThe remote end did not report the successful update of the ref,\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex be0858c..ed5ac23 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -31,6 +31,11 @@\n [[def_cache]]cache::\n \tObsolete for: <<def_index,index>>.\n \n+[[def_case_clone]]case clone::\n+\tTwo entities (e.g. filenames or refs) that differ only in case.\n+\tThese can cause problems on case-insensitive filesystems, and\n+\tGit has machinery to prevent these problems in various cases.\n+\n [[def_chain]]chain::\n \tA list of objects, where each <<def_object,object>> in the list contains\n \ta reference to its successor (for example, the successor of a\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex c323081..8530a6c 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -27,6 +27,7 @@ enum deny_action {\n \n static int deny_deletes;\n static int deny_non_fast_forwards;\n+static int deny_case_clone_refs = DENY_UNCONFIGURED;\n static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n static enum deny_action deny_delete_current = DENY_UNCONFIGURED;\n static int receive_fsck_objects = -1;\n@@ -69,6 +70,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \tif (status)\n \t\treturn status;\n \n+\tif (strcmp(var, \"receive.denycaseclonerefs\") == 0) {\n+\t\tdeny_case_clone_refs = parse_deny_action(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (strcmp(var, \"receive.denydeletes\") == 0) {\n \t\tdeny_deletes = git_config_bool(var, value);\n \t\treturn 0;\n@@ -468,6 +474,22 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)\n \treturn 0;\n }\n \n+static int is_case_clone(const char *refname, const unsigned char *sha1,\n+\t\t\tint flags, void *cb_data)\n+{\n+\tconst char *incoming_refname = cb_data;\n+\treturn !strcasecmp(refname, incoming_refname) &&\n+\t\tstrcmp(refname, incoming_refname);\n+}\n+\n+static int ref_is_denied_case_clone(const char *name)\n+{\n+\tif (!deny_case_clone_refs)\n+\t\treturn 0;\n+\n+\treturn for_each_ref(is_case_clone, (void *) name);\n+}\n+\n static const char *update(struct command *cmd, struct shallow_info *si)\n {\n \tconst char *name = cmd->ref_name;\n@@ -478,7 +500,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \tstruct ref_lock *lock;\n \n \t/* only refs/... are allowed */\n-\tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n+\tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0) ||\n+\t    ref_is_denied_case_clone(name)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n \t\treturn \"funny refname\";\n \t}\n@@ -1171,6 +1194,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\tdie(\"'%s' does not appear to be a git repository\", dir);\n \n \tgit_config(receive_pack_config, NULL);\n+\tif (deny_case_clone_refs == DENY_UNCONFIGURED)\n+\t\tdeny_case_clone_refs = ignore_case;\n \n \tif (0 <= transfer_unpack_limit)\n \t\tunpack_limit = transfer_unpack_limit;\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 0736bcb..a7023dc 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -129,6 +129,52 @@ test_expect_success 'denyNonFastforwards trumps --force' '\n \ttest \"$victim_orig\" = \"$victim_head\"\n '\n \n+test_expect_success 'denyCaseCloneRefs works' '\n+\t(\n+\t    cd victim &&\n+\t    git config receive.denyCaseCloneRefs true &&\n+\t    git config receive.denyDeletes false\n+\t) &&\n+\tgit send-pack ./victim HEAD:refs/heads/caseclone &&\n+\torig_ver=$(git rev-parse HEAD) &&\n+\ttest_must_fail git send-pack ./victim HEAD^:refs/heads/CaseClone &&\n+\t# confirm that this had no effect upstream\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/CaseClone) &&\n+\t    echo \"$ref\" | test_must_fail grep -q CaseClone &&\n+\t    remote_ver=$(git rev-parse caseclone) &&\n+\t    test \"$orig_ver\" = \"$remote_ver\"\n+\t) &&\n+\tgit send-pack ./victim HEAD^:refs/heads/notacaseclone &&\n+\ttest_must_fail git send-pack ./victim :CaseClone &&\n+\t# confirm that this had no effect upstream\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/CaseClone) &&\n+\t    echo \"$ref\" | test_must_fail grep -q CaseClone &&\n+\t    remote_ver=$(git rev-parse caseclone) &&\n+\t    test \"$orig_ver\" = \"$remote_ver\"\n+\t) &&\n+\tgit send-pack ./victim :caseclone &&\n+\t# confirm that this took effect upstream\n+\t(\n+\t    cd victim &&\n+\t    test_must_fail git rev-parse caseclone\n+\t) &&\n+\t# check that we can recreate a branch after deleting a\n+\t# case-clone of it\n+\tcase_clone_ver=$(git rev-parse HEAD^) &&\n+\tgit send-pack ./victim HEAD^:refs/heads/CaseClone &&\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/caseclone) &&\n+\t    test_echo \"$ref\" | test_must_fail grep -q caseclone  &&\n+\t    remote_ver=$(git rev-parse CaseClone) &&\n+\t    test \"$case_clone_ver\" = \"$remote_ver\"\n+\t)\n+'\n+\n test_expect_success 'push --all excludes remote-tracking hierarchy' '\n \tmkdir parent &&\n \t(\n-- \n2.0.0.rc1.24.g0588c94.dirty\n"},{"id":"243442","messageId":"CACsJy8ALeYR1+5ox2GE2Rz3TRM5wqd9fxSCAnHoabo9fmuSdXQ@mail.gmail.com","threadId":"36852","inReplyTo":"1402015954-5320-1-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v3] receive-pack: optionally deny case clone refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-06T01:37:55Z","receivedAt":"2014-06-06T01:37:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jun 6, 2014 at 7:52 AM, David Turner <dturner@twopensource.com> wrote:\n> Create the option receive.denycaseclonerefs, which checks pushed\n> refs to ensure that they are not case clones of an existing\n> ref.  This setting is turned on by default if core.ignorecase is\n> set, but not otherwise.\n\nJust thinking out loud as I haven't had time to read this patch\ncarefully, but I wonder if there's a potential performance problem\nwith ref_is_denied_case_clone(). What if the receiver repo has 60k\nrefs? Should we use name-hash (or a variant of it)? What if the sender\npushes 60k refs in one go, will ref_is_denied_case_clone check against\nexisting refs only, or it will cover more and more refs from the 60k\ninput?\n-- \nDuy\n"},{"id":"243443","messageId":"1402023675.18134.192.camel@stross","threadId":"36852","inReplyTo":"CACsJy8ALeYR1+5ox2GE2Rz3TRM5wqd9fxSCAnHoabo9fmuSdXQ@mail.gmail.com","subject":"Re: [PATCH v3] receive-pack: optionally deny case clone refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-06-06T03:01:15Z","receivedAt":"2014-06-06T03:01:15Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2014-06-06 at 08:37 +0700, Duy Nguyen wrote:\n> On Fri, Jun 6, 2014 at 7:52 AM, David Turner <dturner@twopensource.com> wrote:\n> > Create the option receive.denycaseclonerefs, which checks pushed\n> > refs to ensure that they are not case clones of an existing\n> > ref.  This setting is turned on by default if core.ignorecase is\n> > set, but not otherwise.\n> \n> Just thinking out loud as I haven't had time to read this patch\n> carefully, but I wonder if there's a potential performance problem\n> with ref_is_denied_case_clone(). What if the receiver repo has 60k\n> refs? Should we use name-hash (or a variant of it)? What if the sender\n> pushes 60k refs in one go, will ref_is_denied_case_clone check against\n> existing refs only, or it will cover more and more refs from the 60k\n> input?\n\nGood news!  The sender can't push 60k refs in one go because that would\nrequire a command-line bigger than ARG_MAX.  (Well, OK, this is not\nreally good news, since of course someone could hack around this, and\nsince this is minor bug in git that we should probably fix).\n\nI guess this does present a bit of a DOS attack.  When I discussed the\nprevious version of this patch with Junio, I did not think it was a huge\nproblem, since the the ordinary number of refs pushed is small.  But\nwhen I think of it as a DOS, it sounds much worse.\n\nI don't like the idea of creating a hashmap just for this, because\nreally we need to be storing packed refs in an better on-disk format --\none that allows O(log n) or better access (as has been discussed).  \n\nI'll give this some more thought and see if I can come up with a better\nsolution.\n"}]}