[GNC-dev] gnucash maint: Bug 796982 - Import Bills & Invoices: change in un_escape() routine...

[Geert Janssens]

Op woensdag 19 december 2018 06:32:16 CET schreef John Ralls:
> Updated	 via  https://github.com/Gnucash/gnucash/commit/f2976420 (commit)
> 	from  https://github.com/Gnucash/gnucash/commit/2524482b (commit)
> 
> 
> 
> commit f29764202ec9f7ace6eb726c246799cb19ad5c1a
> Author: John Ralls <jralls at ceridwen.us>
> Date:   Tue Dec 18 21:29:53 2018 -0800
> 
>     Bug 796982 - Import Bills & Invoices: change in un_escape() routine...
> 
>     causes description and notes fields to be mangled.
> 
>     Simple error, but rewrote the function to be more idiomatic, resisting
>     temptation to abuse the ternary operator.
> 
> diff --git a/gnucash/import-export/bi-import/dialog-bi-import.c
> b/gnucash/import-export/bi-import/dialog-bi-import.c index 5b46e84..6aabcc3
> 100644
> --- a/gnucash/import-export/bi-import/dialog-bi-import.c
> +++ b/gnucash/import-export/bi-import/dialog-bi-import.c
> @@ -876,30 +876,26 @@ gnc_bi_import_create_bis (GtkListStore * store,
> QofBook * book, * @param char* String to be modified
>   * @return char* Modified string.
>  */
> -static char * un_escape(char *str)
> +static char*
> +un_escape(char *str)
>  {
>      gchar quote = '"';
>      gchar *newStr = NULL, *tmpstr = str;
>      int n = 0;
> +
>      newStr = g_malloc(strlen(str)+1); // This must be freed in the calling
> code. while(*tmpstr != '\0')
>      {
>          if(*tmpstr == quote)
> -        {
> -            tmpstr++;
> -            if(*tmpstr == quote)
> -            {
> -                newStr[n] = quote;
> -            }
> -        }
> +            // We always want the character after a quote.
> +            newStr[n] = *(++tmpstr);
>          else
> -        {
> -            newStr[n] = *str;
> -        }
> -            tmpstr++;
> -            n++;
> +            newStr[n] = *tmpstr;
> +        ++tmpstr;
> +        ++n;
>      }
> +
>      g_free (str);
> -	newStr[n] = '\0'; //ending the character array
> +    newStr[n] = '\0'; //ending the character array
>      return newStr;
>  }

Won't this introduce a read-past-end-of-string in case the last character of 
the string is a quote ?

As far as I understand that would make
newStr[n] = '\0'
and tmpstr will be increased twice before it's reevaluated, effectively moving 
it past the final '\0':
Once in
newStr[n] = *(++tmpstr);
and a second time in
++tmpstr;

Geert