* [PATCH v2 0/7] Improved infrastructure for refname normalization
From: Michael Haggerty @ 2011-09-10 6:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, cmn, A Large Angry SCM, Michael Haggerty
Patch series re-roll:
The main difference is that I incorporated feedback from Junio that it
should be made more difficult to use unnormalized references. The
mechanism that I selected is a bit different than discussed in the
mailing list; I changed normalize_refname() to accept unnormalized
refnames *only* if the dst argument is non-NULL. I changed
check_ref_format() to reject unnormalized refnames but implemented a
transitional function check_ref_format_unsafe() that has the old
behavior and changed callers to use the _unsafe function to avoid
changing their behavior.
As a prerequisite to storing references caches hierarchically (itself
needed for performance reasons), here is a patch series to help us get
refname normalization under control.
The problem is that some UI accepts unnormalized reference names (like
"/foo/bar" or "foo///bar" instead of "foo/bar") and passes them on to
library routines without normalizing them. The library, on the other
hand, assumes that the refnames are normalized. Sometimes (mostly in
the case of loose references) unnormalized refnames happen to work,
but in other cases (like packed references or when looking up refnames
in the cache) they silently fail. Given that refnames are sometimes
treated as path names, there is a chance that some security-relevant
bugs are lurking in this area, if not in git proper then in scripts
that interact with git.
This patch series adds the following tools for dealing with refnames
and their normalization (without actually doing much to fix the
problem; see below):
* Fix check_ref_format() to make it easier and reliable to specify
which types of refnames are allowed in a particular situation
(multi-level vs. one-level, with vs. without a refspec-like "*").
This function only accepts normalized refnames.
* Add a function normalize_refname() that accepts unnormalized
refnames, checks the format, and outputs a normalized version.
* Add a function check_ref_format_unsafe() that has the old behavior.
Change callers to use this function until they can be fixed.
* Add options to "git check-ref-format" to give scripts access to
these facilities (and to allow them to be tested in the test suite).
* Forbid ".lock" at the end of any refname component, as directories
with such names can conflict with attempts to create lock files for
other refnames.
I suggest the following policy for handling unnormalized refnames more
consistently:
Unnormalized refnames should only be accepted at the UI level and
should be normalized before use. This can be done using code like
int len = strlen(arg);
char *normalized_refname = xmalloc(len);
if (normalize_refname(normalized_refname, len, arg, flags))
die("invalid refname '%s'", arg);
/* From now on, use normalized_refname. */
Refnames coming from other sources, such as from a remote repository,
should be checked that they are in the correct *normalized* format,
like so:
if (check_ref_format(refname, flags))
die("invalid refname '%s'", refname);
Refnames from the local repository (e.g., from the packed references
file) should also be checked that they are in the correct normalized
format, though this policy could be debated if there are performance
concerns.
Refnames probably do not need to be checked at the entrance to the
refs.{c,h} library functions because callers are responsible for not
passing invalid or unnormalized refnames in. However, some assert()s
would probably be justified, especially during the transition while we
are fixing broken callers.
If there is agreement about this policy, I would be happy to write it
up (presumably in Documentation/technical/api-ref.txt, maybe also
incorporating the content from
Documentation/technical/api-ref-iteration.txt).
I do not yet have enough global overview of the code to know which
callers need fixing, but once these tools are in place the callers can
be fixed incrementally.
Michael Haggerty (7):
t1402: add some more tests
Change bad_ref_char() to return a boolean value
git check-ref-format: add options --allow-onelevel and
--refspec-pattern
Change check_ref_format() to take a flags argument
Add a library function normalize_refname()
Do not allow ".lock" at the end of any refname component
Add tools to avoid the use of unnormalized refnames.
Documentation/git-check-ref-format.txt | 44 ++++++++--
builtin/check-ref-format.c | 76 +++++++++---------
builtin/checkout.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/show-ref.c | 2 +-
builtin/tag.c | 4 +-
connect.c | 2 +-
environment.c | 2 +-
fast-import.c | 7 +--
notes-merge.c | 5 +-
pack-refs.c | 2 +-
refs.c | 139 +++++++++++++++++++------------
refs.h | 46 +++++++++-
remote.c | 54 ++++---------
sha1_name.c | 4 +-
t/t1402-check-ref-format.sh | 96 ++++++++++++++++++++--
transport.c | 17 +---
walker.c | 2 +-
20 files changed, 325 insertions(+), 185 deletions(-)
--
1.7.6.8.gd2879
^ permalink raw reply
* [PATCH v2 4/7] Change check_ref_format() to take a flags argument
From: Michael Haggerty @ 2011-09-10 6:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, cmn, A Large Angry SCM, Michael Haggerty
In-Reply-To: <1315637443-14012-1-git-send-email-mhagger@alum.mit.edu>
Change check_ref_format() to take a flags argument that indicates what
is acceptable in the reference name (analogous to --allow-onelevel and
--refspec-pattern). This is more convenient for callers and also
fixes a failure in the test suite (and likely elsewhere in the code)
by enabling "onelevel" and "refspec-pattern" to be allowed
independently of each other.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 21 +----------------
builtin/checkout.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/show-ref.c | 2 +-
builtin/tag.c | 4 +-
connect.c | 2 +-
environment.c | 2 +-
fast-import.c | 7 +-----
notes-merge.c | 5 ++-
pack-refs.c | 2 +-
refs.c | 42 +++++++++++++++------------------
refs.h | 17 +++++++++----
remote.c | 53 +++++++++++-------------------------------
sha1_name.c | 4 +-
t/t1402-check-ref-format.sh | 6 +----
transport.c | 15 ++---------
walker.c | 2 +-
19 files changed, 67 insertions(+), 125 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 6bb9377..c639400 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -53,9 +53,6 @@ static void refname_format_print(const char *arg)
printf("%s\n", refname);
}
-#define REFNAME_ALLOW_ONELEVEL 1
-#define REFNAME_REFSPEC_PATTERN 2
-
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
@@ -83,24 +80,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- switch (check_ref_format(argv[i])) {
- case CHECK_REF_FORMAT_OK:
- break;
- case CHECK_REF_FORMAT_ERROR:
+ if (check_ref_format(argv[i], flags))
return 1;
- case CHECK_REF_FORMAT_ONELEVEL:
- if (!(flags & REFNAME_ALLOW_ONELEVEL))
- return 1;
- else
- break;
- case CHECK_REF_FORMAT_WILDCARD:
- if (!(flags & REFNAME_REFSPEC_PATTERN))
- return 1;
- else
- break;
- default:
- die("internal error: unexpected value from check_ref_format()");
- }
if (print)
refname_format_print(argv[i]);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3bb6525..2882116 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,
new->name = arg;
setup_branch_path(new);
- if (check_ref_format(new->path) == CHECK_REF_FORMAT_OK &&
+ if (!check_ref_format(new->path, 0) &&
resolve_ref(new->path, branch_rev, 1, NULL))
hashcpy(rev, branch_rev);
else
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 412bd32..411aa7d 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5) &&
- check_ref_format(ref->name + 5))
+ check_ref_format(ref->name + 5, 0))
; /* trash */
else if (args.fetch_all &&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ae164da..4e880ef 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -356,7 +356,7 @@ static const char *update(struct command *cmd)
struct ref_lock *lock;
/* only refs/... are allowed */
- if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
+ if (prefixcmp(name, "refs/") || check_ref_format(name + 5, 0)) {
rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
diff --git a/builtin/replace.c b/builtin/replace.c
index fe3a647..15f0e5e 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
"refs/replace/%s",
sha1_to_hex(object)) > sizeof(ref) - 1)
die("replace ref name too long: %.*s...", 50, ref);
- if (check_ref_format(ref))
+ if (check_ref_format(ref, 0))
die("'%s' is not a valid ref name.", ref);
if (!resolve_ref(ref, prev, 1, NULL))
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 45f0340..375a14b 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)
if (strncmp(ref, match, matchlen))
continue;
}
- if (check_ref_format(ref)) {
+ if (check_ref_format(ref, 0)) {
warning("ref '%s' ignored", ref);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..7aceaab 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -407,12 +407,12 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
{
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_reset(sb);
strbuf_addf(sb, "refs/tags/%s", name);
- return check_ref_format(sb->buf);
+ return check_ref_format(sb->buf, 0);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
diff --git a/connect.c b/connect.c
index ee1d4b4..292a9e2 100644
--- a/connect.c
+++ b/connect.c
@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)
len -= 5;
/* REF_NORMAL means that we don't want the magic fake tag refs */
- if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
+ if ((flags & REF_NORMAL) && check_ref_format(name, 0))
return 0;
/* REF_HEADS means that we want regular branch heads */
diff --git a/environment.c b/environment.c
index e96edcf..8acbb87 100644
--- a/environment.c
+++ b/environment.c
@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)
if (strcmp((*c)->buf, "/") != 0)
strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
strbuf_list_free(components);
- if (check_ref_format(buf.buf) != CHECK_REF_FORMAT_OK)
+ if (check_ref_format(buf.buf, 0))
die("bad git namespace path \"%s\"", raw_namespace);
strbuf_addch(&buf, '/');
return strbuf_detach(&buf, NULL);
diff --git a/fast-import.c b/fast-import.c
index 742e7da..4d55ee6 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -722,13 +722,8 @@ static struct branch *new_branch(const char *name)
if (b)
die("Invalid attempt to create duplicate branch: %s", name);
- switch (check_ref_format(name)) {
- case 0: break; /* its valid */
- case CHECK_REF_FORMAT_ONELEVEL:
- break; /* valid, but too few '/', allow anyway */
- default:
+ if (check_ref_format(name, REFNAME_ALLOW_ONELEVEL))
die("Branch name doesn't conform to GIT standards: %s", name);
- }
b = pool_calloc(1, sizeof(struct branch));
b->name = pool_strdup(name);
diff --git a/notes-merge.c b/notes-merge.c
index e1aaf43..bb8d7c8 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -570,7 +570,8 @@ int notes_merge(struct notes_merge_options *o,
/* Dereference o->local_ref into local_sha1 */
if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
die("Failed to resolve local notes ref '%s'", o->local_ref);
- else if (!check_ref_format(o->local_ref) && is_null_sha1(local_sha1))
+ else if (!check_ref_format(o->local_ref, 0) &&
+ is_null_sha1(local_sha1))
local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
else if (!(local = lookup_commit_reference(local_sha1)))
die("Could not parse local commit %s (%s)",
@@ -583,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,
* Failed to get remote_sha1. If o->remote_ref looks like an
* unborn ref, perform the merge using an empty notes tree.
*/
- if (!check_ref_format(o->remote_ref)) {
+ if (!check_ref_format(o->remote_ref, 0)) {
hashclr(remote_sha1);
remote = NULL;
} else {
diff --git a/pack-refs.c b/pack-refs.c
index 1290570..bc0032d 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -72,7 +72,7 @@ static void try_remove_empty_parents(char *name)
for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
while (*p && *p != '/')
p++;
- /* tolerate duplicate slashes; see check_ref_format() */
+ /* tolerate duplicate slashes; see normalize_refname() */
while (*p == '/')
p++;
}
diff --git a/refs.c b/refs.c
index fd29d89..a206a4c 100644
--- a/refs.c
+++ b/refs.c
@@ -872,10 +872,9 @@ static inline int bad_ref_char(int ch)
return 0;
}
-int check_ref_format(const char *ref)
+int check_ref_format(const char *ref, int flags)
{
int ch, level, last;
- int ret = CHECK_REF_FORMAT_OK;
const char *cp = ref;
level = 0;
@@ -884,41 +883,42 @@ int check_ref_format(const char *ref)
; /* tolerate duplicated slashes */
if (!ch)
/* should not end with slashes */
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
/* we are at the beginning of the path component */
if (ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (bad_ref_char(ch)) {
- if (ch == '*' && (!*cp || *cp == '/') &&
- ret == CHECK_REF_FORMAT_OK)
- ret = CHECK_REF_FORMAT_WILDCARD;
+ if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
+ (!*cp || *cp == '/'))
+ /* Accept one wildcard as a full refname component. */
+ flags &= ~REFNAME_REFSPEC_PATTERN;
else
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
}
last = ch;
/* scan the rest of the path component */
while ((ch = *cp++) != 0) {
if (bad_ref_char(ch))
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (ch == '/')
break;
if (last == '.' && ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (last == '@' && ch == '{')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
last = ch;
}
level++;
if (!ch) {
if (ref <= cp - 2 && cp[-2] == '.')
- return CHECK_REF_FORMAT_ERROR;
- if (level < 2)
- return CHECK_REF_FORMAT_ONELEVEL;
+ return -1;
+ if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
+ return -1;
if (has_extension(ref, ".lock"))
- return CHECK_REF_FORMAT_ERROR;
- return ret;
+ return -1;
+ return 0;
}
}
}
@@ -1103,7 +1103,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
{
char refpath[PATH_MAX];
- if (check_ref_format(ref))
+ if (check_ref_format(ref, 0))
return NULL;
strcpy(refpath, mkpath("refs/%s", ref));
return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
@@ -1111,13 +1111,9 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
{
- switch (check_ref_format(ref)) {
- default:
+ if (check_ref_format(ref, REFNAME_ALLOW_ONELEVEL))
return NULL;
- case 0:
- case CHECK_REF_FORMAT_ONELEVEL:
- return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
- }
+ return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
}
static struct lock_file packlock;
diff --git a/refs.h b/refs.h
index dfb086e..b248ce6 100644
--- a/refs.h
+++ b/refs.h
@@ -97,11 +97,18 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi
*/
extern int for_each_reflog(each_ref_fn, void *);
-#define CHECK_REF_FORMAT_OK 0
-#define CHECK_REF_FORMAT_ERROR (-1)
-#define CHECK_REF_FORMAT_ONELEVEL (-2)
-#define CHECK_REF_FORMAT_WILDCARD (-3)
-extern int check_ref_format(const char *target);
+#define REFNAME_ALLOW_ONELEVEL 1
+#define REFNAME_REFSPEC_PATTERN 2
+
+/*
+ * Return 0 iff ref has the correct format for a refname according to
+ * the rules described in Documentation/git-check-ref-format.txt. If
+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
+ * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
+ * allow a "*" wildcard character in place of one of the name
+ * components.
+ */
+extern int check_ref_format(const char *target, int flags);
extern const char *prettify_refname(const char *refname);
extern char *shorten_unambiguous_ref(const char *ref, int strict);
diff --git a/remote.c b/remote.c
index b8ecfa5..7059885 100644
--- a/remote.c
+++ b/remote.c
@@ -493,23 +493,6 @@ static void read_config(void)
}
/*
- * We need to make sure the remote-tracking branches are well formed, but a
- * wildcard refspec in "struct refspec" must have a trailing slash. We
- * temporarily drop the trailing '/' while calling check_ref_format(),
- * and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
- * error return is Ok for a wildcard refspec.
- */
-static int verify_refname(char *name, int is_glob)
-{
- int result;
-
- result = check_ref_format(name);
- if (is_glob && result == CHECK_REF_FORMAT_WILDCARD)
- result = CHECK_REF_FORMAT_OK;
- return result;
-}
-
-/*
* This function frees a refspec array.
* Warning: code paths should be checked to ensure that the src
* and dst pointers are always freeable pointers as well
@@ -532,13 +515,13 @@ static void free_refspecs(struct refspec *refspec, int nr_refspec)
static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
{
int i;
- int st;
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
for (i = 0; i < nr_refspec; i++) {
size_t llen;
int is_glob;
const char *lhs, *rhs;
+ int flags;
is_glob = 0;
@@ -576,6 +559,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
rs[i].pattern = is_glob;
rs[i].src = xstrndup(lhs, llen);
+ flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
if (fetch) {
/*
@@ -585,26 +569,20 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
*/
if (!*rs[i].src)
; /* empty is ok */
- else {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_ref_format(rs[i].src, flags))
+ goto invalid;
/*
* RHS
* - missing is ok, and is same as empty.
* - empty is ok; it means not to store.
* - otherwise it must be a valid looking ref.
*/
- if (!rs[i].dst) {
+ if (!rs[i].dst)
; /* ok */
- } else if (!*rs[i].dst) {
+ else if (!*rs[i].dst)
; /* ok */
- } else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_ref_format(rs[i].dst, flags))
+ goto invalid;
} else {
/*
* LHS
@@ -616,8 +594,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
if (!*rs[i].src)
; /* empty is ok */
else if (is_glob) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_ref_format(rs[i].src, flags))
goto invalid;
}
else
@@ -630,14 +607,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
* - otherwise it must be a valid looking ref.
*/
if (!rs[i].dst) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_ref_format(rs[i].src, flags))
goto invalid;
} else if (!*rs[i].dst) {
goto invalid;
} else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_ref_format(rs[i].dst, flags))
goto invalid;
}
}
@@ -1427,8 +1402,8 @@ int get_fetch_map(const struct ref *remote_refs,
for (rmp = &ref_map; *rmp; ) {
if ((*rmp)->peer_ref) {
- int st = check_ref_format((*rmp)->peer_ref->name + 5);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL) {
+ if (check_ref_format((*rmp)->peer_ref->name + 5,
+ REFNAME_ALLOW_ONELEVEL)) {
struct ref *ignore = *rmp;
error("* Ignoring funny ref '%s' locally",
(*rmp)->peer_ref->name);
@@ -1620,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
int len;
/* we already know it starts with refs/ to get here */
- if (check_ref_format(refname + 5))
+ if (check_ref_format(refname + 5, 0))
return 0;
len = strlen(refname) + 1;
diff --git a/sha1_name.c b/sha1_name.c
index ff5992a..975ec3b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -972,9 +972,9 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
{
strbuf_branchname(sb, name);
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
- return check_ref_format(sb->buf);
+ return check_ref_format(sb->buf, 0);
}
/*
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 795301d..d7e8d90 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -84,11 +84,7 @@ valid_ref "$ref" '--refspec-pattern --allow-onelevel'
ref='*'
invalid_ref "$ref"
-
-#invalid_ref "$ref" --allow-onelevel
-test_expect_failure "ref name '$ref' is invalid with options --allow-onelevel" \
- "test_must_fail git check-ref-format --allow-onelevel '$ref'"
-
+invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
diff --git a/transport.c b/transport.c
index fa279d5..225d9b8 100644
--- a/transport.c
+++ b/transport.c
@@ -754,18 +754,9 @@ void transport_verify_remote_names(int nr_heads, const char **heads)
continue;
remote = remote ? (remote + 1) : local;
- switch (check_ref_format(remote)) {
- case 0: /* ok */
- case CHECK_REF_FORMAT_ONELEVEL:
- /* ok but a single level -- that is fine for
- * a match pattern.
- */
- case CHECK_REF_FORMAT_WILDCARD:
- /* ok but ends with a pattern-match character */
- continue;
- }
- die("remote part of refspec is not a valid name in %s",
- heads[i]);
+ if (check_ref_format(remote, REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))
+ die("remote part of refspec is not a valid name in %s",
+ heads[i]);
}
}
diff --git a/walker.c b/walker.c
index dce7128..e5d8eb2 100644
--- a/walker.c
+++ b/walker.c
@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
{
if (!get_sha1_hex(target, sha1))
return 0;
- if (!check_ref_format(target)) {
+ if (!check_ref_format(target, 0)) {
struct ref *ref = alloc_ref(target);
if (!walker->fetch_ref(walker, ref)) {
hashcpy(sha1, ref->old_sha1);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v2 6/7] Do not allow ".lock" at the end of any refname component
From: Michael Haggerty @ 2011-09-10 6:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, cmn, A Large Angry SCM, Michael Haggerty
In-Reply-To: <1315637443-14012-1-git-send-email-mhagger@alum.mit.edu>
Allowing any refname component to end with ".lock" is looking for
trouble; for example,
$ git br foo.lock/bar
$ git br foo
fatal: Unable to create '[...]/.git/refs/heads/foo.lock': File exists.
Therefore, do not allow any refname component to end with ".lock".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 4 +---
refs.c | 6 +++---
t/t1402-check-ref-format.sh | 4 ++++
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index 3ab22b9..f2d21c7 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -28,7 +28,7 @@ git imposes the following rules on how references are named:
. They can include slash `/` for hierarchical (directory)
grouping, but no slash-separated component can begin with a
- dot `.`.
+ dot `.` or end with the sequence `.lock`.
. They must contain at least one `/`. This enforces the presence of a
category like `heads/`, `tags/` etc. but the actual names are not
@@ -47,8 +47,6 @@ git imposes the following rules on how references are named:
. They cannot end with a slash `/` nor a dot `.`.
-. They cannot end with the sequence `.lock`.
-
. They cannot contain a sequence `@{`.
. They cannot contain a `\`.
diff --git a/refs.c b/refs.c
index 372350e..6985a3f 100644
--- a/refs.c
+++ b/refs.c
@@ -933,14 +933,14 @@ int normalize_refname(char *dst, int dstlen, const char *ref, int flags)
if (component[0] == '.')
/* Components must not start with '.'. */
return -1;
+ if (component_len >= 5 && !memcmp(&component[component_len - 5], ".lock", 5))
+ /* Components must not end with ".lock". */
+ return -1;
} while (ch != 0);
if (last == '.')
/* Refname must not end with '.'. */
return -1;
- if (component_len >= 5 && !memcmp(&component[component_len - 5], ".lock", 5))
- /* Refname must not end with ".lock". */
- return -1;
if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
/* Refname must have at least two components. */
return -1;
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index b0b773b..419788f 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -43,6 +43,8 @@ invalid_ref 'heads/foo?bar'
valid_ref 'foo./bar'
invalid_ref 'heads/foo.lock'
invalid_ref 'heads///foo.lock'
+invalid_ref 'foo.lock/bar'
+invalid_ref 'foo.lock///bar'
valid_ref 'heads/foo@bar'
invalid_ref 'heads/v@{ation'
invalid_ref 'heads/foo\bar'
@@ -158,5 +160,7 @@ invalid_ref_normalized 'heads/./foo'
invalid_ref_normalized 'heads\foo'
invalid_ref_normalized 'heads/foo.lock'
invalid_ref_normalized 'heads///foo.lock'
+invalid_ref_normalized 'foo.lock/bar'
+invalid_ref_normalized 'foo.lock///bar'
test_done
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v2 7/7] Add tools to avoid the use of unnormalized refnames.
From: Michael Haggerty @ 2011-09-10 6:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, cmn, A Large Angry SCM, Michael Haggerty
In-Reply-To: <1315637443-14012-1-git-send-email-mhagger@alum.mit.edu>
Change normalize_refname() to only allow unnormalized refnames if its
dst parameter is non-NULL. Callers who want to support unnormalized
refnames will need the unnormalized copy, and callers unprepared to
deal with a normalized copy shouldn't pretend to accept unnormalized
refnames.
Change check_ref_format() to reject unnormalized refnames.
Add a new temporary function, check_ref_format_unsafe(), which accepts
unnormalized refnames like the old check_ref_format(). Change callers
to use this function so that their behavior is preserved, even though
in many cases it is brokenness that is being preserved. Callers
should be fixed to use one of the new functions, then
check_ref_format_unsafe() should be removed.
Add options --normalize and --no-normalize to "git check-ref-format"
to access the new functionality and use the new options to add tests.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 13 ++++++++-
builtin/check-ref-format.c | 34 ++++++++++++++-----------
builtin/checkout.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/show-ref.c | 2 +-
builtin/tag.c | 2 +-
connect.c | 2 +-
environment.c | 2 +-
fast-import.c | 2 +-
notes-merge.c | 4 +-
refs.c | 17 ++++++++++---
refs.h | 41 +++++++++++++++++++++++--------
remote.c | 17 +++++++------
sha1_name.c | 2 +-
t/t1402-check-ref-format.sh | 3 ++
transport.c | 4 ++-
walker.c | 2 +-
19 files changed, 101 insertions(+), 54 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index f2d21c7..25a7c43 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -9,7 +9,8 @@ SYNOPSIS
--------
[verse]
'git check-ref-format' [--print]
- [--[no-]allow-onelevel] [--refspec-pattern] <refname>
+ [--[no-]allow-onelevel] [--refspec-pattern] [--[no-]normalize]
+ <refname>
'git check-ref-format' --branch <branchname-shorthand>
DESCRIPTION
@@ -71,7 +72,7 @@ reference name expressions (see linkgit:gitrevisions[7]):
. at-open-brace `@{` is used as a notation to access a reflog entry.
With the `--print` option, if 'refname' is acceptable, it prints the
-canonicalized name of a hypothetical reference with that name. That is,
+normalized name of a hypothetical reference with that name. That is,
it prints 'refname' with any extra `/` characters removed.
With the `--branch` option, it expands the ``previous branch syntax''
@@ -95,6 +96,14 @@ OPTIONS
of a one full pathname component (e.g., `foo/*/bar` but not
`foo/bar*`).
+--normalize::
+--no-normalize::
+ Controls whether <refname> is allowed to contain extra `/`
+ characters at the beginning and between name components. If
+ this option is set, such extra slashes are stripped out of the
+ value printed by the `--print` option. The default is
+ `--normalize`.
+
EXAMPLES
--------
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 4c202af..fc1ffd2 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -23,22 +23,12 @@ static int check_ref_format_branch(const char *arg)
return 0;
}
-static int check_ref_format_print(const char *arg, int flags)
-{
- int refnamelen = strlen(arg) + 1;
- char *refname = xmalloc(refnamelen);
-
- if (normalize_refname(refname, refnamelen, arg, flags))
- return 1;
- printf("%s\n", refname);
- return 0;
-}
-
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
int print = 0;
int flags = 0;
+ int normalize = 1;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage(builtin_check_ref_format_usage);
@@ -55,13 +45,27 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
flags &= ~REFNAME_ALLOW_ONELEVEL;
else if (!strcmp(argv[i], "--refspec-pattern"))
flags |= REFNAME_REFSPEC_PATTERN;
+ else if (!strcmp(argv[i], "--normalize"))
+ normalize = 1;
+ else if (!strcmp(argv[i], "--no-normalize"))
+ normalize = 0;
else
usage(builtin_check_ref_format_usage);
}
if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- if (print)
- return check_ref_format_print(argv[i], flags);
- else
- return !!check_ref_format(argv[i], flags);
+ if (normalize) {
+ int refnamelen = strlen(argv[i]) + 1;
+ char *refname = xmalloc(refnamelen);
+ if (normalize_refname(refname, refnamelen, argv[i], flags))
+ return 1;
+ if (print)
+ printf("%s\n", refname);
+ } else {
+ if (check_ref_format(argv[i], flags))
+ return 1;
+ if (print)
+ printf("%s\n", argv[i]);
+ }
+ return 0;
}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2882116..cf16ac3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,
new->name = arg;
setup_branch_path(new);
- if (!check_ref_format(new->path, 0) &&
+ if (!check_ref_format_unsafe(new->path, 0) &&
resolve_ref(new->path, branch_rev, 1, NULL))
hashcpy(rev, branch_rev);
else
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 411aa7d..640e1ac 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5) &&
- check_ref_format(ref->name + 5, 0))
+ check_ref_format_unsafe(ref->name + 5, 0))
; /* trash */
else if (args.fetch_all &&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4e880ef..24d917b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -356,7 +356,7 @@ static const char *update(struct command *cmd)
struct ref_lock *lock;
/* only refs/... are allowed */
- if (prefixcmp(name, "refs/") || check_ref_format(name + 5, 0)) {
+ if (prefixcmp(name, "refs/") || check_ref_format_unsafe(name + 5, 0)) {
rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
diff --git a/builtin/replace.c b/builtin/replace.c
index 15f0e5e..ac6b154 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
"refs/replace/%s",
sha1_to_hex(object)) > sizeof(ref) - 1)
die("replace ref name too long: %.*s...", 50, ref);
- if (check_ref_format(ref, 0))
+ if (check_ref_format_unsafe(ref, 0))
die("'%s' is not a valid ref name.", ref);
if (!resolve_ref(ref, prev, 1, NULL))
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 375a14b..ba5f66f 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)
if (strncmp(ref, match, matchlen))
continue;
}
- if (check_ref_format(ref, 0)) {
+ if (check_ref_format_unsafe(ref, 0)) {
warning("ref '%s' ignored", ref);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 7aceaab..10ee87c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -412,7 +412,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
strbuf_reset(sb);
strbuf_addf(sb, "refs/tags/%s", name);
- return check_ref_format(sb->buf, 0);
+ return check_ref_format_unsafe(sb->buf, 0);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
diff --git a/connect.c b/connect.c
index 292a9e2..3fea6b1 100644
--- a/connect.c
+++ b/connect.c
@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)
len -= 5;
/* REF_NORMAL means that we don't want the magic fake tag refs */
- if ((flags & REF_NORMAL) && check_ref_format(name, 0))
+ if ((flags & REF_NORMAL) && check_ref_format_unsafe(name, 0))
return 0;
/* REF_HEADS means that we want regular branch heads */
diff --git a/environment.c b/environment.c
index 8acbb87..ec293e8 100644
--- a/environment.c
+++ b/environment.c
@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)
if (strcmp((*c)->buf, "/") != 0)
strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
strbuf_list_free(components);
- if (check_ref_format(buf.buf, 0))
+ if (check_ref_format_unsafe(buf.buf, 0))
die("bad git namespace path \"%s\"", raw_namespace);
strbuf_addch(&buf, '/');
return strbuf_detach(&buf, NULL);
diff --git a/fast-import.c b/fast-import.c
index 4d55ee6..70271f0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -722,7 +722,7 @@ static struct branch *new_branch(const char *name)
if (b)
die("Invalid attempt to create duplicate branch: %s", name);
- if (check_ref_format(name, REFNAME_ALLOW_ONELEVEL))
+ if (check_ref_format_unsafe(name, REFNAME_ALLOW_ONELEVEL))
die("Branch name doesn't conform to GIT standards: %s", name);
b = pool_calloc(1, sizeof(struct branch));
diff --git a/notes-merge.c b/notes-merge.c
index bb8d7c8..7e7dd3a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -570,7 +570,7 @@ int notes_merge(struct notes_merge_options *o,
/* Dereference o->local_ref into local_sha1 */
if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
die("Failed to resolve local notes ref '%s'", o->local_ref);
- else if (!check_ref_format(o->local_ref, 0) &&
+ else if (!check_ref_format_unsafe(o->local_ref, 0) &&
is_null_sha1(local_sha1))
local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
else if (!(local = lookup_commit_reference(local_sha1)))
@@ -584,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,
* Failed to get remote_sha1. If o->remote_ref looks like an
* unborn ref, perform the merge using an empty notes tree.
*/
- if (!check_ref_format(o->remote_ref, 0)) {
+ if (!check_ref_format_unsafe(o->remote_ref, 0)) {
hashclr(remote_sha1);
remote = NULL;
} else {
diff --git a/refs.c b/refs.c
index 6985a3f..26720e8 100644
--- a/refs.c
+++ b/refs.c
@@ -879,8 +879,11 @@ int normalize_refname(char *dst, int dstlen, const char *ref, int flags)
ch = *cp;
do {
- while (ch == '/')
- ch = *++cp; /* tolerate leading and repeated slashes */
+ if (dst) {
+ /* tolerate leading and repeated slashes */
+ while (ch == '/')
+ ch = *++cp;
+ }
/*
* We are at the start of a path component. Record
@@ -947,6 +950,12 @@ int normalize_refname(char *dst, int dstlen, const char *ref, int flags)
return 0;
}
+int check_ref_format_unsafe(const char *ref, int flags)
+{
+ char unused[PATH_MAX];
+ return normalize_refname(unused, sizeof(unused), ref, flags);
+}
+
int check_ref_format(const char *ref, int flags)
{
return normalize_refname(NULL, 0, ref, flags);
@@ -1132,7 +1141,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
{
char refpath[PATH_MAX];
- if (check_ref_format(ref, 0))
+ if (check_ref_format_unsafe(ref, 0))
return NULL;
strcpy(refpath, mkpath("refs/%s", ref));
return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
@@ -1140,7 +1149,7 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
{
- if (check_ref_format(ref, REFNAME_ALLOW_ONELEVEL))
+ if (check_ref_format_unsafe(ref, REFNAME_ALLOW_ONELEVEL))
return NULL;
return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
}
diff --git a/refs.h b/refs.h
index 8a15f83..bdfd230 100644
--- a/refs.h
+++ b/refs.h
@@ -102,24 +102,43 @@ extern int for_each_reflog(each_ref_fn, void *);
/*
* Check that ref is a valid refname according to the rules described
- * in Documentation/git-check-ref-format.txt and normalize it by
- * stripping out superfluous "/" characters. If dst != NULL, write
- * the normalized refname to dst, which must be an allocated character
- * array with length dstlen (typically at least as long as ref). dst
- * may point at the same memory as ref. Return 0 iff the refname was
- * OK and fit into dst. If REFNAME_ALLOW_ONELEVEL is set in flags,
- * then accept one-level reference names. If REFNAME_REFSPEC_PATTERN
- * is set in flags, then allow a "*" wildcard characters in place of
- * one of the name components.
+ * in Documentation/git-check-ref-format.txt. If
+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept reference names
+ * with only a single component. If REFNAME_REFSPEC_PATTERN is set in
+ * flags, then allow a "*" wildcard character in place of one of the
+ * name components.
+ *
+ * The handling of normalized/unnormalized refnames is determined by
+ * dst:
+ *
+ * - If dst is non-NULL, then it must be an allocated character array
+ * with length dstlen. Usually dstlen should be at least
+ * strlen(ref)+1. Dst may point at the same memory as ref. In this
+ * case, write a normalized copy of ref to dst, stripping out
+ * superfluous "/" characters.
+ *
+ * - If dst is NULL, verify that the reference is already normalized
+ * and do no copying.
+ *
+ * Return 0 iff the refname was OK and fit into dst.
*/
extern int normalize_refname(char *dst, int dstlen, const char *ref, int flags);
/*
- * Return 0 iff ref has the correct format for a refname. See
- * normalize_refname() for details.
+ * Return 0 iff ref has the correct format for a refname and is
+ * correctly normalized. See normalize_refname() for details.
*/
extern int check_ref_format(const char *ref, int flags);
+/*
+ * Return 0 iff ref has the correct format for a refname, *without*
+ * requiring it to be normalized. This function is unsafe because
+ * unnormalized refnames should not be used; please use
+ * normalize_refname() if you want to accept unnormalized refnames or
+ * check_ref_format() if you only want to accept normalized refnames.
+ */
+extern int check_ref_format_unsafe(const char *ref, int flags);
+
extern const char *prettify_refname(const char *refname);
extern char *shorten_unambiguous_ref(const char *ref, int strict);
diff --git a/remote.c b/remote.c
index 7059885..00e7523 100644
--- a/remote.c
+++ b/remote.c
@@ -559,7 +559,8 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
rs[i].pattern = is_glob;
rs[i].src = xstrndup(lhs, llen);
- flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
+ flags = REFNAME_ALLOW_ONELEVEL |
+ (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
if (fetch) {
/*
@@ -569,7 +570,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
*/
if (!*rs[i].src)
; /* empty is ok */
- else if (check_ref_format(rs[i].src, flags))
+ else if (check_ref_format_unsafe(rs[i].src, flags))
goto invalid;
/*
* RHS
@@ -581,7 +582,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
; /* ok */
else if (!*rs[i].dst)
; /* ok */
- else if (check_ref_format(rs[i].dst, flags))
+ else if (check_ref_format_unsafe(rs[i].dst, flags))
goto invalid;
} else {
/*
@@ -594,7 +595,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
if (!*rs[i].src)
; /* empty is ok */
else if (is_glob) {
- if (check_ref_format(rs[i].src, flags))
+ if (check_ref_format_unsafe(rs[i].src, flags))
goto invalid;
}
else
@@ -607,12 +608,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
* - otherwise it must be a valid looking ref.
*/
if (!rs[i].dst) {
- if (check_ref_format(rs[i].src, flags))
+ if (check_ref_format_unsafe(rs[i].src, flags))
goto invalid;
} else if (!*rs[i].dst) {
goto invalid;
} else {
- if (check_ref_format(rs[i].dst, flags))
+ if (check_ref_format_unsafe(rs[i].dst, flags))
goto invalid;
}
}
@@ -1402,7 +1403,7 @@ int get_fetch_map(const struct ref *remote_refs,
for (rmp = &ref_map; *rmp; ) {
if ((*rmp)->peer_ref) {
- if (check_ref_format((*rmp)->peer_ref->name + 5,
+ if (check_ref_format_unsafe((*rmp)->peer_ref->name + 5,
REFNAME_ALLOW_ONELEVEL)) {
struct ref *ignore = *rmp;
error("* Ignoring funny ref '%s' locally",
@@ -1595,7 +1596,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
int len;
/* we already know it starts with refs/ to get here */
- if (check_ref_format(refname + 5, 0))
+ if (check_ref_format_unsafe(refname + 5, 0))
return 0;
len = strlen(refname) + 1;
diff --git a/sha1_name.c b/sha1_name.c
index 975ec3b..8458454 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -974,7 +974,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
if (name[0] == '-')
return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
- return check_ref_format(sb->buf, 0);
+ return check_ref_format_unsafe(sb->buf, 0);
}
/*
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 419788f..411c271 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -30,9 +30,12 @@ invalid_ref '/'
invalid_ref '/' --allow-onelevel
valid_ref 'foo/bar/baz'
valid_ref 'refs///heads/foo'
+invalid_ref 'refs///heads/foo' --no-normalize
invalid_ref 'heads/foo/'
valid_ref '/heads/foo'
+invalid_ref '/heads/foo' --no-normalize
valid_ref '///heads/foo'
+invalid_ref '///heads/foo' --no-normalize
invalid_ref './foo'
invalid_ref './foo/bar'
invalid_ref 'foo/./bar'
diff --git a/transport.c b/transport.c
index 225d9b8..e00610d 100644
--- a/transport.c
+++ b/transport.c
@@ -754,7 +754,9 @@ void transport_verify_remote_names(int nr_heads, const char **heads)
continue;
remote = remote ? (remote + 1) : local;
- if (check_ref_format(remote, REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))
+ if (check_ref_format_unsafe(remote,
+ REFNAME_ALLOW_ONELEVEL|
+ REFNAME_REFSPEC_PATTERN))
die("remote part of refspec is not a valid name in %s",
heads[i]);
}
diff --git a/walker.c b/walker.c
index e5d8eb2..a1c1ee2 100644
--- a/walker.c
+++ b/walker.c
@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
{
if (!get_sha1_hex(target, sha1))
return 0;
- if (!check_ref_format(target, 0)) {
+ if (!check_ref_format_unsafe(target, 0)) {
struct ref *ref = alloc_ref(target);
if (!walker->fetch_ref(walker, ref)) {
hashcpy(sha1, ref->old_sha1);
--
1.7.6.8.gd2879
^ permalink raw reply related
* Re: git credential helper design [was: What's cooking in git.git (Aug 2011, #07; Wed, 24)]
From: Jeff King @ 2011-09-10 6:53 UTC (permalink / raw)
To: John Szakmeister; +Cc: Junio C Hamano, Lukas Sandström, Ted Zlatanov, git
In-Reply-To: <CAEBDL5XnoCtiKQB8jRxvueWc9zy-yzC+MxgTLmP1amY+U=7aOw@mail.gmail.com>
On Fri, Sep 09, 2011 at 05:55:38AM -0400, John Szakmeister wrote:
> A little feedback here: I do look into my keychain on Mac OS X. I
> tend to keep most of my credentials in a separate keychain that gets
> whenever my computer sleeps or the screen saver kicks on. So that
> blob ends up being user-visible to some degree. Could I munge it into
> something else? Sure. But I do wonder if it might be better to make
> it something closer to what the user expects to see.
Sure, I agree. I guess my question is: what does the user expect to see?
> > https://foo@example.com/specific-repo.git
> >
> > if the user wants a repo-specific authentication context.
>
> Or pass that the information via --domain and --path parameters. It'd
> be nice to keep most credential backends from having to parse urls.
> Not that its hard, just cumbersome. But the keychain implementation
> and the gnome-keyring implementation could both benefit from having
> the pieces broken out separately. Likewise, it's probably not
> difficult to parse it out of the token if we needed to.
Perhaps it's worth providing the information in two forms: parsed and
broken out by individual pieces, and as a more opaque blob. Then systems
which care can use the pieces, and systems which are trying to be as
simple as possible can use the blob.
That still leaves the question of how the user specifies policy about
which parts of the blob are relevant. That is, how do they say that only
the "domain" portion of the hostname is relevant? Or that the path is or
is not relevant?
I was really hoping for the user to be able to specify this at the git
level, to give each storage helper roughly the same feature set.
Maybe it would be enough to do something like:
1. Assemble all of the parts (protocol, username (if any), hostname,
path) into a canonicalized URL representing the authentication
context.
2. Look for git config matching the context URL, and allow
transformations (e.g., match and replace the whole thing, or even
regexp-style substitution).
3. Break the resulting context URL back into constituent parts.
4. Give the helper the context URL, and the broken down parts from
(3). It chooses which to use.
> One thing that crossed my mind while looking at this: what happens
> when a command is meant to be non-interactive? Looking at the
> kdewallet implementation, it appears that not only is the credential
> helper intended to help do the lookup, but also ask the user for a
> password, if it doesn't find anything. That doesn't seem like it
> would play well in a non-interactive environment. Additionally, the
> act of looking up the entry could pop up a dialog in most
> keychain-like applications. Is there a need to be sensitive to the
> fact that we may be run non-interactively?
I think this is somewhat outside the boundaries of what git can provide.
We don't know whether we are interactive or not; we can only make
guesses based on things like whether there is a terminal available. The
helper should be able to make an even better guess, because it can ask
for system-specific things (e.g., a Linux one might check whether
$DISPLAY is set before trying to pop up a dialog). And helpers are free
to simply return nothing. Even though most people will only configure a
single helper, there is actually a stack, and git will try the next one,
and so on until it gets an answer (or if it hits the end without an
answer, will complain).
-Peff
^ permalink raw reply
* Re: [PATCH v3] date.c: Support iso8601 timezone formats
From: Haitao Li @ 2011-09-10 8:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vhb4lvflb.fsf@alter.siamese.dyndns.org>
On Sat, Sep 10, 2011 at 12:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Haitao Li <lihaitao@gmail.com> writes:
>
>> Timezone designators including additional separator (`:`) are ignored.
>> Actually zone designators in below formats are all valid according to
>> ISO8601:2004, section 4.3.2:
>> [+-]hh, [+-]hhmm, [+-]hh:mm
>
> Thanks for a re-roll.
>
>> This patch teaches git recognizing zone designators with hours and
>> minutes separated by colon, or minutes are empty.
>
> The last sentence above makes it sound as if you are accepting
>
> "2011-09-17 12:34:56 +09:"
>
> but I suspect that is not what you intend to allow. Perhaps "we allowed
> hh and hhmm and this teaches Git to recognize hh:mm format as well"?
>
Yes, this is a better one. Sorry for my English, and thanks for the suggestion.
>> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
>> index f87abb5..5235b7a 100755
>> --- a/t/t0006-date.sh
>> +++ b/t/t0006-date.sh
>> @@ -40,6 +40,11 @@ check_parse 2008-02 bad
>> check_parse 2008-02-14 bad
>> check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
>> check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'
>> +check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'
>> +check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 -0500'
>> +check_parse '2008-02-14 20:30:45 -05' '2008-02-14 20:30:45 -0500'
>> +check_parse '2008-02-14 20:30:45 -:30' '2008-02-14 20:30:45 +0000'
>> +check_parse '2008-02-14 20:30:45 -05:00' '2008-02-14 20:30:45 -0500'
>> check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5
>
> The above are from Peff, no? We should credit him for tests in the
> proposed log message.
Yes, we should credit Peff. Sorry for not knowing log message is used for this.
>
> Because the three formats 8601 specifies are "hh", "hhmm", or "hh:mm"
> after +/-, among the above new tests, it appears to me that zone
> designators "-5" and "-:30" should yield "bad", instead of being accepted.
Yes, the spec clearly states 2 digits are mandatory. "-5" should be
regarded as invalid here.
The above test *ignores* ":30" by setting offset to "+0000", this is
to conform to how it works previously, less than 3 digits in offset
are ignored. I agree it's better to *reject* them instead.
> The same for "+09:" I mentioned above, which is not in the new test.
>
Will add.
Thanks again for the suggestions!
^ permalink raw reply
* Re: [PATCH v3] date.c: Support iso8601 timezone formats
From: Haitao Li @ 2011-09-10 8:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vd3f9ve9m.fsf@alter.siamese.dyndns.org>
>
> Also, I do not quite understand why the match_tz() logic needs to be that
> long.
>
> Wouldn't something like this patch (on top of yours) easier to follow?
I was wrong about accepting one digit in hours or minutes. And yes
your version is conciser and easier to follow. Thanks!
>
> date.c | 50 +++++++++++++++++++++-----------------------------
> 1 files changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/date.c b/date.c
> index f8722c1..6079b1a 100644
> --- a/date.c
> +++ b/date.c
> @@ -551,44 +551,36 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
>
> static int match_tz(const char *date, int *offp)
> {
> + int min;
> char *end;
> - int offset = strtoul(date+1, &end, 10);
> - int min, hour;
> - int n = end - date - 1;
> + int hour = strtoul(date + 1, &end, 10);
> + int n = end - (date + 1);
>
> - /*
> - * ISO8601:2004(E) allows time zone designator been separated
> - * by a clone in the extended format
> - */
> - if (*end == ':') {
> - if (isdigit(end[1])) {
> - hour = offset;
> - min = strtoul(end+1, &end, 10);
> - } else {
> - /* Mark as invalid */
> - n = -1;
> - }
> - } else {
> - if (n == 1 || n == 2) {
> - /* Only hours specified */
> - hour = offset;
> - min = 0;
> - } else {
> - hour = offset / 100;
> - min = offset % 100;
> - }
> + if (n == 4) {
> + /* hhmm */
> + min = hour % 100;
> + hour = hour / 100;
> + } else if (n != 2) {
> + min = 99; /* random crap */
> + } else if (*end == ':') {
> + /* hh:mm? */
> + min = strtoul(end + 1, &end, 10);
> + if (end - (date + 1) != 5)
> + min = 99; /* random crap */
> }
>
> /*
> - * Don't accept any random crap.. We might want to check that
> - * the minutes are divisible by 15 or something too. (Offset of
> + * Don't accept any random crap. Even though some places have
> + * offset larger than 12 hours (e.g. Pacific/Kiritimati is at
> + * UTC+14), there is something wrong if hour part is much
> + * larger than that. We might also want to check that the
> + * minutes are divisible by 15 or something too. (Offset of
> * Kathmandu, Nepal is UTC+5:45)
> */
> - if (n > 0 && min < 60) {
> - offset = hour*60+min;
> + if (min < 60 && hour < 24) {
> + int offset = hour * 60 + min;
> if (*date == '-')
> offset = -offset;
> -
> *offp = offset;
> }
> return end - date;
>
^ permalink raw reply
* .gitignore don't ignore a file
From: Bastien Sevajol @ 2011-09-10 10:29 UTC (permalink / raw)
To: git
Hello !
I don't understand why my gitingnore don't want to ignore a file.
I have this in my gitignore:
> app/cache/
> app/logs/
> app/logs/dev.log
> app/logs/prog.log
> app/logs/test.log
> *.log
> *.*~
> nbproject
i've try with app/logs/.gitignore with this:
> dev.log
But, git don't ignore app/logs/dev.log :/
Do you now why ?
thank's =)
bux.
^ permalink raw reply
* Re: .gitignore don't ignore a file
From: Gustaf Hendeby @ 2011-09-10 11:01 UTC (permalink / raw)
To: Bastien Sevajol; +Cc: git
In-Reply-To: <4E6B3C19.4040908@gmail.com>
Hi!
On 09/10/2011 12:29 PM, Bastien Sevajol wrote:
> Hello !
> I don't understand why my gitingnore don't want to ignore a file.
> I have this in my gitignore:
>
>> app/cache/
>> app/logs/
>> app/logs/dev.log
>> app/logs/prog.log
>> app/logs/test.log
>> *.log
>> *.*~
>> nbproject
>
> i've try with app/logs/.gitignore with this:
>
>> dev.log
>
> But, git don't ignore app/logs/dev.log :/
>
> Do you now why ?
> thank's =)
Is the file currently committed (or staged) to your repository? In that
case; Git never ignore files that are put under version control. If
not, could you please elaborate a bit more on in what way the file is
not ignored. It is not completely clear to me what you expect, and what
in fact happens.
/Gustaf
^ permalink raw reply
* Re: merge result
From: Matthieu Moy @ 2011-09-10 11:35 UTC (permalink / raw)
To: Lynn Lin; +Cc: git
In-Reply-To: <CAPgpnMTMPQQPkS-gKLvUJNKLfMWuAT-oA3NCiSRFxu7PknYsnA@mail.gmail.com>
Lynn Lin <lynn.xin.lin@gmail.com> writes:
>>> 1->2->3-4>5 Master
>>> |
>>> 4->6->7 A
>>
>> A more accurate drawing would be
>>
>> 1->2->3-4>5 Master
>> |
>> 4'->6->7 A
>>
>> and after merging, you'd get
>>
>> 1->2->3-4>5-->8 A, master
>> | /
>> 4'->6->7
>>
>> with 8 having both 4 and 4' as ancestors. There's nothing wrong with it.
>> Git cannot remove either 4 or 4' without rewritting history, and "git
>> merge" does not rewrite history.
> so confused here.If 4' is just next 4 commit,how can the diff work? for example
>
> 1->2->4->4'->6->...
>
> diff 4 and 4' is a little confused,correct?
History is not linear. When you type "git log", you may think that 4 and
4' follow each other, but try "gitk" or "git log --oneline --graph" to
see a better view of history.
It's possible to have several times the same change applied to multiple
branches (e.g. when doing cherry-picking), but having twice the same
change in a row is not really possible.
Suppose your commit 4 removes the line "foobar". Then, commits 1, 2 and
3 have the line "foobar" (think of commits as snapshots in history, not
as diff. 3 is a snapshot, and when you run "git show 3", it shows you
the diff from 2 to 3). Commits 4 and 4' don't have it anymore, and then
obviously 5, 6, 7 don't have it either. At the time of merge, Git will
notice that neither of the merges to commit have the line "foobar" and
the result 8 won't have it either.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] t3200: test branch creation with -v
From: Michael J Gruber @ 2011-09-10 13:29 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20110909194357.GA31446@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 09.09.2011 21:43:
> On Fri, Sep 09, 2011 at 09:40:59PM +0200, Michael J Gruber wrote:
>
>> +test_expect_success 'git branch -v t should work' ' + git branch
>> -v t && + test .git/refs/heads/t &&
>
> test -f ?
>
> Also, don't we have test_path_is_file which yields slightly prettier
> output (and maybe some portability benefits; I don't remember)?
>
>> + git branch -d t && + test ! -f .git/refs/heads/t
>
> Ditto for 'test_path_is_missing' here.
>
> -Peff
Well, I tried to follow the surrounding style. That t3200 could benefit
from some attention, though, which I did not want to mix in with the
issue at hand.
Michael
^ permalink raw reply
* Letter from Hong Kong
From: Mr. Daniel Tsai @ 2011-09-09 22:02 UTC (permalink / raw)
To: danieltsai0
Hello my friend. My name is Daniel Tsai and
I live in Hong Kong. I want you to be my
partner in a business transaction of 44.5Million USD.
If you are interested for more details,
you MUST reply me to my private email address:
danieltsai11@yahoo.com.hk or danieltsai95@aol.com
when I get your message, I will tell you what
to do next. Thank you.
Mr. Daniel Tsai
^ permalink raw reply
* Re: [PATCH v3 0/4] Signed push
From: Sverre Rabbelier @ 2011-09-10 15:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Shawn O. Pearce
In-Reply-To: <7vipp1otyp.fsf@alter.siamese.dyndns.org>
Heya,
On Sat, Sep 10, 2011 at 07:19, Junio C Hamano <gitster@pobox.com> wrote:
> Even under v2 design, if somebody who has access to both k.org (public)
> and github (proprietary in the hypothetical universe) would want to
> combine the signed-push notes to see a unified picture (perhaps I push to
> these two sites with different frequencies), he can fetch signed-push
> notes from both sites and merge them himself. But v3 design also allows
> anybody who has access to k.org (which is public so by definition that
> truly is anybody) to peek into signed-push notes at k.org to learn more
> than he should be able to.
I think this is also some further motivation to have a
refs/remotes/github/notes/signed-push and a
refs/remotes/korg/notes/signed-push, rather than have everything
automatically go into refs/notes/signed-push when fetching from a
remote.
(If I misremembered and it's already that way, please ignore this message :P)
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH v3 0/4] Signed push
From: Junio C Hamano @ 2011-09-10 16:30 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: git, Jeff King, Shawn O. Pearce
In-Reply-To: <CAGdFq_hWVPCEeJKKccp4Wc-j+XMSFXqRf6VYd7ngLER8RhODRQ@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
> I think this is also some further motivation to have a
Did you miss that I already mentioned that workaround? It does not _fix_
the fundamental breakage, which is that you are _forcing_ the sending side
to keep copies, though.
^ permalink raw reply
* Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax
From: Ramsay Jones @ 2011-09-10 17:40 UTC (permalink / raw)
To: Sam Vilain
Cc: Michael J Gruber, Junio C Hamano, Eric Wong, GIT Mailing-list,
mhagger
In-Reply-To: <4E27098B.906@vilain.net>
Sam Vilain wrote:
> On 20/07/11 10:07, Michael J Gruber wrote:
>> path@REV are so-called peg revisions, introduced in svn 1.1, and denote
>> "I mean the file named path in REV" (as opposed to "the file named path
>> now and maybe differently back then"). It (now) defaults to BASE (for
>> worktree) resp. HEAD (for URLs). A bit like our rename detection.
>>
>> -r REV specifies the operative revision. After resolving the
>> name/location using the pegrev, the version at the resolved path at the
>> oprative version is operated on.
>>
>> svn 1.5.0 (June 2008) introduced peg revisions to "svn copy", so I
>> assume our people were following svn trunk and adjusting in 2007 already
>> (to r22964). There were some fixes to "svn copy" with peg later on.
>>
>> I do not understand the above commit message at all; and I did not find
>> anything about how "svn copy -r REV" acted in svn 1.4. I would assume
>> "operative revision", and the above commit message seems to imply that
>> peg defaulted to REV here (not HEAD) and that that changed in 1.5.0, but
>> that is a wild guess (svnbook 1.4 does not so anything).
>
> What happened is that I noticed that the code stopped working after svn
> 1.5 was released. Previously I wrote it to detect the merge properties
> as left by SVK and the experimental/contrib python script for merging.
> I was testing at times using trunk SVN versions. You could probably
> figure it out by running ffab6268^ with svn 1.4.x vs svn 1.5.x if you
> cared. My comment tries to explain what you describe above, but without
> the correct terms. I could see via experimentation what the difference
> was between "-r N" and '/path@N', and that the behaviour changed in svn
> 1.5. Apologies for not explaining this thoroughly enough in the
> submitted description!
Hmm, I was hoping that someone would say something like:
"This test does not depend on the difference between the peg revision
and the operative revision, because the history represented in the test
repo is so simple that there *is* no difference, so Acked By: ... "
But, since that didn't happen, maybe the patch given below would be more
acceptable? (I personally prefer the original patch ...)
Given that I didn't quite follow Sam's explanation, I still don't know
if t9104-git-svn-follow-parent.sh needs to be changed (again, this test
*passes* for me), so ... :-P
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] t9159-*.sh: Add an svn version check
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
t/t9159-git-svn-no-parent-mergeinfo.sh | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/t/t9159-git-svn-no-parent-mergeinfo.sh b/t/t9159-git-svn-no-parent-mergeinfo.sh
index 85120b7..69e4815 100755
--- a/t/t9159-git-svn-no-parent-mergeinfo.sh
+++ b/t/t9159-git-svn-no-parent-mergeinfo.sh
@@ -2,6 +2,14 @@
test_description='git svn handling of root commits in merge ranges'
. ./lib-git-svn.sh
+svn_ver="$(svn --version --quiet)"
+case $svn_ver in
+0.* | 1.[0-4].*)
+ skip_all="skipping git-svn test - SVN too old ($svn_ver)"
+ test_done
+ ;;
+esac
+
test_expect_success 'test handling of root commits in merge ranges' '
mkdir -p init/trunk init/branches init/tags &&
echo "r1" > init/trunk/file.txt &&
--
1.7.6
^ permalink raw reply related
* [PATCH 4/2] remote: only update remote-tracking branch if updating refspec
From: Martin von Zweigbergk @ 2011-09-10 19:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Martin von Zweigbergk
In-Reply-To: <7vr53rx9r6.fsf@alter.siamese.dyndns.org>
'git remote rename' will only update the remote's fetch refspec if it
looks like a default one. If the remote has no default fetch refspec,
as in
[remote "origin"]
url = git://git.kernel.org/pub/scm/git/git.git
fetch = +refs/heads/*:refs/remotes/upstream/*
we would not update the fetch refspec and even if there is a ref
called "refs/remotes/origin/master", we should not rename it, since it
was not created by fetching from the remote.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
Some questions on style:
1. Should I wrap the statement in "else" block in braces when "then"
block has braces? I couldn't find anything conclusive by looking at
existing code.
2. Is it ok (as I did) to return from the function prematurely even
though it is not an error case? This will of course make the
function more brittle when it comes to adding code at the end of
it. Would you prefer to invert the condition and put the remainder
of the function inside the "then" block? Or even to extract the
code into a new function?
builtin/remote.c | 10 +++++++---
t/t5505-remote.sh | 3 ++-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 61326cb..b25dfb4 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -625,7 +625,7 @@ static int mv(int argc, const char **argv)
old_remote_context = STRBUF_INIT;
struct string_list remote_branches = STRING_LIST_INIT_NODUP;
struct rename_info rename;
- int i;
+ int i, refspec_updated = 0;
if (argc != 3)
usage_with_options(builtin_remote_rename_usage, options);
@@ -667,12 +667,13 @@ static int mv(int argc, const char **argv)
strbuf_reset(&buf2);
strbuf_addstr(&buf2, oldremote->fetch_refspec[i]);
ptr = strstr(buf2.buf, old_remote_context.buf);
- if (ptr)
+ if (ptr) {
+ refspec_updated = 1;
strbuf_splice(&buf2,
ptr-buf2.buf + strlen(":refs/remotes/"),
strlen(rename.old), rename.new,
strlen(rename.new));
- else
+ } else
warning("Not updating non-default fetch respec\n"
"\t%s\n"
"\tPlease update the configuration manually if necessary.",
@@ -695,6 +696,9 @@ static int mv(int argc, const char **argv)
}
}
+ if (!refspec_updated)
+ return 0;
+
/*
* First remove symrefs, then rename the rest, finally create
* the new symrefs.
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 15186c8..e8af615 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -637,7 +637,8 @@ test_expect_success 'rename does not update a non-default fetch refspec' '
(cd four.one &&
git config remote.origin.fetch +refs/heads/*:refs/heads/origin/* &&
git remote rename origin upstream &&
- test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/heads/origin/*")
+ test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/heads/origin/*" &&
+ git rev-parse -q origin/master)
'
--
1.7.7.rc0.317.gd0afe
^ permalink raw reply related
* [PATCH] contrib: add a credential helper for Mac OS X's keychain
From: Jay Soffian @ 2011-09-10 19:44 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, Junio C Hamano, Jeff King
A credential helper which uses /usr/bin/security to add, search,
and remove entries from the Mac OS X keychain.
Tested with 10.6.8.
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
This is a quick script to explore the new credential API. A more robust
implementation would be to link to OS X's Security framework from C.
contrib/credential/git-credential-osxkeychain | 148 +++++++++++++++++++++++++
1 files changed, 148 insertions(+), 0 deletions(-)
create mode 100755 contrib/credential/git-credential-osxkeychain
diff --git a/contrib/credential/git-credential-osxkeychain b/contrib/credential/git-credential-osxkeychain
new file mode 100755
index 0000000000..ae5ec00d68
--- /dev/null
+++ b/contrib/credential/git-credential-osxkeychain
@@ -0,0 +1,148 @@
+#!/usr/bin/python
+# Copyright 2011 Jay Soffian. All rights reserved.
+# FreeBSD License.
+"""
+A git credential helper that interfaces with the Mac OS X keychain via
+/usr/bin/security.
+"""
+
+import os
+import re
+import sys
+import termios
+from getpass import _raw_input
+from optparse import OptionParser
+from subprocess import Popen, PIPE
+
+USERNAME = 'USERNAME'
+PASSWORD = 'PASSWORD'
+PROMPTS = dict(USERNAME='Username', PASSWORD='Password')
+
+def prompt_tty(what, desc):
+ """Prompt on TTY for username or password with optional description"""
+ prompt = '%s%s: ' % (PROMPTS[what], " for '%s'" % desc if desc else '')
+ # Borrowed mostly from getpass.py
+ fd = os.open('/dev/tty', os.O_RDWR|os.O_NOCTTY)
+ tty = os.fdopen(fd, 'w+', 1)
+ if what == USERNAME:
+ return _raw_input(prompt, tty, tty)
+ old = termios.tcgetattr(fd) # a copy to save
+ new = old[:]
+ new[3] &= ~termios.ECHO # 3 == 'lflags'
+ try:
+ termios.tcsetattr(fd, termios.TCSADRAIN, new)
+ return _raw_input(prompt, tty, tty)
+ finally:
+ termios.tcsetattr(fd, termios.TCSADRAIN, old)
+ tty.write('\n')
+
+def emit_user_pass(username, password):
+ if username:
+ print 'username=' + username
+ if password:
+ print 'password=' + password
+
+def make_security_args(command, protocol, hostname, username):
+ args = ['/usr/bin/security', command]
+ # tlfd is 'dflt' backwards - obvious /usr/bin/security bug
+ # but allows us to ignore matching saved web forms.
+ args.extend(['-t', 'tlfd'])
+ args.extend(['-r', protocol])
+ if hostname:
+ args.extend(['-s', hostname])
+ if username:
+ args.extend(['-a', username])
+ return args
+
+def find_internet_password(protocol, hostname, username):
+ args = make_security_args('find-internet-password',
+ protocol, hostname, username)
+ args.append('-g') # asks for password on stderr
+ p = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+ # grok stdout for username
+ out, err = p.communicate()
+ if p.returncode != 0:
+ return
+ for line in out.splitlines(): # pylint:disable-msg=E1103
+ m = re.search(r'^\s+"acct"<blob>=[^"]*"(.*)"$', line)
+ if m:
+ username = m.group(1)
+ break
+ # grok stderr for password
+ m = re.search(r'^password:[^"]*"(.*)"$', err)
+ if not m:
+ return
+ emit_user_pass(username, m.group(1))
+ return True
+
+def delete_internet_password(protocol, hostname, username):
+ args = make_security_args('delete-internet-password',
+ protocol, hostname, username)
+ p = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+ p.communicate()
+
+def add_internet_password(protocol, hostname, username, password):
+ # We do this over a pipe so that we can provide the password more
+ # securely than as an argument which would show up in ps output.
+ # Unfortunately this is possibly less robust since the security man
+ # page does not document how to quote arguments. Emprically it seems
+ # that using the double-quote, escaping \ and " works properly.
+ username = username.replace('\\', '\\\\').replace('"', '\\"')
+ password = password.replace('\\', '\\\\').replace('"', '\\"')
+ command = ' '.join([
+ 'add-internet-password', '-U',
+ '-r', protocol,
+ '-s', hostname,
+ '-a "%s"' % username,
+ '-w "%s"' % password,
+ '-j default',
+ '-l "%s (%s)"' % (hostname, username),
+ ]) + '\n'
+ args = ['/usr/bin/security', '-i']
+ p = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+ p.communicate(command)
+
+def main():
+ p = OptionParser()
+ p.add_option('--description')
+ p.add_option('--reject', action='store_true')
+ p.add_option('--unique', dest='token', help='REQUIRED OPTION')
+ p.add_option('--username')
+ opts, _ = p.parse_args()
+
+ if not opts.token:
+ p.error('--unique option required')
+ if not ':' in opts.token:
+ print >> sys.stderr, "Invalid token: '%s'" % opts.token
+ return 1
+ protocol, hostname = opts.token.split(':', 1)
+ if protocol not in ('http', 'https'):
+ print >> sys.stderr, "Unsupported protocol: '%s'" % protocol
+ return 1
+ if protocol == 'https':
+ protocol = 'htps'
+
+ # "GitHub for Mac" compatibility
+ if hostname == 'github.com':
+ hostname = 'github.com/mac'
+
+ # if this is a rejection delete the existing creds
+ if opts.reject:
+ delete_internet_password(protocol, hostname, opts.username)
+ return 0
+
+ # otherwise look for creds
+ if find_internet_password(protocol, hostname, opts.username):
+ return 0
+
+ # creds not found, so prompt the user then store the creds
+ username = opts.username
+ if username is None:
+ username = prompt_tty(USERNAME, opts.description)
+ password = prompt_tty(PASSWORD, opts.description)
+ add_internet_password(protocol, hostname, username, password)
+ emit_user_pass(username, password)
+ return 0
+
+if __name__ == '__main__':
+ sys.exit(main())
--
1.7.6.346.g5a895
^ permalink raw reply related
* Re: [PATCH v3 0/4] Signed push
From: Robin H. Johnson @ 2011-09-10 20:05 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
Cc: Sverre Rabbelier, Jeff King, Shawn O. Pearce
In-Reply-To: <7vehzopdga.fsf@alter.siamese.dyndns.org>
On Sat, Sep 10, 2011 at 09:30:29AM -0700, Junio C Hamano wrote:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>
> > I think this is also some further motivation to have a
>
> Did you miss that I already mentioned that workaround? It does not _fix_
> the fundamental breakage, which is that you are _forcing_ the sending side
> to keep copies, though.
In the case of a shared Git repo, I'd like to pull copies added by other
developers, so that I can verify the content locally as well. In that
case, I'd need to have copies of the certificates I created as well.
--
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail : robbat2@gentoo.org
GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
^ permalink raw reply
* Re: [PATCH v3 0/4] Signed push
From: Ted Ts'o @ 2011-09-10 19:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, git, Jeff King, Shawn O. Pearce
In-Reply-To: <7vehzopdga.fsf@alter.siamese.dyndns.org>
On Sat, Sep 10, 2011 at 09:30:29AM -0700, Junio C Hamano wrote:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>
> > I think this is also some further motivation to have a
>
> Did you miss that I already mentioned that workaround? It does not _fix_
> the fundamental breakage, which is that you are _forcing_ the sending side
> to keep copies, though.
I guess I'm confused about what the problem is with this?
If I do something like this:
git tag -s -m for_linus-20110910 for_linus-20110910
git push github
git push --tags github
I'm "forcing" the sending side to keep the signed tag, no? Isn't that
kind of implicit in allowing someone to push to your repo?
- Ted
^ permalink raw reply
* Re: [PATCH v3 0/4] Signed push
From: Junio C Hamano @ 2011-09-11 1:42 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Sverre Rabbelier, git, Jeff King, Shawn O. Pearce
In-Reply-To: <20110910192225.GA5397@thunk.org>
Ted Ts'o <tytso@mit.edu> writes:
> I guess I'm confused about what the problem is with this?
Yeah, I have to agree.
> If I do something like this:
>
> git tag -s -m for_linus-20110910 for_linus-20110910
> git push github
> git push --tags github
>
> I'm "forcing" the sending side to keep the signed tag, no?
No, you are not forced to _keep_ it. After pushing you can delete it
locally.
The reason your "tag" example is fundamentally different is because a tag
like for_linus_20110910 is a one-shot thing and you can choose to remove
it from your local namespace once you are done pushing. It does not affect
your ability to make another signed tag for_linus_20110911 before pushing
tomorrow.
The point in this round of "signed push" topic is to allow people not tag
every time before they push, making it easier to sign their pushes to
encourage it, so that other people can have a way to verify the commits
near the tip of branches that are not still tagged in between releases.
Instead of contaminating refs/tags/ namespace with daily tags, the idea
was to keep a single "signed-push" notes tree on the receiving end (which
is the distribution point for consumers) that contain the signed record of
pushes.
The original "signed push" (what I called v2) design was for the sender to
prepare the record that goes into the notes tree, but record the notes
tree at the receiving end (this does _not_ prevent the sender from
fetching it back to keep his local copy, but the sender is _not_ required
to do so). It needs updates to both sending and receiving end.
An alternative idea (which I implemented as v3) that came up during the
discussion was to instead have the sender add this record locally to the
signed-push notes tree, and push it out along with the branches. For this
push not to lose _existing_ records of pushes at the receiving end, the
pusher is required to have an up-to-date copy of signed-push notes tree,
and add the new record to it before pushing it out. One upside is that
this does not need updates to receiving end.
I do not know if you read the message Sverre was responding to, but the
"you have to have local copy" requirement has another and potentially
bigger downside (which Sverre did not quote) for people who push out to
multiple places.
Perhaps we shouldn't worry about tag namespace contamination to make
things easier and simpler and stop using notes tree?
^ permalink raw reply
* "git archive" seems to be broken wrt zip files
From: Linus Torvalds @ 2011-09-11 4:58 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
So I wouldn't ever have noticed on my own, but now that I've tried
github for the kernel, somebody else reported that the downloaded zip
file (seriously? the kernel as a zip file?) is corrupt.
And it doesn't really seem to be a github issue. I can re-create it
with a simple
git archive --format=zip HEAD -o ../kernel.zip
on my kernel repository: the end result does not unzip correctly:
mkdir temp-directory
cd temp-directory
unzip kernel.zip
...
inflating: virt/kvm/iommu.c
inflating: virt/kvm/irq_comm.c
inflating: virt/kvm/kvm_main.c
finishing deferred symbolic links:
arch/microblaze/boot/dts/system.dts -> ../../platform/generic/system.dts
drivers/scsi/aic94xx/aic94xx_reg.h -> /*^J * Aic94xx SAS/SATA
driver hardware registers definitions.[ rest of the file ]
symlink error: File name too long
iow, for some reason that "drivers/scsi/aic94xx/aic94xx_reg.h" file
seems to have been encoded as a symlink.
Anybody seen this?
Linus
^ permalink raw reply
* Re: "git archive" seems to be broken wrt zip files
From: Jeff King @ 2011-09-11 6:22 UTC (permalink / raw)
To: Linus Torvalds; +Cc: René Scharfe, Junio C Hamano, Git Mailing List
In-Reply-To: <CA+55aFx43OxExGNrJs+AyKNtdr+KCZZoE=iaQTz8uHoUSrQv0w@mail.gmail.com>
On Sat, Sep 10, 2011 at 09:58:08PM -0700, Linus Torvalds wrote:
> So I wouldn't ever have noticed on my own, but now that I've tried
> github for the kernel, somebody else reported that the downloaded zip
> file (seriously? the kernel as a zip file?) is corrupt.
>
> And it doesn't really seem to be a github issue. I can re-create it
> with a simple
>
> git archive --format=zip HEAD -o ../kernel.zip
>
> on my kernel repository: the end result does not unzip correctly:
Hmm. I can easily replicate the problem here, but interestingly it does
not happen with sub-trees like:
git archive --format=zip HEAD:drivers -o ../kernel.zip
Going back in history, I can replicate it with René's 62cdce1
(git-archive --format=zip: add symlink support, 2006-10-07). So there's
nothing to bisect.
Cc'ing René.
-Peff
^ permalink raw reply
* Re: "git archive" seems to be broken wrt zip files
From: Jeff King @ 2011-09-11 6:27 UTC (permalink / raw)
To: Linus Torvalds; +Cc: René Scharfe, Junio C Hamano, Git Mailing List
In-Reply-To: <20110911062206.GA29620@sigill.intra.peff.net>
On Sun, Sep 11, 2011 at 02:22:06AM -0400, Jeff King wrote:
> Hmm. I can easily replicate the problem here, but interestingly it does
> not happen with sub-trees like:
>
> git archive --format=zip HEAD:drivers -o ../kernel.zip
>
> Going back in history, I can replicate it with René's 62cdce1
> (git-archive --format=zip: add symlink support, 2006-10-07). So there's
> nothing to bisect.
Weirder still. I get roughly the same output as you:
finishing deferred symbolic links:
arch/microblaze/boot/dts/system.dts -> ../../platform/generic/system.dts
drivers/scsi/aic94xx/aic94xx_reg.h -> /*^J * Aic94xx SAS/SATA driver...
But looking at the generated file with zipinfo, I see:
$ zipinfo kernel.zip
...
lrwxrwxrwx 2.3 unx 33 b- stor 11-Aug-25 14:02 arch/microblaze/boot/dts/system.dts
...
-rw---- 0.0 fat 10470 b- defN 11-Aug-25 14:02 drivers/scsi/aic94xx/aic94xx_reg.h
IOW, the zip file looks right. I wonder if this is actually a bug in
"unzip".
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/4] Signed push
From: Sverre Rabbelier @ 2011-09-11 8:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ted Ts'o, git, Jeff King, Shawn O. Pearce
In-Reply-To: <7v4o0jq2fx.fsf@alter.siamese.dyndns.org>
Heya,
On Sun, Sep 11, 2011 at 03:42, Junio C Hamano <gitster@pobox.com> wrote:
> I do not know if you read the message Sverre was responding to, but the
> "you have to have local copy" requirement has another and potentially
> bigger downside (which Sverre did not quote) for people who push out to
> multiple places.
That's not what I meant though. I was responding to your "than you can
later _inspect_ the certificates from multiple locations". I was
indicating that it would be easier to do such inspection if you can
optionally fetch the notes from different remotes to different
locations in the refs/remotes namespace.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Git wiki down?
From: suvayu ali @ 2011-09-11 11:56 UTC (permalink / raw)
To: git
Is something wrong with the git wiki?
$ ping http://git.wiki.kernel.org/
ping: unknown host http://git.wiki.kernel.org/
On the browser I get redirected to the OpenDNS page when there is a
DNS problem for some site. The OpenDNS cache also says there is no
such site. Was the git wiki affected in the recent DNS hack?
--
Suvayu
Open source is the future. It sets us free.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox