Jonathan Tan <jonathanta...@google.com> writes:

> Git advertises the same capabilities^{} ref in its ref advertisement for push
> but since it never did so for fetch, the client didn't need to handle this
> case. Handle it.
>
> In this aspect, JGit is compliant with the specification in pack-protocol.txt.

The last sentence somehow looks out of place.

> +     int got_dummy_ref_with_capabilities_declaration = 0;
>  
>       *list = NULL;
>       for (saw_response = 0; ; saw_response = 1) {
> @@ -172,8 +173,24 @@ struct ref **get_remote_heads(int in, char *src_buf, 
> size_t src_len,
>                       continue;
>               }
>  
> +             if (!strcmp(name, "capabilities^{}")) {
> +                     if (saw_response)
> +                             warning("protocol error: unexpected 
> capabilities^{}, "
> +                                     "continuing anyway");

OK.  saw_response tells us that we saw ".have", a valid ref, or "shallow",
but "capabilities^{}" should happen before any of them, so that is a
protocol violation.  Makes perfect sense.

> +                     if (got_dummy_ref_with_capabilities_declaration)
> +                             warning("protocol error: multiple 
> capabilities^{}, "
> +                                     "continuing anyway");
> +                     got_dummy_ref_with_capabilities_declaration = 1;
> +                     continue;
> +             }
> +
>               if (!check_ref(name, flags))
>                       continue;
> +
> +             if (got_dummy_ref_with_capabilities_declaration)
> +                     warning("protocol error: unexpected ref after 
> capabilities^{}, "
> +                             "using this ref and continuing anyway");

Likewise. "capabilities^{}" is used when we cannot piggyback the
capability list after a real ref, so it is unusual to see a real ref
after seeing one.  Makes perfect sense.

Do we want to abort the connection in these cases, I wonder, though?

Thanks.

Reply via email to