On 12.06.14 00:30, David Turner wrote:
[]
Just a general question:
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index c323081..6431758 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -27,6 +27,7 @@ enum deny_action {
>
> static int deny_deletes;
> static int deny_non_fast_forwards;
> +static int deny_case_clone_refs = DENY_UNCONFIGURED;
> static enum deny_action deny_current_branch = DENY_UNCONFIGURED;
> static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
> static int receive_fsck_objects = -1;
> @@ -69,6 +70,11 @@ static int receive_pack_config(const char *var, const char
> *value, void *cb)
> if (status)
> return status;
>
> + if (strcmp(var, "receive.denycaseclonerefs") == 0) {
> + deny_case_clone_refs = parse_deny_action(var, value);
_action() : Which action ?
May be this is a better name:
parse_deny_case_clone_refs()
> + return 0;
> + }
> +
> if (strcmp(var, "receive.denydeletes") == 0) {
> deny_deletes = git_config_bool(var, value);
> return 0;
> @@ -468,6 +474,138 @@ static int update_shallow_ref(struct command *cmd,
> struct shallow_info *si)
> return 0;
> }
>
> +/*
> + * This contains not just refs, but ref prefixes -- i.e. not just
> + * refs/heads/foo/bar, but refs, refs/heads, and refs/heads/foo
> + */
> +struct ref_cache_entry {
> + struct hashmap_entry ent;
> + unsigned int count; /* count of refs having this as a component */
> + char ref[FLEX_ARRAY];
> +};
> +
> +static struct hashmap *ref_case_clone_cache;
> +
> +static int ref_cache_entry_cmp(const struct ref_cache_entry *e1,
> + const struct ref_cache_entry *e2, const char
> *ref)
> +{
> + return strcasecmp(e1->ref, ref ? ref : e2->ref);
> +}
> +
> +/*
> + * Insert a ref into the ref cache, as well as all of its ancestor
> + * directory names -- so if we insert refs/heads/something/other,
> + * refs, refs/heads, refs/heads/something/other will be included.
> + */
> +static int ref_cache_insert(const char *refname, struct hashmap *map)
> +{
> + int total_len = 0, comp_len;
> +
> + while ((comp_len = check_refname_component(refname + total_len, 0)) >=
> 0) {
> + struct ref_cache_entry *old;
> + struct ref_cache_entry *entry = xmalloc(sizeof(*entry) +
> total_len + comp_len + 1);
> + total_len += comp_len;
Could the total length can be calculated first and already used in xmalloc() ?
That will give 3 lines of code, but the reader is sure we are allocating the
right length.
> + struct ref_cache_entry;
> + total_len += comp_len;
> + entry = xmalloc(sizeof(*entry) + total_len + 1);
> + memcpy(entry->ref, refname, total_len);
> + entry->ref[total_len] = 0;
> + entry->count = 1;
> + hashmap_entry_init(entry, memihash(entry->ref, total_len));
> + old = hashmap_get(map, entry, entry->ref);
> + if (old) {
> + old->count ++;
> + free(entry);
I'm not sure if I read it right:
If there is an old entry "old", we anyway create a new one and delete it
immediately ?
> + } else
> + hashmap_add(map, entry);
> + total_len ++;
> + }
> +}
> +
> +/*
> + * Remove a ref from the ref cache, as well as any of its ancestor
> + * directory names that no longer contain any refs.
> + */
> +static int ref_cache_delete(const char *refname, struct hashmap *map)
> +{
> + int total_len = 0, comp_len;
> +
> + struct ref_cache_entry *entry = xmalloc(sizeof(*entry) +
> strlen(refname));
> +
> + while ((comp_len = check_refname_component(refname + total_len, 0)) >=
> 0) {
> + struct ref_cache_entry *old;
> + total_len += comp_len;
> + memcpy(entry->ref, refname, total_len);
> + entry->ref[total_len] = 0;
> + hashmap_entry_init(entry, memihash(entry->ref, total_len));
> + old = hashmap_get(map, entry, entry->ref);
> + if (old) {
> + old->count --;
> + if (old->count == 0) {
> + hashmap_remove(map, old, old->ref);
> + free(old);
> + }
> + } else {
> + warn("Ref cache coherency failure: %s from %s",
> entry->ref, refname);
> + break;
> + }
> + total_len ++;
> + }
> + free(entry);
> +}
> +
> +
> +static int ref_cache_insert_cb(const char *refname, const unsigned char
> *sha1,
> + int flags, void *cb_data)
> +{
> + ref_cache_insert(refname, cb_data);
> +}
> +
> +static void ensure_ref_case_clone_cache(void)
> +{
> + if (ref_case_clone_cache)
> + return;
> + ref_case_clone_cache = xmalloc(sizeof(*ref_case_clone_cache));
> + hashmap_init(ref_case_clone_cache,
> + (hashmap_cmp_fn)ref_cache_entry_cmp, 1000);
> +
> + for_each_ref(ref_cache_insert_cb, (void *)ref_case_clone_cache);
> +}
> +
> +/*
> + * Search the ref cache for a ref that is a case clone of this
> + * incoming ref; this includes prefix case clones so that
> + * refs/heads/case/clone will conflict with refs/heads/CASE/other
> + */
> +static int ref_is_case_clone(const char *name) {
> + struct ref_cache_entry key;
> + struct ref_cache_entry *existing;
> + int total_len = 0, comp_len;
> + char *name_so_far = strdup(name);
> +
> + while ((comp_len = check_refname_component(name + total_len, 0)) >= 0) {
> + total_len += comp_len;
> + name_so_far[total_len] = 0;
> + hashmap_entry_init(&key, memihash(name_so_far, total_len));
> + existing = hashmap_get(ref_case_clone_cache, &key, name_so_far);
> + if (!existing)
> + return 0;
> + if (memcmp(existing->ref, name_so_far, total_len))
> + return 1;
> + name_so_far[total_len] = '/';
> + total_len ++;
> + }
> +
> + free(name_so_far);
> + return 0;
> +}
> +
> +static int ref_is_denied_case_clone(const char *name)
> +{
> + if (!deny_case_clone_refs)
> + return 0;
> + ensure_ref_case_clone_cache();
> +
> + return ref_is_case_clone(name);
> +}
> +
> static const char *update(struct command *cmd, struct shallow_info *si)
> {
> const char *name = cmd->ref_name;
> @@ -478,7 +616,8 @@ static const char *update(struct command *cmd, struct
> shallow_info *si)
> struct ref_lock *lock;
>
> /* only refs/... are allowed */
> - if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
> + if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0) ||
> + ref_is_denied_case_clone(name)) {
> rp_error("refusing to create funny ref '%s' remotely", name);
> return "funny refname";
Not related to this patch: the word "funny" may be not so funny.
But related to the patch:
If the update is denied because of ref_is_denied_case_clone() says so,
the user wants to know this. So that she/he is able to understand it better.
Here we may want something like
rp_error("refusing to create ref '%s' remotely because of a
case insensitive duplicate", name);
>
> }
> @@ -573,6 +712,8 @@ static const char *update(struct command *cmd, struct
> shallow_info *si)
> rp_error("failed to delete %s", name);
> return "failed to delete";
> }
> + if (deny_case_clone_refs)
> + ref_cache_delete(name, ref_case_clone_cache);
> return NULL; /* good */
> }
> else {
> @@ -589,6 +730,8 @@ static const char *update(struct command *cmd, struct
> shallow_info *si)
> if (write_ref_sha1(lock, new_sha1, "push")) {
> return "failed to write"; /* error() already called */
> }
> + if (deny_case_clone_refs)
> + ref_cache_insert(name, ref_case_clone_cache);
> return NULL; /* good */
> }
> }
> @@ -1171,6 +1314,8 @@ int cmd_receive_pack(int argc, const char **argv, const
> char *prefix)
> die("'%s' does not appear to be a git repository", dir);
>
> git_config(receive_pack_config, NULL);
> + if (deny_case_clone_refs == DENY_UNCONFIGURED)
> + deny_case_clone_refs = ignore_case;
>
> if (0 <= transfer_unpack_limit)
> unpack_limit = transfer_unpack_limit;
> diff --git a/refs.c b/refs.c
> index 28d5eca..7d534cc 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -29,12 +29,7 @@ static inline int bad_ref_char(int ch)
> return 0;
> }
>
> -/*
> - * Try to read one refname component from the front of refname. Return
> - * the length of the component found, or -1 if the component is not
> - * legal.
> - */
> -static int check_refname_component(const char *refname, int flags)
> +int check_refname_component(const char *refname, int flags)
> {
> const char *cp;
> char last = '\0';
> diff --git a/refs.h b/refs.h
> index 87a1a79..38f3272 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -200,6 +200,12 @@ extern int for_each_reflog(each_ref_fn, void *);
> * "." or "..").
> */
> extern int check_refname_format(const char *refname, int flags);
> +/*
> + * Try to read one refname component from the front of refname. Return
> + * the length of the component found, or -1 if the component is not
> + * legal.
> + */
> +extern int check_refname_component(const char *refname, int flags);
>
> extern const char *prettify_refname(const char *refname);
> extern char *shorten_unambiguous_ref(const char *refname, int strict);
> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index 0736bcb..de0a88d 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -129,6 +129,96 @@ test_expect_success 'denyNonFastforwards trumps --force'
> '
> test "$victim_orig" = "$victim_head"
> '
>
> +test_expect_success 'denyCaseCloneRefs works' '
> + (
> + cd victim &&
> + git config receive.denyCaseCloneRefs true &&
> + git config receive.denyDeletes false
> + ) &&
> + git send-pack ./victim HEAD:refs/heads/case/clone &&
> + orig_ver=$(git rev-parse HEAD) &&
> + test_must_fail git send-pack ./victim HEAD^:refs/heads/Case/Clone &&
> + # confirm that this had no effect upstream
> + (
> + cd victim &&
> + ref=$(git for-each-ref --format="%(refname)" refs/heads/Case/Clone)
> &&
> + echo "$ref" | test_must_fail grep -q Case/Clone &&
> + remote_ver=$(git rev-parse case/clone) &&
> + test "$orig_ver" = "$remote_ver"
> + ) &&
> + git send-pack ./victim HEAD^:refs/heads/notacase/clone &&
> + test_must_fail git send-pack ./victim :Case/Clone &&
> + # confirm that this had no effect upstream
> + (
> + cd victim &&
> + ref=$(git for-each-ref --format="%(refname)" refs/heads/Case/Clone)
> &&
> + echo "$ref" | test_must_fail grep -q Case/Clone &&
I'm not sure if this is the ideal combination:
Collect information in a shell variable, echo that to stdout and use grep -q
to find out that something is NOT there-
especially as I have in the back of my mind the warning "grep -q" is not
portable...
But grep -q is in POSIX, so it may be that versions of grep being part of
busybox
have this restriction.
I don't have a better suggestion either, just a loose idea:
cd victim &&
cat "this_or_that" >expected &&
git for-each-ref --format="%(refname)" refs/heads/Case/Clone | sort
>actual &&
test_cmp expect actual
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
