* [PATCH v2] refs: loosen restrictions on wildcard '*' refspecs
@ 2015-07-08 13:00 Jacob Keller
2015-07-10 3:45 ` Jacob Keller
0 siblings, 1 reply; 2+ messages in thread
From: Jacob Keller @ 2015-07-08 13:00 UTC (permalink / raw)
To: git; +Cc: Jacob Keller, Daniel Barkalow, Junio C Hamano
This patch updates the check_refname_component logic in order to allow for
a less strict refspec format in regards to REFNAME_REFSPEC_PATTERN.
Previously the '*' could only replace a single full component, and could
not replace arbitrary text. Now, refs such as `foo/bar*:foo/bar*` will be
accepted. This allows for somewhat more flexibility in references and does
not break any current users. The ref matching code already allows this but
the check_refname_format did not.
This patch also streamlines the code by making this new check part of
check_refname_component instead of checking after we error during
check_refname_format, which makes more sense with how we check other
issues in refname components.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Cc: Daniel Barkalow <barkalow@iabervon.iabervon.org>
Cc: Junio C Hamano <gitster@pobox.com>
---
- v2
* update test suite
Documentation/git-check-ref-format.txt | 4 ++--
refs.c | 39 +++++++++++++++++++---------------
refs.h | 4 ++--
t/t1402-check-ref-format.sh | 8 ++++---
4 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index fc02959..9044dfa 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -94,8 +94,8 @@ OPTIONS
Interpret <refname> as a reference name pattern for a refspec
(as used with remote repositories). If this option is
enabled, <refname> is allowed to contain a single `*`
- in place of a one full pathname component (e.g.,
- `foo/*/bar` but not `foo/bar*`).
+ in the refspec (e.g., `foo/bar*/baz` or `foo/bar*baz/`
+ but not `foo/bar*/baz*`).
--normalize::
Normalize 'refname' by removing any leading slash (`/`)
diff --git a/refs.c b/refs.c
index 7ac05cf..8702644 100644
--- a/refs.c
+++ b/refs.c
@@ -20,11 +20,12 @@ struct ref_lock {
* 2: ., look for a preceding . to reject .. in refs
* 3: {, look for a preceding @ to reject @{ in refs
* 4: A bad character: ASCII control characters, "~", "^", ":" or SP
+ * 5: check for patterns to reject unless REFNAME_REFSPEC_PATTERN is set
*/
static unsigned char refname_disposition[256] = {
1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
- 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 2, 1,
+ 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
@@ -71,11 +72,13 @@ static unsigned char refname_disposition[256] = {
* - any path component of it begins with ".", or
* - it has double dots "..", or
* - it has ASCII control character, "~", "^", ":" or SP, anywhere, or
- * - it ends with a "/".
- * - it ends with ".lock"
- * - it contains a "\" (backslash)
+ * - it ends with a "/", or
+ * - it ends with ".lock", or
+ * - it contains a "\" (backslash), or
+ * - it contains a "@{" portion, or
+ * - it contains a '*' unless REFNAME_REFSPEC_PATTERN is set
*/
-static int check_refname_component(const char *refname, int flags)
+static int check_refname_component(const char *refname, int *flags)
{
const char *cp;
char last = '\0';
@@ -96,6 +99,16 @@ static int check_refname_component(const char *refname, int flags)
break;
case 4:
return -1;
+ case 5:
+ if (!(*flags & REFNAME_REFSPEC_PATTERN))
+ return -1; /* refspec can't be a pattern */
+
+ /*
+ * Unset the pattern flag so that we only accept a single glob for
+ * the entire refspec.
+ */
+ *flags &= ~ REFNAME_REFSPEC_PATTERN;
+ break;
}
last = ch;
}
@@ -120,18 +133,10 @@ 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, flags);
- if (component_len <= 0) {
- if ((flags & REFNAME_REFSPEC_PATTERN) &&
- refname[0] == '*' &&
- (refname[1] == '\0' || refname[1] == '/')) {
- /* Accept one wildcard as a full refname component. */
- flags &= ~REFNAME_REFSPEC_PATTERN;
- component_len = 1;
- } else {
- return -1;
- }
- }
+ component_len = check_refname_component(refname, &flags);
+ if (component_len <= 0)
+ return -1;
+
component_count++;
if (refname[component_len] == '\0')
break;
diff --git a/refs.h b/refs.h
index 8c3d433..1fd1272 100644
--- a/refs.h
+++ b/refs.h
@@ -224,8 +224,8 @@ extern int for_each_reflog(each_ref_fn, void *);
* 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. No leading or repeated slashes are accepted.
+ * allow a single "*" wildcard character in the refspec. No leading or
+ * repeated slashes are accepted.
*/
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 e5dc62e..0790edf 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -62,9 +62,11 @@ invalid_ref 'heads/foo\bar'
invalid_ref "$(printf 'heads/foo\t')"
invalid_ref "$(printf 'heads/foo\177')"
valid_ref "$(printf 'heads/fu\303\237')"
-invalid_ref 'heads/*foo/bar' --refspec-pattern
-invalid_ref 'heads/foo*/bar' --refspec-pattern
-invalid_ref 'heads/f*o/bar' --refspec-pattern
+valid_ref 'heads/*foo/bar' --refspec-pattern
+valid_ref 'heads/foo*/bar' --refspec-pattern
+valid_ref 'heads/f*o/bar' --refspec-pattern
+invalid_ref 'heads/f*o*/bar' --refspec-pattern
+invalid_ref 'heads/foo*/bar*' --refspec-pattern
ref='foo'
invalid_ref "$ref"
--
2.5.0.rc1.2.gbb9760d
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] refs: loosen restrictions on wildcard '*' refspecs
2015-07-08 13:00 [PATCH v2] refs: loosen restrictions on wildcard '*' refspecs Jacob Keller
@ 2015-07-10 3:45 ` Jacob Keller
0 siblings, 0 replies; 2+ messages in thread
From: Jacob Keller @ 2015-07-10 3:45 UTC (permalink / raw)
To: git; +Cc: Jacob Keller, Daniel Barkalow, Junio C Hamano
On Wed, Jul 8, 2015 at 6:00 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> This patch updates the check_refname_component logic in order to allow for
> a less strict refspec format in regards to REFNAME_REFSPEC_PATTERN.
> Previously the '*' could only replace a single full component, and could
> not replace arbitrary text. Now, refs such as `foo/bar*:foo/bar*` will be
> accepted. This allows for somewhat more flexibility in references and does
> not break any current users. The ref matching code already allows this but
> the check_refname_format did not.
>
> This patch also streamlines the code by making this new check part of
> check_refname_component instead of checking after we error during
> check_refname_format, which makes more sense with how we check other
> issues in refname components.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> Cc: Daniel Barkalow <barkalow@iabervon.iabervon.org>
> Cc: Junio C Hamano <gitster@pobox.com>
> ---
>
> - v2
> * update test suite
>
> Documentation/git-check-ref-format.txt | 4 ++--
> refs.c | 39 +++++++++++++++++++---------------
> refs.h | 4 ++--
> t/t1402-check-ref-format.sh | 8 ++++---
> 4 files changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
> index fc02959..9044dfa 100644
> --- a/Documentation/git-check-ref-format.txt
> +++ b/Documentation/git-check-ref-format.txt
> @@ -94,8 +94,8 @@ OPTIONS
> Interpret <refname> as a reference name pattern for a refspec
> (as used with remote repositories). If this option is
> enabled, <refname> is allowed to contain a single `*`
> - in place of a one full pathname component (e.g.,
> - `foo/*/bar` but not `foo/bar*`).
> + in the refspec (e.g., `foo/bar*/baz` or `foo/bar*baz/`
> + but not `foo/bar*/baz*`).
>
> --normalize::
> Normalize 'refname' by removing any leading slash (`/`)
> diff --git a/refs.c b/refs.c
> index 7ac05cf..8702644 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -20,11 +20,12 @@ struct ref_lock {
> * 2: ., look for a preceding . to reject .. in refs
> * 3: {, look for a preceding @ to reject @{ in refs
> * 4: A bad character: ASCII control characters, "~", "^", ":" or SP
> + * 5: check for patterns to reject unless REFNAME_REFSPEC_PATTERN is set
> */
> static unsigned char refname_disposition[256] = {
> 1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
> 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
> - 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 2, 1,
> + 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1,
> 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
> 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
> @@ -71,11 +72,13 @@ static unsigned char refname_disposition[256] = {
> * - any path component of it begins with ".", or
> * - it has double dots "..", or
> * - it has ASCII control character, "~", "^", ":" or SP, anywhere, or
> - * - it ends with a "/".
> - * - it ends with ".lock"
> - * - it contains a "\" (backslash)
> + * - it ends with a "/", or
> + * - it ends with ".lock", or
> + * - it contains a "\" (backslash), or
> + * - it contains a "@{" portion, or
> + * - it contains a '*' unless REFNAME_REFSPEC_PATTERN is set
> */
> -static int check_refname_component(const char *refname, int flags)
> +static int check_refname_component(const char *refname, int *flags)
> {
> const char *cp;
> char last = '\0';
> @@ -96,6 +99,16 @@ static int check_refname_component(const char *refname, int flags)
> break;
> case 4:
> return -1;
> + case 5:
> + if (!(*flags & REFNAME_REFSPEC_PATTERN))
> + return -1; /* refspec can't be a pattern */
> +
> + /*
> + * Unset the pattern flag so that we only accept a single glob for
> + * the entire refspec.
> + */
> + *flags &= ~ REFNAME_REFSPEC_PATTERN;
> + break;
> }
> last = ch;
> }
> @@ -120,18 +133,10 @@ 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, flags);
> - if (component_len <= 0) {
> - if ((flags & REFNAME_REFSPEC_PATTERN) &&
> - refname[0] == '*' &&
> - (refname[1] == '\0' || refname[1] == '/')) {
> - /* Accept one wildcard as a full refname component. */
> - flags &= ~REFNAME_REFSPEC_PATTERN;
> - component_len = 1;
> - } else {
> - return -1;
> - }
> - }
> + component_len = check_refname_component(refname, &flags);
> + if (component_len <= 0)
> + return -1;
> +
> component_count++;
> if (refname[component_len] == '\0')
> break;
> diff --git a/refs.h b/refs.h
> index 8c3d433..1fd1272 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -224,8 +224,8 @@ extern int for_each_reflog(each_ref_fn, void *);
> * 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. No leading or repeated slashes are accepted.
> + * allow a single "*" wildcard character in the refspec. No leading or
> + * repeated slashes are accepted.
> */
> 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 e5dc62e..0790edf 100755
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -62,9 +62,11 @@ invalid_ref 'heads/foo\bar'
> invalid_ref "$(printf 'heads/foo\t')"
> invalid_ref "$(printf 'heads/foo\177')"
> valid_ref "$(printf 'heads/fu\303\237')"
> -invalid_ref 'heads/*foo/bar' --refspec-pattern
> -invalid_ref 'heads/foo*/bar' --refspec-pattern
> -invalid_ref 'heads/f*o/bar' --refspec-pattern
> +valid_ref 'heads/*foo/bar' --refspec-pattern
> +valid_ref 'heads/foo*/bar' --refspec-pattern
> +valid_ref 'heads/f*o/bar' --refspec-pattern
> +invalid_ref 'heads/f*o*/bar' --refspec-pattern
> +invalid_ref 'heads/foo*/bar*' --refspec-pattern
>
> ref='foo'
> invalid_ref "$ref"
> --
> 2.5.0.rc1.2.gbb9760d
>
Hi Junio,
ping on this? I think I updated it as per your discussion on v2, but I
haven't seen any comments on this version yet.
Regards,
Jake
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-07-10 3:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-08 13:00 [PATCH v2] refs: loosen restrictions on wildcard '*' refspecs Jacob Keller
2015-07-10 3:45 ` Jacob Keller
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).