On 09.01.2015 17:52, Junio C Hamano wrote:
> Stefan Beller <sbel...@google.com> 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?
Yes we did. This is unrelated to the previous patch. I stuffed it into
this thread as I found it was touching the same feature.
>
> ... 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.
I had problems with wording the commit message because I have no
expertise with the feature. I am sorry for wasting your time there.
>
> 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?
yes that's what I was trying to hint at. The hook would just see
it is unsolicited instead of not having the state available.
>
> 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 <sbel...@google.com>
>> ---
>> 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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
