From: Pietro Cerutti <gahr@gahr.ch>
Date: Wed, 11 Apr 2018 14:01:57 +0000 (+0100)
Subject:  Let mutt_ch_choose report conversion failure (#1137)
X-Git-Tag: neomutt-20180512~58
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9d713fb8391b1993e899f7846205ff5fe17ec2c4;p=neomutt

 Let mutt_ch_choose report conversion failure (#1137)

* Let mutt_ch_choose report conversion failure

Issue #1136

* Let mutt_ch_convert_string return errno if iconv(..) fails

Previously, mutt_ch_convert_string returned error only if any arguments
failed the sanity check or iconv_open() failed.

With this change, an error value is returned even if the iconv()
conversion fails, such as when the input string contains an invalid
character in the source character set (EILSEQ).

I have looked at each caller and it seems to me that this change won't
introduce any misbehaviour, but I'm far from being 100% confident.

Issue #1136

* test int functions against 0

* Better error reporting in mutt_ch_iconv

Issue #1136
---

diff --git a/handler.c b/handler.c
index 7c51d91a0..ad4b87709 100644
--- a/handler.c
+++ b/handler.c
@@ -102,7 +102,7 @@ static void convert_to_state(iconv_t cd, char *bufi, size_t *l, struct State *s)
   while (true)
   {
     ob = bufo, obl = sizeof(bufo);
-    mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, 0, "?");
+    mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, 0, "?", NULL);
     if (ob == bufo)
       break;
     state_prefix_put(bufo, ob - bufo, s);
diff --git a/imap/utf7.c b/imap/utf7.c
index 7c3bfa9b9..a7d9854c3 100644
--- a/imap/utf7.c
+++ b/imap/utf7.c
@@ -320,7 +320,7 @@ void imap_utf_encode(struct ImapData *idata, char **s)
   if (Charset)
   {
     char *t = mutt_str_strdup(*s);
-    if (t && !mutt_ch_convert_string(&t, Charset, "utf-8", 0))
+    if (t && mutt_ch_convert_string(&t, Charset, "utf-8", 0) == 0)
     {
       FREE(s);
       if (idata->unicode)
@@ -348,7 +348,7 @@ void imap_utf_decode(struct ImapData *idata, char **s)
     else
       t = utf7_to_utf8(*s, strlen(*s), 0, 0);
 
-    if (t && !mutt_ch_convert_string(&t, "utf-8", Charset, 0))
+    if (t && mutt_ch_convert_string(&t, "utf-8", Charset, 0) == 0)
     {
       FREE(s);
       *s = t;
diff --git a/mutt/charset.c b/mutt/charset.c
index 26635ae86..6ecb7e163 100644
--- a/mutt/charset.c
+++ b/mutt/charset.c
@@ -564,6 +564,7 @@ iconv_t mutt_ch_iconv_open(const char *tocode, const char *fromcode, int flags)
  * @param[in,out] outbytesleft Length of result buffer
  * @param[in]     inrepls      Input replacement characters
  * @param[in]     outrepl      Output replacement characters
+ * @param[out]    iconverrno   Errno if iconv() fails, 0 if it succeeds
  * @retval num Number of characters converted
  *
  * Like iconv, but keeps going even when the input is invalid
@@ -571,7 +572,8 @@ iconv_t mutt_ch_iconv_open(const char *tocode, const char *fromcode, int flags)
  * if you're supplying an outrepl, the target charset should be.
  */
 size_t mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char **outbuf,
-                     size_t *outbytesleft, const char **inrepls, const char *outrepl)
+                     size_t *outbytesleft, const char **inrepls, const char *outrepl,
+                     int *iconverrno)
 {
   size_t rc = 0;
   const char *ib = *inbuf;
@@ -581,9 +583,13 @@ size_t mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char *
 
   while (true)
   {
+    errno = 0;
     const size_t ret1 = iconv(cd, (ICONV_CONST char **) &ib, &ibl, &ob, &obl);
     if (ret1 != (size_t) -1)
       rc += ret1;
+    if (iconverrno)
+      *iconverrno = errno;
+
     if (ibl && obl && errno == EILSEQ)
     {
       if (inrepls)
@@ -660,8 +666,9 @@ const char *mutt_ch_iconv_lookup(const char *chs)
  * @param[in]     from  Current character set
  * @param[in]     to    Target character set
  * @param[in]     flags Flags, e.g.
- * @retval 0  Success
- * @retval -1 Error
+ * @retval 0      Success
+ * @retval -1     Invalid arguments or failure to open an iconv channel
+ * @retval errno  Failure in iconv conversion
  *
  * Parameter flags is given as-is to mutt_ch_iconv_open().
  * See there for its meaning and usage policy.
@@ -671,45 +678,48 @@ int mutt_ch_convert_string(char **ps, const char *from, const char *to, int flag
   iconv_t cd;
   const char *repls[] = { "\357\277\275", "?", 0 };
   char *s = *ps;
+  int rc = 0;
 
   if (!s || !*s)
     return 0;
 
-  if (to && from && (cd = mutt_ch_iconv_open(to, from, flags)) != (iconv_t) -1)
-  {
-    size_t len;
-    const char *ib = NULL;
-    char *buf = NULL, *ob = NULL;
-    size_t ibl, obl;
-    const char **inrepls = NULL;
-    char *outrepl = NULL;
-
-    if (mutt_ch_is_utf8(to))
-      outrepl = "\357\277\275";
-    else if (mutt_ch_is_utf8(from))
-      inrepls = repls;
-    else
-      outrepl = "?";
+  if (!to || !from)
+    return -1;
 
-    len = strlen(s);
-    ib = s;
-    ibl = len + 1;
-    obl = MB_LEN_MAX * ibl;
-    ob = buf = mutt_mem_malloc(obl + 1);
+  cd = mutt_ch_iconv_open(to, from, flags);
+  if (cd == (iconv_t) -1)
+    return -1;
 
-    mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, inrepls, outrepl);
-    iconv_close(cd);
+  size_t len;
+  const char *ib = NULL;
+  char *buf = NULL, *ob = NULL;
+  size_t ibl, obl;
+  const char **inrepls = NULL;
+  char *outrepl = NULL;
+
+  if (mutt_ch_is_utf8(to))
+    outrepl = "\357\277\275";
+  else if (mutt_ch_is_utf8(from))
+    inrepls = repls;
+  else
+    outrepl = "?";
 
-    *ob = '\0';
+  len = strlen(s);
+  ib = s;
+  ibl = len + 1;
+  obl = MB_LEN_MAX * ibl;
+  ob = buf = mutt_mem_malloc(obl + 1);
 
-    FREE(ps);
-    *ps = buf;
+  mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, inrepls, outrepl, &rc);
+  iconv_close(cd);
 
-    mutt_str_adjust(ps);
-    return 0;
-  }
-  else
-    return -1;
+  *ob = '\0';
+
+  FREE(ps);
+  *ps = buf;
+
+  mutt_str_adjust(ps);
+  return rc;
 }
 
 /**
@@ -848,7 +858,7 @@ int mutt_ch_fgetconv(struct FgetConv *fc)
   if (fc->ibl)
   {
     size_t obl = sizeof(fc->bufo);
-    mutt_ch_iconv(fc->cd, (const char **) &fc->ib, &fc->ibl, &fc->ob, &obl, fc->inrepls, 0);
+    mutt_ch_iconv(fc->cd, (const char **) &fc->ib, &fc->ibl, &fc->ob, &obl, fc->inrepls, 0, NULL);
     if (fc->p < fc->ob)
       return (unsigned char) *(fc->p)++;
   }
@@ -954,7 +964,7 @@ char *mutt_ch_choose(const char *fromcode, const char *charsets, char *u,
     t[n] = '\0';
 
     s = mutt_str_substr_dup(u, u + ulen);
-    if (mutt_ch_convert_string(&s, fromcode, t, 0))
+    if (mutt_ch_convert_string(&s, fromcode, t, 0) != 0)
     {
       FREE(&t);
       FREE(&s);
diff --git a/mutt/charset.h b/mutt/charset.h
index 1c585d19e..2597d4c4f 100644
--- a/mutt/charset.h
+++ b/mutt/charset.h
@@ -93,7 +93,7 @@ void             mutt_ch_lookup_remove(void);
 const char *     mutt_ch_charset_lookup(const char *chs);
 
 iconv_t          mutt_ch_iconv_open(const char *tocode, const char *fromcode, int flags);
-size_t           mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char **outbuf, size_t *outbytesleft, const char **inrepls, const char *outrepl);
+size_t           mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char **outbuf, size_t *outbytesleft, const char **inrepls, const char *outrepl, int *iconverrno);
 const char *     mutt_ch_iconv_lookup(const char *chs);
 int              mutt_ch_convert_string(char **ps, const char *from, const char *to, int flags);
 int              mutt_ch_convert_nonmime_string(char **ps);
diff --git a/mutt/idna.c b/mutt/idna.c
index b8fdb1646..2ecf680a7 100644
--- a/mutt/idna.c
+++ b/mutt/idna.c
@@ -142,10 +142,10 @@ char *mutt_idna_intl_to_local(const char *user, const char *domain, int flags)
 #endif /* HAVE_LIBIDN */
 
   /* we don't want charset-hook effects, so we set flags to 0 */
-  if (mutt_ch_convert_string(&local_user, "utf-8", Charset, 0) == -1)
+  if (mutt_ch_convert_string(&local_user, "utf-8", Charset, 0) != 0)
     goto cleanup;
 
-  if (mutt_ch_convert_string(&local_domain, "utf-8", Charset, 0) == -1)
+  if (mutt_ch_convert_string(&local_domain, "utf-8", Charset, 0) != 0)
     goto cleanup;
 
   /* make sure that we can convert back and come out with the same
@@ -154,7 +154,7 @@ char *mutt_idna_intl_to_local(const char *user, const char *domain, int flags)
   {
     reversed_user = mutt_str_strdup(local_user);
 
-    if (mutt_ch_convert_string(&reversed_user, Charset, "utf-8", 0) == -1)
+    if (mutt_ch_convert_string(&reversed_user, Charset, "utf-8", 0) != 0)
     {
       mutt_debug(1, "Not reversible. Charset conv to utf-8 failed for user = '%s'.\n",
                  reversed_user);
@@ -169,7 +169,7 @@ char *mutt_idna_intl_to_local(const char *user, const char *domain, int flags)
 
     reversed_domain = mutt_str_strdup(local_domain);
 
-    if (mutt_ch_convert_string(&reversed_domain, Charset, "utf-8", 0) == -1)
+    if (mutt_ch_convert_string(&reversed_domain, Charset, "utf-8", 0) != 0)
     {
       mutt_debug(1, "Not reversible. Charset conv to utf-8 failed for domain = '%s'.\n",
                  reversed_domain);
@@ -236,10 +236,10 @@ char *mutt_idna_local_to_intl(const char *user, const char *domain)
   char *intl_domain = mutt_str_strdup(domain);
 
   /* we don't want charset-hook effects, so we set flags to 0 */
-  if (mutt_ch_convert_string(&intl_user, Charset, "utf-8", 0) == -1)
+  if (mutt_ch_convert_string(&intl_user, Charset, "utf-8", 0) != 0)
     goto cleanup;
 
-  if (mutt_ch_convert_string(&intl_domain, Charset, "utf-8", 0) == -1)
+  if (mutt_ch_convert_string(&intl_domain, Charset, "utf-8", 0) != 0)
     goto cleanup;
 
 #ifdef HAVE_LIBIDN
diff --git a/mutt/rfc2047.c b/mutt/rfc2047.c
index 47b44b5ef..e9ef6d89f 100644
--- a/mutt/rfc2047.c
+++ b/mutt/rfc2047.c
@@ -439,7 +439,7 @@ static int rfc2047_encode(const char *d, size_t dlen, int col, const char *fromc
 
   /* Try to convert to UTF-8. */
   char *u = mutt_str_substr_dup(d, d + dlen);
-  if (mutt_ch_convert_string(&u, fromcode, icode, 0))
+  if (mutt_ch_convert_string(&u, fromcode, icode, 0) != 0)
   {
     rc = 1;
     icode = 0;