imap.preformattedHTML and imap.sslverify

* imap.preformattedHTML and imap.sslverify
@ 2010-02-06 19:26 Junio C Hamano
  2010-02-08 22:31 ` Jeremy White
  0 siblings, 1 reply; 49+ messages in thread
From: Junio C Hamano @ 2010-02-06 19:26 UTC (permalink / raw)
  To: git; +Cc: Jeremy White, Robert Shearman

I hate to bring up a topic that is almost a year old, but has either of
these configuration variables ever worked?

The code does this while reading its configuration file:

        static int git_imap_config(const char *key, const char *val, void *cb)
        {
                char imap_key[] = "imap.";

                if (strncmp(key, imap_key, sizeof imap_key - 1))
                        return 0;

                if (!val)
                        return config_error_nonbool(key);
                ...
                else if (!strcmp("sslverify", key))
                        server.ssl_verify = git_config_bool(key, val);
                else if (!strcmp("preformattedHTML", key))
                        server.use_html = git_config_bool(key, val);

Two issues:

 - The body of the function is protected by "nonbool" written back in the
   days when there was no boolean variables in imap.* namespace.  Hence,
   a user cannot write

           [imap]
                sslverify

   and turn it on.  The user needs to write


           [imap]
                sslverify = True

   which is against the parsing rules for boolean variables.

 - The config parser downcases the key before calling the parse callback
   function, so !strcmp("preformattedHTML", key) will never trigger.

The fix is obvious (see below), but I am far more disturbed by the
apparent lack of testing.  Especially, preformattedHTML one would have
never worked as setting the configuration is the only way to trigger this.

Could peole _test_ this patch and report, as I don't use this program at
all.

Thanks.

 imap-send.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index de8114b..ea769a9 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1335,11 +1335,16 @@ static int git_imap_config(const char *key, const char *val, void *cb)
 	if (strncmp(key, imap_key, sizeof imap_key - 1))
 		return 0;
 
-	if (!val)
-		return config_error_nonbool(key);
-
 	key += sizeof imap_key - 1;
 
+	/* check booleans first, and barf on others */
+	if (!strcmp("sslverify", key))
+		server.ssl_verify = git_config_bool(key, val);
+	else if (!strcmp("preformattedhtml", key))
+		server.use_html = git_config_bool(key, val);
+	else if (!val)
+		return config_error_nonbool(key);
+
 	if (!strcmp("folder", key)) {
 		imap_folder = xstrdup(val);
 	} else if (!strcmp("host", key)) {
@@ -1360,10 +1365,6 @@ static int git_imap_config(const char *key, const char *val, void *cb)
 		server.port = git_config_int(key, val);
 	else if (!strcmp("tunnel", key))
 		server.tunnel = xstrdup(val);
-	else if (!strcmp("sslverify", key))
-		server.ssl_verify = git_config_bool(key, val);
-	else if (!strcmp("preformattedHTML", key))
-		server.use_html = git_config_bool(key, val);
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 49+ messages in thread