This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [BZ # 14672] Fix iconv -c


On Fri, Nov 16, 2012 at 6:03 AM, Andreas Jaeger <aj@suse.com> wrote:
> Pasting the description from Bugzilla:
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> While studying the source code of the iconv(1) program, I noticed a mistake
> in
> the code handling the '-c' switch (which orders iconv to ignore any invalid
> characters in the input stream). The mistake causes the program not to
> recognise some of the encoding names in the presence of the '-c' flag, but
> the
> conditions for observing the bug are relatively exotic, so it is rather
> probable that nobody had ever stumbled upon the problem.
>
> This is the relevant snippet of the code from main() in iconv/iconv_prog.c:
>
> 156:      if (*errhand == '/')
> 157:    {
> 158:      --nslash;
> 159:      errhand = strchrnul (errhand, '/');
> 160:
> 161:      if (*errhand == '/')
> 162:        {
> 163:          --nslash;
> 164:          errhand = strchr (errhand, '\0');
> 165:        }
> 166:    }
>
> Notice that line 159 has no effect at all, since it searches for the first
> occurrence of '/' in a string that starts with a '/' (as tested in line
> 156).
> The program should instead be calling strchrnul(errhand+1, '/'), since we
> are
> trying to find the *next* slash in the string.
>
> The consequence is that a single slash gets counted twice and the code that
> follows (not shown above) later fails to append one even if it should. The
> bug
> only shows when encoding names of the form ABC/DEF are used, i.e., a call to
>
> iconv -c -f LATIN1 -t ISO/TR_11548-1
>
> fails because the target encoding name gets rewritten into
> "ISO/TR_11548-1,IGNORE" instead of "ISO/TR_11548-1/IGNORE" and iconv no
> longer
> recognises this as a valid encoding name.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> I've tested the appended patch on Linux/x86-64 and it handles now this
> case.
>
> Ok to commit?
>
> Andreas
>
>
> 2012-11-16  Andrej Lajovic  <natrij@gmail.com>
>
>         [BZ #14672]
>         * iconv/iconv_prog.c (main): Fix -c handling of '/'.

Looks good to me.

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]