When we create a signature, it may happen that gpg returns with
"success" but not with an actual detached signature on stdout.
Check for the correct signature creation status to catch these cases
better. Really, --status-fd parsing is the only way to check gpg status
reliably. We do the same for verify already.
Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
Obviously the big change from your v3 is that this just passes the
buffer to pipe_command() instead of doing the read itself.
I changed the name of your "err" strbuf to "gpg_status" to be more
descriptive, and to match the similar usage on the verification side.
In the test, I bumped the config-setting inside the test, and used
test_config to get the automatic cleanup (using "git -c" would also have
worked).
I left the test description as-is, though it is a bit misleading. We are
not really checking for bad input anymore, but rather "did gpg send us
the special stats". Your fake "echo" fails on both counts, so the test
is still effective. A more robust test would be to output something that
_looked_ like real gpg output, but not to output the correct status
token.
gpg-interface.c | 8 ++++++--
t/t7004-tag.sh | 9 ++++++++-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index 74f08a2..08356f9 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -153,9 +153,11 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
struct child_process gpg = CHILD_PROCESS_INIT;
int ret;
size_t i, j, bottom;
+ struct strbuf gpg_status = STRBUF_INIT;
argv_array_pushl(&gpg.args,
gpg_program,
+ "--status-fd=2",
"-bsau", signing_key,
NULL);
@@ -167,10 +169,12 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
*/
sigchain_push(SIGPIPE, SIG_IGN);
ret = pipe_command(&gpg, buffer->buf, buffer->len,
- signature, 1024, NULL, 0);
+ signature, 1024, &gpg_status, 0);
sigchain_pop(SIGPIPE);
- if (ret || signature->len == bottom)
+ ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
+ strbuf_release(&gpg_status);
+ if (ret)
return error(_("gpg failed to sign the data"));
/* Strip CR from the line endings, in case we are on Windows. */
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f9b7d79..8b0f71a 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1202,10 +1202,17 @@ test_expect_success GPG,RFC1991 \
# try to sign with bad user.signingkey
git config user.signingkey BobTheMouse
test_expect_success GPG \
- 'git tag -s fails if gpg is misconfigured' \
+ 'git tag -s fails if gpg is misconfigured (bad key)' \
'test_must_fail git tag -s -m tail tag-gpg-failure'
git config --unset user.signingkey
+# try to produce invalid signature
+test_expect_success GPG \
+ 'git tag -s fails if gpg is misconfigured (bad signature format)' \
+ 'test_config gpg.program echo &&
+ test_must_fail git tag -s -m tail tag-gpg-failure'
+
+
# try to verify without gpg:
rm -rf gpghome
--
2.9.0.160.g4984cba
--
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