From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] receive-pack.c: don't miss exporting unsolicited push certificates
Date: Fri, 09 Jan 2015 17:52:11 -0800 [thread overview]
Message-ID: <xmqqmw5r9zck.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1420849874-32013-1-git-send-email-sbeller@google.com> (Stefan Beller's message of "Fri, 9 Jan 2015 16:31:14 -0800")
Stefan Beller <sbeller@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?
... 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@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);
> }
> }
next prev parent reply other threads:[~2015-01-10 1:52 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-09 20:47 [RFC/PATCH] receive-pack.c: only accept push-cert if push_cert_nonce was advertised Stefan Beller
2015-01-09 22:39 ` Junio C Hamano
2015-01-09 23:05 ` Junio C Hamano
2015-01-09 23:15 ` Stefan Beller
2015-01-09 23:57 ` Junio C Hamano
2015-01-10 0:31 ` [PATCH] receive-pack.c: don't miss exporting unsolicited push certificates Stefan Beller
2015-01-10 1:52 ` Junio C Hamano [this message]
2015-01-10 3:55 ` Stefan Beller
2015-01-12 19:07 ` Junio C Hamano
2015-01-14 0:11 ` Stefan Beller
2015-01-14 18:08 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmw5r9zck.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox