get_sha1() cannot currently parse a valid object name like
"HEAD:@{upstream}" (assuming that such an oddly named file
exists in the HEAD commit). It takes two passes to parse the
string:
1. It first considers the whole thing as a ref, which
results in looking for the upstream of "HEAD:".
2. It finds the colon, parses "HEAD" as a tree-ish, and then
finds the path "@{upstream}" in the tree.
For a path that looks like a normal reflog (e.g.,
"HEAD:@{yesterday}"), the first pass is a no-op. We try to
dwim_ref("HEAD:"), that returns zero refs, and we proceed
with colon-parsing.
For "HEAD:@{upstream}", though, the first pass ends up in
interpret_upstream_mark, which tries to find the branch
"HEAD:". When it sees that the branch does not exist, it
actually dies rather than returning an error to the caller.
As a result, we never make it to the second pass.
One obvious way of fixing this would be to teach
interpret_upstream_mark to simply report "no, this isn't an
upstream" in such a case. However, that would make the
error-reporting for legitimate upstream cases significantly
worse. Something like "bogus@{upstream}" would simply report
"unknown revision: bogus@{upstream}", while the current code
diagnoses a wide variety of possible misconfigurations (no
such branch, branch exists but does not have upstream, etc).
However, we can take advantage of the fact that a branch
name cannot contain a colon. Therefore even if we find an
upstream mark, any prefix with a colon must mean that
the upstream mark we found is actually a pathname, and
should be disregarded completely. This patch implements that
logic.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
I think this would actually be cleaner if get_sha1() simply did the
colon-parsing first, and omitted the first pass completely. Then
sub-functions would not have to care about arbitrary junk that can come
in paths; they would always be parsing just the revision-specifier.
However, given the way this code has developed over the years and its
general fragility, I would be entirely unsurprised if there is some case
that relies on the current scheme. So I went with the simple fix here,
which should be much less likely to have any fallout. And I could not
come up with an example that is actually broken under the current code
(we just do some suboptimal things, like looking for "foo:bar" as a ref
in the filesystem, even though it is syntactically bogus).
sha1_name.c | 3 +++
t/t1507-rev-parse-upstream.sh | 16 ++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/sha1_name.c b/sha1_name.c
index 6d5038d..b253a88 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1093,6 +1093,9 @@ static int interpret_upstream_mark(const char *name, int namelen,
if (!len)
return -1;
+ if (memchr(name, ':', at))
+ return -1;
+
set_shortened_ref(buf, get_upstream_branch(name, at));
return len + at;
}
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2a19e79..cace1ca 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -210,4 +210,20 @@ test_expect_success 'log -g other@{u}@{now}' '
test_cmp expect actual
'
+test_expect_success '@{reflog}-parsing does not look beyond colon' '
+ echo content >@{yesterday} &&
+ git add @{yesterday} &&
+ git commit -m "funny reflog file" &&
+ git hash-object @{yesterday} >expect &&
+ git rev-parse HEAD:@{yesterday} >actual
+'
+
+test_expect_success '@{upstream}-parsing does not look beyond colon' '
+ echo content >@{upstream} &&
+ git add @{upstream} &&
+ git commit -m "funny upstream file" &&
+ git hash-object @{upstream} >expect &&
+ git rev-parse HEAD:@{upstream} >actual
+'
+
test_done
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html