| Message ID | 20190117200207.81825-1-jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ls-refs: filter refs based on non-namespace name | expand |
Jonathan Tan <jonathantanmy@google.com> writes: > Protocol v2 allows for filtering of ref advertisement based on a > client-given name prefix, but inclusion/exclusion is determined based on > the non-namespace-stripped version (e.g. matching a ref prefix of > "refs/heads" against "refs/namespaces/my-namespace/refs/heads/master") > instead of the namespace-stripped version, which is the one that the > user actually sees. > > Determine inclusion/exclusion based on the namespace-stripped version. > > This bug was discovered through applying patches [1] that override > protocol.version to 2 in repositories when running tests, allowing us to > notice differences in behavior across different protocol versions. > > [1] https://public-inbox.org/git/cover.1547677183.git.jonathantanmy@google.com/ > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > Another bug discovered through GIT_TEST_PROTOCOL_VERSION. OK, so "ls-refs: filter based on non-namespace name" (in the title) is a means to the objective 'ls-refs: make sure it honors namespaces" which is a bugfix? The new test peeks at the protocol level, but wouldn't we be able to see the breakage by running ls-remote or something and observing its result as well, or is the bug only observable with test-tool and not triggerable by end-user facing git commands? > If the patches in [1] above are merged with this patch, a test that > previously failed on GIT_TEST_PROTOCOL_VERSION=2 now passes. > > I'm not sure of the relevance of the last paragraph and the "[1]" in the > commit message - feel free to remove it. Since the relevant patches are > not merged yet, the e-mails are probably the best reference, and I have > tried to summarize what they do concisely. > --- > ls-refs.c | 2 +- > t/t5701-git-serve.sh | 21 +++++++++++++++++++++ > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/ls-refs.c b/ls-refs.c > index a06f12eca8..7782bb054b 100644 > --- a/ls-refs.c > +++ b/ls-refs.c > @@ -40,7 +40,7 @@ static int send_ref(const char *refname, const struct object_id *oid, > const char *refname_nons = strip_namespace(refname); > struct strbuf refline = STRBUF_INIT; > > - if (!ref_match(&data->prefixes, refname)) > + if (!ref_match(&data->prefixes, refname_nons)) > return 0; > > strbuf_addf(&refline, "%s %s", oid_to_hex(oid), refname_nons); > diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh > index ae79c6bbc0..ec13064ecd 100755 > --- a/t/t5701-git-serve.sh > +++ b/t/t5701-git-serve.sh > @@ -112,6 +112,27 @@ test_expect_success 'basic ref-prefixes' ' > test_cmp expect actual > ' > > +test_expect_success 'ref prefix with namespaced repository' ' > + # Create a namespaced ref > + git update-ref refs/namespaces/ns/refs/heads/master "$(git rev-parse refs/tags/one)" && > + > + test-tool pkt-line pack >in <<-EOF && > + command=ls-refs > + 0001 > + ref-prefix refs/heads/master > + 0000 > + EOF > + > + cat >expect <<-EOF && > + $(git rev-parse refs/tags/one) refs/heads/master > + 0000 > + EOF > + > + GIT_NAMESPACE=ns git serve --stateless-rpc <in >out && > + test-tool pkt-line unpack <out >actual && > + test_cmp expect actual > +' > + > test_expect_success 'refs/heads prefix' ' > test-tool pkt-line pack >in <<-EOF && > command=ls-refs
diff --git a/ls-refs.c b/ls-refs.c index a06f12eca8..7782bb054b 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -40,7 +40,7 @@ static int send_ref(const char *refname, const struct object_id *oid, const char *refname_nons = strip_namespace(refname); struct strbuf refline = STRBUF_INIT; - if (!ref_match(&data->prefixes, refname)) + if (!ref_match(&data->prefixes, refname_nons)) return 0; strbuf_addf(&refline, "%s %s", oid_to_hex(oid), refname_nons); diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index ae79c6bbc0..ec13064ecd 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -112,6 +112,27 @@ test_expect_success 'basic ref-prefixes' ' test_cmp expect actual ' +test_expect_success 'ref prefix with namespaced repository' ' + # Create a namespaced ref + git update-ref refs/namespaces/ns/refs/heads/master "$(git rev-parse refs/tags/one)" && + + test-tool pkt-line pack >in <<-EOF && + command=ls-refs + 0001 + ref-prefix refs/heads/master + 0000 + EOF + + cat >expect <<-EOF && + $(git rev-parse refs/tags/one) refs/heads/master + 0000 + EOF + + GIT_NAMESPACE=ns git serve --stateless-rpc <in >out && + test-tool pkt-line unpack <out >actual && + test_cmp expect actual +' + test_expect_success 'refs/heads prefix' ' test-tool pkt-line pack >in <<-EOF && command=ls-refs
Protocol v2 allows for filtering of ref advertisement based on a client-given name prefix, but inclusion/exclusion is determined based on the non-namespace-stripped version (e.g. matching a ref prefix of "refs/heads" against "refs/namespaces/my-namespace/refs/heads/master") instead of the namespace-stripped version, which is the one that the user actually sees. Determine inclusion/exclusion based on the namespace-stripped version. This bug was discovered through applying patches [1] that override protocol.version to 2 in repositories when running tests, allowing us to notice differences in behavior across different protocol versions. [1] https://public-inbox.org/git/cover.1547677183.git.jonathantanmy@google.com/ Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- Another bug discovered through GIT_TEST_PROTOCOL_VERSION. If the patches in [1] above are merged with this patch, a test that previously failed on GIT_TEST_PROTOCOL_VERSION=2 now passes. I'm not sure of the relevance of the last paragraph and the "[1]" in the commit message - feel free to remove it. Since the relevant patches are not merged yet, the e-mails are probably the best reference, and I have tried to summarize what they do concisely. --- ls-refs.c | 2 +- t/t5701-git-serve.sh | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)