Stefan Beller <sbeller@xxxxxxxxxx> writes:
> If the server is configured to not advertise push certificates,
> a push certificate that gets pushed nevertheless will not be fully
> recorded because push_cert_nonce is NULL.
Sorry, but I do not quite see what you are trying to get at.
When we did not advertise that this feature is supported, we know
the incoming nonce is meaningless junk because we didn't supply the
correct answer the pusher can give us; we do not even have the
correct answer ourselves.
Besides, while reviewing the other patch, didn't we agree that we
should reject such a push? Then there is nothing to log anyway, no?
... reads the patch and code beyond the context while scratching
head ...
I notice that the above three lines do not correspond what you did
in this patch. The certificate is exported via the blob interface
fully with or without this change.
What is not given to the hook is the push-cert-nonce-status. While
it is sufficient to tell the hook that we are not getting a good
nonce (i.e. the hook does not see GIT_PUSH_CERT_NONCE_STATUS=G), we
are not showing that nonce-status is "unsolicited", even though we
internally compute and decide that; is that what you are trying to
fix?
Still puzzled...
>
> The recording of GIT_PUSH_CERT_NONCE_STATUS should be dependent on
> the status being there instead of push_cert_nonce being non NULL.
>
> Without this patch an unsolicited nonce never makes to the stage when
> being exported with GIT_PUSH_CERT_NONCE_STATUS, because in the unsolicited
> case push_cert_nonce is always NULL.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> builtin/receive-pack.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 628d13a..0e4878e 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -504,18 +504,18 @@ static void prepare_push_cert_sha1(struct child_process *proc)
> sigcheck.key ? sigcheck.key : "");
> argv_array_pushf(&proc->env_array, "GIT_PUSH_CERT_STATUS=%c",
> sigcheck.result);
> - if (push_cert_nonce) {
> + if (push_cert_nonce)
> argv_array_pushf(&proc->env_array,
> "GIT_PUSH_CERT_NONCE=%s",
> push_cert_nonce);
> + if (nonce_status)
> argv_array_pushf(&proc->env_array,
> "GIT_PUSH_CERT_NONCE_STATUS=%s",
> nonce_status);
> - if (nonce_status == NONCE_SLOP)
> - argv_array_pushf(&proc->env_array,
> - "GIT_PUSH_CERT_NONCE_SLOP=%ld",
> - nonce_stamp_slop);
> - }
> + if (nonce_status == NONCE_SLOP)
> + argv_array_pushf(&proc->env_array,
> + "GIT_PUSH_CERT_NONCE_SLOP=%ld",
> + nonce_stamp_slop);
> }
> }
--
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