* [RFC 0/7] Make check_refname_format() more flexible
@ 2012-04-30 23:02 mhagger
2012-04-30 23:02 ` [RFC 1/7] check_refname_component(): iterate via index rather than via pointer mhagger
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Here are some patches that I have had kicking around for a while to
make check_refname_format() more flexible. In particular, they add a
REFNAME_FULL option (for checking that the reference name is a valid
full reference name (i.e., either starts with "refs/" or is ALL_CAPS)
and a REFNAME_RELAXED option (which relaxes the rules to the level
that they only prevent "dangerous" refnames, not just confusing ones).
REFNAME_RELAXED is the name I chose instead of REFNAME_NONSTRICT as
discussed on the mailing list. (I decided that "REFNAME_NONSTRICT"
looks too much like "REFNAME_STRICT" when reading quickly over it,
plus it avoids double negatives.)
Patch 2 fixes a big with the handling of the REFNAME_DOT_COMPONENT
option. (I believe that this option is anyway obsoleted by the
changes that removed extra_refs; I'll double-check.)
The patches also do some refactoring, like extracting a new function
parse_refname_prefix(). If some of these fixes will be needed on
older branches, then they should be redone without the refactorings.
Ideally all of the options to check_refname_format() should be covered
by tests; on the other hand I don't think it is a good idea to cruft
up the interface to "git check-ref-format" with options that are
probably not useful outside of the test suite. Is there a precedent
for how to add tests for such code?
Michael Haggerty (7):
check_refname_component(): iterate via index rather than via pointer
check_refname_component(): fix check with REFNAME_DOT_COMPONENT
parse_refname_component(): accept a length parameter
parse_refname_component(): parse until terminator
parse_refname_prefix(): new function
check_refname_format(): add REFNAME_FULL option
check_refname_format(): implement REFNAME_RELAXED option
Documentation/git-check-ref-format.txt | 14 ++++
builtin/check-ref-format.c | 4 +
refs.c | 136 +++++++++++++++++++++++---------
refs.h | 16 +++-
t/t1402-check-ref-format.sh | 31 ++++++++
5 files changed, 158 insertions(+), 43 deletions(-)
--
1.7.10
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 1/7] check_refname_component(): iterate via index rather than via pointer
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
@ 2012-04-30 23:02 ` mhagger
2012-04-30 23:02 ` [RFC 2/7] check_refname_component(): fix check with REFNAME_DOT_COMPONENT mhagger
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This will make later changes easier.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 09322fe..927d36c 100644
--- a/refs.c
+++ b/refs.c
@@ -35,12 +35,12 @@ static inline int bad_ref_char(int ch)
*/
static int check_refname_component(const char *refname, int flags)
{
- const char *cp;
+ int i, len = strlen(refname);
char last = '\0';
- for (cp = refname; ; cp++) {
- char ch = *cp;
- if (ch == '\0' || ch == '/')
+ for (i = 0; i < len; i++) {
+ char ch = refname[i];
+ if (ch == '/')
break;
if (bad_ref_char(ch))
return -1; /* Illegal character in refname. */
@@ -50,7 +50,7 @@ static int check_refname_component(const char *refname, int flags)
return -1; /* Refname contains "@{". */
last = ch;
}
- if (cp == refname)
+ if (i == 0)
return 0; /* Component has zero length. */
if (refname[0] == '.') {
if (!(flags & REFNAME_DOT_COMPONENT))
@@ -59,12 +59,12 @@ static int check_refname_component(const char *refname, int flags)
* Even if leading dots are allowed, don't allow "."
* as a component (".." is prevented by a rule above).
*/
- if (refname[1] == '\0')
+ if (len == 1)
return -1; /* Component equals ".". */
}
- if (cp - refname >= 5 && !memcmp(cp - 5, ".lock", 5))
+ if (i >= 5 && !memcmp(&refname[i - 5], ".lock", 5))
return -1; /* Refname ends with ".lock". */
- return cp - refname;
+ return i;
}
int check_refname_format(const char *refname, int flags)
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 2/7] check_refname_component(): fix check with REFNAME_DOT_COMPONENT
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
2012-04-30 23:02 ` [RFC 1/7] check_refname_component(): iterate via index rather than via pointer mhagger
@ 2012-04-30 23:02 ` mhagger
2012-04-30 23:02 ` [RFC 3/7] parse_refname_component(): accept a length parameter mhagger
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
When REFNAME_DOT_COMPONENT is set, we want to allow refname components
that start with '.' but not equal to ".". The old code would have
allowed refname components equal to "." if they were not the last
component of the refname. Fix the test.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 927d36c..c293fd2 100644
--- a/refs.c
+++ b/refs.c
@@ -59,7 +59,7 @@ static int check_refname_component(const char *refname, int flags)
* Even if leading dots are allowed, don't allow "."
* as a component (".." is prevented by a rule above).
*/
- if (len == 1)
+ if (i == 1)
return -1; /* Component equals ".". */
}
if (i >= 5 && !memcmp(&refname[i - 5], ".lock", 5))
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 3/7] parse_refname_component(): accept a length parameter
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
2012-04-30 23:02 ` [RFC 1/7] check_refname_component(): iterate via index rather than via pointer mhagger
2012-04-30 23:02 ` [RFC 2/7] check_refname_component(): fix check with REFNAME_DOT_COMPONENT mhagger
@ 2012-04-30 23:02 ` mhagger
2012-04-30 23:02 ` [RFC 4/7] parse_refname_component(): parse until terminator mhagger
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index c293fd2..39008f2 100644
--- a/refs.c
+++ b/refs.c
@@ -29,16 +29,16 @@ static inline int bad_ref_char(int ch)
}
/*
- * 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.
+ * Try to read one refname component from the first refnamelen
+ * characters 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)
+static int check_refname_component(const char *refname, int refnamelen, int flags)
{
- int i, len = strlen(refname);
+ int i;
char last = '\0';
- for (i = 0; i < len; i++) {
+ for (i = 0; i < refnamelen; i++) {
char ch = refname[i];
if (ch == '/')
break;
@@ -69,11 +69,12 @@ static int check_refname_component(const char *refname, int flags)
int check_refname_format(const char *refname, int flags)
{
+ int refnamelen = strlen(refname);
int component_len, component_count = 0;
while (1) {
/* We are at the start of a path component. */
- component_len = check_refname_component(refname, flags);
+ component_len = check_refname_component(refname, refnamelen, flags);
if (component_len <= 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
refname[0] == '*' &&
@@ -90,6 +91,7 @@ int check_refname_format(const char *refname, int flags)
break;
/* Skip to next component. */
refname += component_len + 1;
+ refnamelen -= component_len + 1;
}
if (refname[component_len - 1] == '.')
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 4/7] parse_refname_component(): parse until terminator
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
` (2 preceding siblings ...)
2012-04-30 23:02 ` [RFC 3/7] parse_refname_component(): accept a length parameter mhagger
@ 2012-04-30 23:02 ` mhagger
2012-04-30 23:02 ` [RFC 5/7] parse_refname_prefix(): new function mhagger
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Rename check_refname_component() to parse_refname_component() and
change it to parse whatever part of the beginning of the string that
can be considered a valid refname component. The old version reported
an error if a refname component was terminated by anything other then
end-of-string or '/'; this version just reports the length of the part
of the string that can be considered a valid refname component
(whether it is terminated by end-of-string, '/', "..", "@{", or some
character that is illegal in a refname component).
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 64 +++++++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 21 deletions(-)
diff --git a/refs.c b/refs.c
index 39008f2..6e4cba0 100644
--- a/refs.c
+++ b/refs.c
@@ -29,25 +29,32 @@ static inline int bad_ref_char(int ch)
}
/*
- * Try to read one refname component from the first refnamelen
- * characters of refname. Return the length of the component found,
- * or -1 if the component is not legal.
+ * Try to read one refname component from the front of refname.
+ * Return the length of the part of the string that could constitute a
+ * valid refname component (which might be zero), or -1 if the start
+ * of the string looks like a refname component but is formatted
+ * illegally. A refname component can be terminated by the end of the
+ * string, '/', any character forbidden by bad_ref_char(), "..", or
+ * "@{".
*/
-static int check_refname_component(const char *refname, int refnamelen, int flags)
+static int parse_refname_component(const char *refname, int refnamelen, int flags)
{
int i;
char last = '\0';
for (i = 0; i < refnamelen; i++) {
char ch = refname[i];
- if (ch == '/')
+ if (ch == '/') {
break;
- if (bad_ref_char(ch))
- return -1; /* Illegal character in refname. */
- if (last == '.' && ch == '.')
- return -1; /* Refname contains "..". */
- if (last == '@' && ch == '{')
- return -1; /* Refname contains "@{". */
+ } else if (bad_ref_char(ch)) {
+ break; /* Component terminated by illegal character. */
+ } else if (last == '.' && ch == '.') {
+ i--;
+ break; /* Component terminated by "..". */
+ } else if (last == '@' && ch == '{') {
+ i--;
+ break; /* Refname terminated by "@{". */
+ }
last = ch;
}
if (i == 0)
@@ -57,13 +64,14 @@ static int check_refname_component(const char *refname, int refnamelen, int flag
return -1; /* Component starts with '.'. */
/*
* Even if leading dots are allowed, don't allow "."
- * as a component (".." is prevented by a rule above).
+ * as a component (".." is treated as a terminator by
+ * a rule above).
*/
if (i == 1)
return -1; /* Component equals ".". */
}
if (i >= 5 && !memcmp(&refname[i - 5], ".lock", 5))
- return -1; /* Refname ends with ".lock". */
+ return -1; /* Component is not allowed to end with ".lock". */
return i;
}
@@ -74,11 +82,14 @@ int check_refname_format(const char *refname, int flags)
while (1) {
/* We are at the start of a path component. */
- component_len = check_refname_component(refname, refnamelen, flags);
- if (component_len <= 0) {
+ component_len = parse_refname_component(refname, refnamelen, flags);
+ if (component_len < 0) {
+ /* This case includes a refname that ends with '/': */
+ return -1;
+ } else if (component_len == 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
- refname[0] == '*' &&
- (refname[1] == '\0' || refname[1] == '/')) {
+ refnamelen >= 1 && refname[0] == '*' &&
+ (refnamelen == 1 || refname[1] == '/')) {
/* Accept one wildcard as a full refname component. */
flags &= ~REFNAME_REFSPEC_PATTERN;
component_len = 1;
@@ -87,11 +98,22 @@ int check_refname_format(const char *refname, int flags)
}
}
component_count++;
- if (refname[component_len] == '\0')
+ /* See what terminated the component: */
+ if (component_len == refnamelen) {
+ /* We've parsed the whole string: */
break;
- /* Skip to next component. */
- refname += component_len + 1;
- refnamelen -= component_len + 1;
+ } else if (refname[component_len] == '/') {
+ /* Skip to the start of the next component: */
+ refname += component_len + 1;
+ refnamelen -= component_len + 1;
+ } else {
+ /*
+ * The component was ended by something else
+ * (something that cannot be part of a legal
+ * refname).
+ */
+ return -1;
+ }
}
if (refname[component_len - 1] == '.')
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 5/7] parse_refname_prefix(): new function
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
` (3 preceding siblings ...)
2012-04-30 23:02 ` [RFC 4/7] parse_refname_component(): parse until terminator mhagger
@ 2012-04-30 23:02 ` mhagger
2012-04-30 23:02 ` [RFC 6/7] check_refname_format(): add REFNAME_FULL option mhagger
2012-04-30 23:02 ` [RFC 7/7] check_refname_format(): implement REFNAME_RELAXED option mhagger
6 siblings, 0 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Add a new function, parse_refname_prefix(), which looks for a possible
refname at the front of a string and returns its length. Use this
function to implement check_refname_format().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 6e4cba0..70578ee 100644
--- a/refs.c
+++ b/refs.c
@@ -75,21 +75,28 @@ static int parse_refname_component(const char *refname, int refnamelen, int flag
return i;
}
-int check_refname_format(const char *refname, int flags)
+/*
+ * Try to interpret the beginning of string refname as a reference
+ * name. Return the length of the part of the string that could
+ * constitute a valid refname, or -1 if the start of the string cannot
+ * possibly be interpreted as a refname. flags has the same
+ * interpretation as for check_refname_format().
+ */
+static int parse_refname_prefix(const char *refname, int refnamelen, int flags)
{
- int refnamelen = strlen(refname);
+ const char *p = refname;
+ int len = refnamelen;
int component_len, component_count = 0;
while (1) {
/* We are at the start of a path component. */
- component_len = parse_refname_component(refname, refnamelen, flags);
+ component_len = parse_refname_component(p, len, flags);
if (component_len < 0) {
- /* This case includes a refname that ends with '/': */
return -1;
} else if (component_len == 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
- refnamelen >= 1 && refname[0] == '*' &&
- (refnamelen == 1 || refname[1] == '/')) {
+ len >= 1 && p[0] == '*' &&
+ (len == 1 || p[1] == '/')) {
/* Accept one wildcard as a full refname component. */
flags &= ~REFNAME_REFSPEC_PATTERN;
component_len = 1;
@@ -99,28 +106,34 @@ int check_refname_format(const char *refname, int flags)
}
component_count++;
/* See what terminated the component: */
- if (component_len == refnamelen) {
+ if (component_len == len) {
/* We've parsed the whole string: */
break;
- } else if (refname[component_len] == '/') {
+ } else if (p[component_len] == '/') {
/* Skip to the start of the next component: */
- refname += component_len + 1;
- refnamelen -= component_len + 1;
+ p += component_len + 1;
+ len -= component_len + 1;
} else {
/*
* The component was ended by something else
* (something that cannot be part of a legal
* refname).
*/
- return -1;
+ break;
}
}
- if (refname[component_len - 1] == '.')
+ if (p[component_len - 1] == '.')
return -1; /* Refname ends with '.'. */
if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
return -1; /* Refname has only one component. */
- return 0;
+ return p + component_len - refname;
+}
+
+int check_refname_format(const char *refname, int flags)
+{
+ int refnamelen = strlen(refname);
+ return parse_refname_prefix(refname, refnamelen, flags) == refnamelen ? 0 : -1;
}
struct ref_entry;
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 6/7] check_refname_format(): add REFNAME_FULL option
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
` (4 preceding siblings ...)
2012-04-30 23:02 ` [RFC 5/7] parse_refname_prefix(): new function mhagger
@ 2012-04-30 23:02 ` mhagger
2012-04-30 23:02 ` [RFC 7/7] check_refname_format(): implement REFNAME_RELAXED option mhagger
6 siblings, 0 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Add a REFNAME_FULL option that causes check_refname_format() to
require full reference names. A full refname has to start with
"refs/" or consist entirely of capital letters and underscores. If
the REFNAME_FULL bit is set in the flags, check_refname_format() and
parse_refname_prefix() verify that the refname is a valid full
refname.
Add a "--full" option to "git check-ref-format" that can be used to
access the new option. Use this to add some tests involving "git
check-ref-format --full".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 14 +++++++++++
builtin/check-ref-format.c | 4 +++
refs.c | 42 +++++++++++++++++++++-----------
refs.h | 12 ++++++---
t/t1402-check-ref-format.sh | 31 +++++++++++++++++++++++
5 files changed, 85 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index 103e7b1..9a8aafb 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -27,6 +27,11 @@ if refs are packed by `git gc`).
git imposes the following rules on how references are named:
+. They must either start with `refs/` (e.g., `refs/heads/master`) or
+ consist entirely of capital letters and underscores (e.g., `HEAD` or
+ `MERGE_HEAD`). This rule is enforced if the `--full` option is
+ used.
+
. They can include slash `/` for hierarchical (directory)
grouping, but no slash-separated component can begin with a
dot `.` or end with the sequence `.lock`.
@@ -83,6 +88,15 @@ typed the branch name.
OPTIONS
-------
+--full::
+--no-full::
+ Controls whether the refname must represent a full refname
+ (i.e., starting with `refs/`). If `--full` and
+ `--allow-onelevel` are both specified, then special top-level
+ refnames consisting of capital letters and underscored (like
+ `HEAD` or `MERGE_HEAD`) are also allowed. The default is
+ `--no-full`.
+
--allow-onelevel::
--no-allow-onelevel::
Controls whether one-level refnames are accepted (i.e.,
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 28a7320..a73626d 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -64,6 +64,10 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
for (i = 1; i < argc && argv[i][0] == '-'; i++) {
if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
normalize = 1;
+ else if (!strcmp(argv[i], "--full"))
+ flags |= REFNAME_FULL;
+ else if (!strcmp(argv[i], "--no-full"))
+ flags &= ~REFNAME_FULL;
else if (!strcmp(argv[i], "--allow-onelevel"))
flags |= REFNAME_ALLOW_ONELEVEL;
else if (!strcmp(argv[i], "--no-allow-onelevel"))
diff --git a/refs.c b/refs.c
index 70578ee..c4ab64d 100644
--- a/refs.c
+++ b/refs.c
@@ -105,28 +105,42 @@ static int parse_refname_prefix(const char *refname, int refnamelen, int flags)
}
}
component_count++;
- /* See what terminated the component: */
- if (component_len == len) {
- /* We've parsed the whole string: */
+
+ if (component_len == len || p[component_len] != '/')
break;
- } else if (p[component_len] == '/') {
- /* Skip to the start of the next component: */
- p += component_len + 1;
- len -= component_len + 1;
- } else {
+
+ if (component_count == 1 && flags & REFNAME_FULL) {
/*
- * The component was ended by something else
- * (something that cannot be part of a legal
- * refname).
+ * That was the first of multiple components;
+ * make sure that it is equal to "refs":
*/
- break;
+ if (component_len != 4 || prefixcmp(p, "refs"))
+ return -1;
}
+
+ /* Skip to the start of the next component: */
+ p += component_len + 1;
+ len -= component_len + 1;
}
if (p[component_len - 1] == '.')
return -1; /* Refname ends with '.'. */
- if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
- return -1; /* Refname has only one component. */
+ if (component_count == 1) {
+ if (!(flags & REFNAME_ALLOW_ONELEVEL))
+ return -1; /* Refname has only one component. */
+ if (flags & REFNAME_FULL) {
+ /*
+ * Make sure that the (single-component)
+ * refname is ALL_CAPS:
+ */
+ int i;
+ for (i = 0; i < component_len; i++) {
+ char ch = p[i];
+ if (!(('A' <= ch && ch <= 'Z') || ch == '_'))
+ return -1;
+ }
+ }
+ }
return p + component_len - refname;
}
diff --git a/refs.h b/refs.h
index d6c2fe2..efe13bb 100644
--- a/refs.h
+++ b/refs.h
@@ -111,9 +111,10 @@ int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long,
*/
extern int for_each_reflog(each_ref_fn, void *);
-#define REFNAME_ALLOW_ONELEVEL 1
-#define REFNAME_REFSPEC_PATTERN 2
-#define REFNAME_DOT_COMPONENT 4
+#define REFNAME_ALLOW_ONELEVEL 0x01
+#define REFNAME_REFSPEC_PATTERN 0x02
+#define REFNAME_DOT_COMPONENT 0x04
+#define REFNAME_FULL 0x08
/*
* Return 0 iff refname has the correct format for a refname according
@@ -124,7 +125,10 @@ extern int for_each_reflog(each_ref_fn, void *);
* components. No leading or repeated slashes are accepted. If
* REFNAME_DOT_COMPONENT is set in flags, then allow refname
* components to start with "." (but not a whole component equal to
- * "." or "..").
+ * "." or ".."). If REFNAME_FULL is set in flags, then additionally
+ * verify that the refname is a valid full refname--that it either
+ * starts with "refs/" or that it consists of only capital letters and
+ * underscores (like "HEAD" or "MERGE_HEAD").
*/
extern int check_refname_format(const char *refname, int flags);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1ae4d87..2f8a48e 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -70,6 +70,7 @@ invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
invalid_ref "$ref" --normalize
valid_ref "$ref" '--allow-onelevel --normalize'
+invalid_ref "$ref" '--full'
ref='foo/bar'
valid_ref "$ref"
@@ -77,12 +78,19 @@ valid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
valid_ref "$ref" --normalize
+invalid_ref "$ref" '--full'
ref='foo/*'
invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--full'
+invalid_ref "$ref" '--refspec-pattern --full'
+
+ref='refs/*'
+invalid_ref "$ref" '--full'
+valid_ref "$ref" '--refspec-pattern --full'
ref='*/foo'
invalid_ref "$ref"
@@ -91,18 +99,23 @@ valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
invalid_ref "$ref" --normalize
valid_ref "$ref" '--refspec-pattern --normalize'
+invalid_ref "$ref" '--full'
+invalid_ref "$ref" '--refspec-pattern --full'
ref='foo/*/bar'
invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--full'
ref='*'
invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--full'
+invalid_ref "$ref" '--refspec-pattern --full'
ref='foo/*/*'
invalid_ref "$ref" --refspec-pattern
@@ -126,6 +139,24 @@ valid_ref NOT_MINGW "$ref" '--allow-onelevel --normalize'
invalid_ref NOT_MINGW "$ref" '--refspec-pattern --normalize'
valid_ref NOT_MINGW "$ref" '--refspec-pattern --allow-onelevel --normalize'
+ref='HEAD'
+invalid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --full
+valid_ref "$ref" '--allow-onelevel --full'
+
+valid_ref FETCH_HEAD '--allow-onelevel --full'
+valid_ref refs/foo --full
+invalid_ref HEAD/ '--allow-onelevel --full'
+valid_ref HEAD/ '--allow-onelevel --full --normalize'
+invalid_ref refs '--full'
+invalid_ref refs '--allow-onelevel --full'
+invalid_ref refs/ '--allow-onelevel --full'
+invalid_ref HEAD/foo '--full'
+invalid_ref head '--allow-onelevel --full'
+valid_ref 'refs/foo/*' '--refspec-pattern --full'
+valid_ref 'refs/*/foo' '--refspec-pattern --full'
+
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
sha1=$(echo A | git commit-tree $T) &&
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 7/7] check_refname_format(): implement REFNAME_RELAXED option
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
` (5 preceding siblings ...)
2012-04-30 23:02 ` [RFC 6/7] check_refname_format(): add REFNAME_FULL option mhagger
@ 2012-04-30 23:02 ` mhagger
6 siblings, 0 replies; 8+ messages in thread
From: mhagger @ 2012-04-30 23:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Implement a new option, REFNAME_RELAXED, which accepts "slightly"
illegal refnames--those that might cause confusion with notations
like "R1..R2", "^R1", etc. but are unlikely to cause internal errors.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 31 +++++++++++++++++++------------
refs.h | 6 +++++-
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index c4ab64d..3a35e5e 100644
--- a/refs.c
+++ b/refs.c
@@ -17,14 +17,20 @@
*/
/* Return true iff ch is not allowed in reference names. */
-static inline int bad_ref_char(int ch)
+static inline int bad_ref_char(int ch, int flags)
{
- if (((unsigned) ch) <= ' ' || ch == 0x7f ||
- ch == '~' || ch == '^' || ch == ':' || ch == '\\')
- return 1;
- /* 2.13 Pattern Matching Notation */
- if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
- return 1;
+ if (flags & REFNAME_RELAXED) {
+ if (ch == '\0' || ch == '\n' || ch == ' ' ||
+ ch == ':' || ch == '\\')
+ return 1;
+ } else {
+ if (((unsigned) ch) <= ' ' || ch == 0x7f ||
+ ch == '~' || ch == '^' || ch == ':' || ch == '\\')
+ return 1;
+ /* 2.13 Pattern Matching Notation */
+ if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
+ return 1;
+ }
return 0;
}
@@ -46,12 +52,12 @@ static int parse_refname_component(const char *refname, int refnamelen, int flag
char ch = refname[i];
if (ch == '/') {
break;
- } else if (bad_ref_char(ch)) {
+ } else if (bad_ref_char(ch, flags)) {
break; /* Component terminated by illegal character. */
- } else if (last == '.' && ch == '.') {
+ } else if (!(flags & REFNAME_RELAXED) && last == '.' && ch == '.') {
i--;
break; /* Component terminated by "..". */
- } else if (last == '@' && ch == '{') {
+ } else if (!(flags & REFNAME_RELAXED) && last == '@' && ch == '{') {
i--;
break; /* Refname terminated by "@{". */
}
@@ -64,11 +70,12 @@ static int parse_refname_component(const char *refname, int refnamelen, int flag
return -1; /* Component starts with '.'. */
/*
* Even if leading dots are allowed, don't allow "."
- * as a component (".." is treated as a terminator by
- * a rule above).
+ * or ".." as a component.
*/
if (i == 1)
return -1; /* Component equals ".". */
+ if (i == 2 && refname[1] == '.')
+ return -1; /* Component equals "..". */
}
if (i >= 5 && !memcmp(&refname[i - 5], ".lock", 5))
return -1; /* Component is not allowed to end with ".lock". */
diff --git a/refs.h b/refs.h
index efe13bb..dd6cfe5 100644
--- a/refs.h
+++ b/refs.h
@@ -115,6 +115,7 @@ extern int for_each_reflog(each_ref_fn, void *);
#define REFNAME_REFSPEC_PATTERN 0x02
#define REFNAME_DOT_COMPONENT 0x04
#define REFNAME_FULL 0x08
+#define REFNAME_RELAXED 0x10
/*
* Return 0 iff refname has the correct format for a refname according
@@ -128,7 +129,10 @@ extern int for_each_reflog(each_ref_fn, void *);
* "." or ".."). If REFNAME_FULL is set in flags, then additionally
* verify that the refname is a valid full refname--that it either
* starts with "refs/" or that it consists of only capital letters and
- * underscores (like "HEAD" or "MERGE_HEAD").
+ * underscores (like "HEAD" or "MERGE_HEAD"). If REFNAME_RELAXED is
+ * set in flags, then relax the rules to allow control characters
+ * except for SP and LF, wildcard characters '?', '*', and '[', ".."
+ * (but not as a complete refname component), and "@{".
*/
extern int check_refname_format(const char *refname, int flags);
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-04-30 23:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-30 23:02 [RFC 0/7] Make check_refname_format() more flexible mhagger
2012-04-30 23:02 ` [RFC 1/7] check_refname_component(): iterate via index rather than via pointer mhagger
2012-04-30 23:02 ` [RFC 2/7] check_refname_component(): fix check with REFNAME_DOT_COMPONENT mhagger
2012-04-30 23:02 ` [RFC 3/7] parse_refname_component(): accept a length parameter mhagger
2012-04-30 23:02 ` [RFC 4/7] parse_refname_component(): parse until terminator mhagger
2012-04-30 23:02 ` [RFC 5/7] parse_refname_prefix(): new function mhagger
2012-04-30 23:02 ` [RFC 6/7] check_refname_format(): add REFNAME_FULL option mhagger
2012-04-30 23:02 ` [RFC 7/7] check_refname_format(): implement REFNAME_RELAXED option mhagger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).