upload-pack: fix transfer.hiderefs over smart-http

When upload-pack advertises the refs (either for a normal,
non-stateless request, or for the initial contact in a
stateless one), we call for_each_ref with the send_ref
function as its callback. send_ref, in turn, calls
mark_our_ref, which checks whether the ref is hidden, and
sets OUR_REF or HIDDEN_REF on the object as appropriate.  If
it is hidden, mark_our_ref also returns "1" to signal
send_ref that the ref should not be advertised.

If we are not advertising refs, (i.e., the follow-up
invocation by an http client to send its "want" lines), we
use mark_our_ref directly as a callback to for_each_ref. Its
marking does the right thing, but when it then returns "1"
to for_each_ref, the latter interprets this as an error and
stops iterating. As a result, we skip marking all of the
refs that come lexicographically after it. Any "want" lines
from the client asking for those objects will fail, as they
were not properly marked with OUR_REF.

To solve this, we introduce a wrapper callback around
mark_our_ref which always returns 0 (even if the ref is
hidden, we want to keep iterating). We also tweak the
signature of mark_our_ref to exclude unnecessary parameters
that were present only to conform to the callback interface.
This should make it less likely for somebody to accidentally
use it as a callback in the future.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 6cbc12d9a7ad059f77967c0bdb702d8a5ec75bb9..b97077351ddd8f4dd29ddb1931754b0b83f4d808 100755 (executable)
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -213,6 +213,17 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set
        test_cmp expect_cookies.txt cookies_tail.txt
 '
 
        test_cmp expect_cookies.txt cookies_tail.txt
 '
 

+test_expect_success 'transfer.hiderefs works over smart-http' '
+       test_commit hidden &&
+       test_commit visible &&
+       git push public HEAD^:refs/heads/a HEAD:refs/heads/b &&
+       git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
+               config transfer.hiderefs refs/heads/a &&
+       git clone --bare "$HTTPD_URL/smart/repo.git" hidden.git &&
+       test_must_fail git -C hidden.git rev-parse --verify a &&
+       git -C hidden.git rev-parse --verify b
+'
+

 test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
        (
        cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
 test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
        (
        cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&

diff --git a/upload-pack.c b/upload-pack.c
index 433211a238cfee261e6e1f9e73d14df9178f004a..af2a13acc6d54c07e301c87de678bf2ca1863d11 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -680,7 +680,7 @@ static void receive_needs(void)
 }
 
 /* return non-zero if the ref is hidden, otherwise 0 */
 }
 
 /* return non-zero if the ref is hidden, otherwise 0 */

-static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int mark_our_ref(const char *refname, const unsigned char *sha1)

 {
        struct object *o = lookup_unknown_object(sha1);
 
 {
        struct object *o = lookup_unknown_object(sha1);
 


@@ -694,6 +694,12 @@ static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag
        return 0;
 }
 
        return 0;
 }
 

+static int check_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+{
+       mark_our_ref(refname, sha1);
+       return 0;
+}
+

 static void format_symref_info(struct strbuf *buf, struct string_list *symref)
 {
        struct string_list_item *item;
 static void format_symref_info(struct strbuf *buf, struct string_list *symref)
 {
        struct string_list_item *item;


@@ -712,7 +718,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
        const char *refname_nons = strip_namespace(refname);
        unsigned char peeled[20];
 
        const char *refname_nons = strip_namespace(refname);
        unsigned char peeled[20];
 

-       if (mark_our_ref(refname, sha1, flag, NULL))
+       if (mark_our_ref(refname, sha1))

                return 0;
 
        if (capabilities) {
                return 0;
 
        if (capabilities) {


@@ -766,8 +772,8 @@ static void upload_pack(void)
                advertise_shallow_grafts(1);
                packet_flush(1);
        } else {
                advertise_shallow_grafts(1);
                packet_flush(1);
        } else {

-               head_ref_namespaced(mark_our_ref, NULL);
-               for_each_namespaced_ref(mark_our_ref, NULL);
+               head_ref_namespaced(check_ref, NULL);
+               for_each_namespaced_ref(check_ref, NULL);

        }
        string_list_clear(&symref, 1);
        if (advertise_refs)
        }
        string_list_clear(&symref, 1);
        if (advertise_refs)