Commit 0782fc43f809b1c2a5001453ccb064a4afcfc2b9

Carlos Martín Nieto 2014-09-16T01:47:30

net: use only structs to pass information about cert Instead of spreading the data in function arguments, some of which aren't used for ssh and having a struct only for ssh, use a struct for both, using a common parent to pass to the callback.

diff --git a/include/git2/transport.h b/include/git2/transport.h
index 06d090f..4fa6372 100644
--- a/include/git2/transport.h
+++ b/include/git2/transport.h
@@ -24,6 +24,11 @@ GIT_BEGIN_DECL
  * Hostkey information taken from libssh2
  */
 typedef struct {
+	/**
+	 * Type of certificate. Here to share the header with
+	 * `git_cert`.
+	 */
+	git_cert_t cert_type;
         /**
          * A hostkey type from libssh2, either
          * `LIBSSH2_HOSTKEY_HASH_MD5` or `LIBSSH2_HOSTKEY_HASH_SHA1`
@@ -36,6 +41,25 @@ typedef struct {
         unsigned char hash[20];
 } git_cert_hostkey;
 
+/**
+ * X.509 certificate information
+ */
+typedef struct {
+	/**
+	 * Type of certificate. Here to share the header with
+	 * `git_cert`.
+	 */
+	git_cert_t cert_type;
+	/**
+	 * Pointer to the X.509 certificate data
+	 */
+	void *data;
+	/**
+	 * Length of the memory block pointed to by `data`.
+	 */
+	size_t len;
+} git_cert_x509;
+
 /*
  *** Begin interface for credentials acquisition ***
  */
diff --git a/include/git2/types.h b/include/git2/types.h
index 3544037..7ee7cc3 100644
--- a/include/git2/types.h
+++ b/include/git2/types.h
@@ -253,8 +253,6 @@ typedef int (*git_transfer_progress_cb)(const git_transfer_progress *stats, void
  */
 typedef int (*git_transport_message_cb)(const char *str, int len, void *payload);
 
-
-
 /**
  * Type of host certificate structure that is passed to the check callback
  */
@@ -272,6 +270,16 @@ typedef enum git_cert_t {
 } git_cert_t;
 
 /**
+ * Parent type for `git_cert_hostkey` and `git_cert_x509`.
+ */
+typedef struct {
+	/**
+	 * Type of certificate. A `GIT_CERT_` value.
+	 */
+	git_cert_t cert_type;
+} git_cert;
+
+/**
  * Callback for the user's custom certificate checks.
  *
  * @param type The type of certificate or host info, SSH or X.509
@@ -281,7 +289,7 @@ typedef enum git_cert_t {
  * this certificate is valid
  * @param payload Payload provided by the caller
  */
-typedef int (*git_transport_certificate_check_cb)(git_cert_t type, void *data, size_t len, int valid, void *payload);
+typedef int (*git_transport_certificate_check_cb)(git_cert *cert, int valid, void *payload);
 
 /**
  * Opaque structure representing a submodule.
diff --git a/src/transports/http.c b/src/transports/http.c
index 1bbef81..7ef0b51 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -552,6 +552,7 @@ static int http_connect(http_subtransport *t)
 #ifdef GIT_SSL
 	if ((!error || error == GIT_ECERTIFICATE) && t->owner->certificate_check_cb != NULL) {
                 X509 *cert = SSL_get_peer_certificate(t->socket.ssl.ssl);
+		git_cert_x509 cert_info;
                 int len, is_valid;
 		unsigned char *guard, *encoded_cert;
 
@@ -577,7 +578,10 @@ static int http_connect(http_subtransport *t)
 
 		giterr_clear();
 		is_valid = error != GIT_ECERTIFICATE;
-                error = t->owner->certificate_check_cb(GIT_CERT_X509, encoded_cert, len, is_valid, t->owner->message_cb_payload);
+		cert_info.cert_type = GIT_CERT_X509;
+		cert_info.data = encoded_cert;
+		cert_info.len = len;
+                error = t->owner->certificate_check_cb((git_cert *) &cert_info, is_valid, t->owner->message_cb_payload);
 		git__free(encoded_cert);
 
 		if (error < 0) {
diff --git a/src/transports/ssh.c b/src/transports/ssh.c
index a0f6dd7..2982091 100644
--- a/src/transports/ssh.c
+++ b/src/transports/ssh.c
@@ -484,6 +484,8 @@ static int _git_ssh_setup_conn(
 		const char *key;
 		size_t certlen;
 
+		cert.cert_type = GIT_CERT_HOSTKEY_LIBSSH2;
+
 		cert.type = LIBSSH2_HOSTKEY_HASH_SHA1;
 		key = libssh2_hostkey_hash(session, LIBSSH2_HOSTKEY_HASH_SHA1);
 		if (key != NULL) {
@@ -504,7 +506,7 @@ static int _git_ssh_setup_conn(
 
 		/* We don't currently trust any hostkeys */
 		giterr_clear();
-                error = t->owner->certificate_check_cb(GIT_CERT_HOSTKEY_LIBSSH2, &cert, certlen, 0, t->owner->message_cb_payload);
+                error = t->owner->certificate_check_cb((git_cert *) &cert, 0, t->owner->message_cb_payload);
 		if (error < 0) {
 			if (!giterr_last())
 				giterr_set(GITERR_NET, "user cancelled hostkey check");
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index 0393997..5c74b56 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -211,6 +211,7 @@ static int certificate_check(winhttp_stream *s, int valid)
 	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
 	PCERT_CONTEXT cert_ctx;
 	DWORD cert_ctx_size = sizeof(cert_ctx);
+	git_cert_x509 cert;
 
 	/* If there is no override, we should fail if WinHTTP doesn't think it's fine */
 	if (t->owner->certificate_check_cb == NULL && !valid)
@@ -225,7 +226,10 @@ static int certificate_check(winhttp_stream *s, int valid)
 	}
 
 	giterr_clear();
-	error = t->owner->certificate_check_cb(GIT_CERT_X509, cert_ctx->pbCertEncoded, cert_ctx->cbCertEncoded, valid, t->owner->cred_acquire_payload);
+	cert.cert_type = GIT_CERT_X509;
+	cert.data = cert_ctx->pbCertEncoded;
+	cert.len = cert_ctx->cbCertEncoded;
+	error = t->owner->certificate_check_cb((git_cert *) &cert, valid, t->owner->cred_acquire_payload);
 	CertFreeCertificateContext(cert_ctx);
 
 	if (error < 0 && !giterr_last())
diff --git a/tests/online/clone.c b/tests/online/clone.c
index f88a4d6..2c36b3d 100644
--- a/tests/online/clone.c
+++ b/tests/online/clone.c
@@ -473,13 +473,12 @@ void test_online_clone__ssh_cannot_change_username(void)
 	cl_git_fail(git_clone(&g_repo, "ssh://git@github.com/libgit2/TestGitRepository", "./foo", &g_options));
 }
 
-int ssh_certificate_check(git_cert_t type, void *data, size_t len, int valid, void *payload)
+int ssh_certificate_check(git_cert *cert, int valid, void *payload)
 {
 	git_cert_hostkey *key;
 	git_oid expected = {{0}}, actual = {{0}};
 	const char *expected_str;
 
-	GIT_UNUSED(len);
 	GIT_UNUSED(valid);
 	GIT_UNUSED(payload);
 
@@ -487,10 +486,10 @@ int ssh_certificate_check(git_cert_t type, void *data, size_t len, int valid, vo
 	if (!expected_str)
 		cl_skip();
 
-	cl_git_pass(git_oid_fromstr(&expected, expected_str));
-	cl_assert_equal_i(GIT_CERT_HOSTKEY_LIBSSH2, type);
+	cl_git_pass(git_oid_fromstrp(&expected, expected_str));
+	cl_assert_equal_i(GIT_CERT_HOSTKEY_LIBSSH2, cert->cert_type);
 
-	key = (git_cert_hostkey *) data;
+	key = (git_cert_hostkey *) cert;
 	git_oid_fromraw(&actual, key->hash);
 
 	cl_assert(git_oid_equal(&expected, &actual));
@@ -511,11 +510,9 @@ void test_online_clone__url_with_no_path_returns_EINVALIDSPEC(void)
 		GIT_EINVALIDSPEC);
 }
 
-static int fail_certificate_check(git_cert_t type, void *data, size_t len, int valid, void *payload)
+static int fail_certificate_check(git_cert *cert, int valid, void *payload)
 {
-	GIT_UNUSED(type);
-	GIT_UNUSED(data);
-	GIT_UNUSED(len);
+	GIT_UNUSED(cert);
 	GIT_UNUSED(valid);
 	GIT_UNUSED(payload);
 
@@ -535,11 +532,9 @@ void test_online_clone__certificate_invalid(void)
 #endif
 }
 
-static int succeed_certificate_check(git_cert_t type, void *data, size_t len, int valid, void *payload)
+static int succeed_certificate_check(git_cert *cert, int valid, void *payload)
 {
-	GIT_UNUSED(type);
-	GIT_UNUSED(data);
-	GIT_UNUSED(len);
+	GIT_UNUSED(cert);
 	GIT_UNUSED(valid);
 	GIT_UNUSED(payload);