From patchwork Thu Feb 25 08:58:21 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Liebler X-Patchwork-Id: 11076 Received: (qmail 90772 invoked by alias); 25 Feb 2016 09:00:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 90753 invoked by uid 89); 25 Feb 2016 09:00:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=BODY, 0x80, 8927, 892, 7 X-HELO: plane.gmane.org To: libc-alpha@sourceware.org From: Stefan Liebler Subject: Re: [PATCH 14/14] Fix UTF-16 surrogate handling. Date: Thu, 25 Feb 2016 09:58:21 +0100 Lines: 387 Message-ID: References: <1456219278-5258-1-git-send-email-stli@linux.vnet.ibm.com> <1456219278-5258-15-git-send-email-stli@linux.vnet.ibm.com> Mime-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 In-Reply-To: On 02/23/2016 06:42 PM, Joseph Myers wrote: > If this is user-visible in a release, there should be a bug filed in > Bugzilla (if there isn't one already open), and a testcase added to the > testsuite. > okay. I've filed the bug "Bug 19727 - Converting from/to UTF-xx with iconv() does not always report errors on UTF-16 surrogates values." (https://sourceware.org/bugzilla/show_bug.cgi?id=19727) This patch also adds a new testcase, which checks UTF conversions with input values in range of UTF16 surrogates. The test converts from UTF-xx to INTERNAL, INTERNAL to UTF-xx and directly between UTF-xx to UTF-yy. The latter conversion is needed because s390 has iconv-modules, which converts from/to UTF in one step. The new testcase was tested on a s390, power and intel machine. ChangeLog: [BZ #19727] * iconvdata/utf-16.c (BODY): Report an error if first word is not a valid high surrogate. * iconvdata/utf-32.c (BODY): Report an error if the value is in range of an utf16 surrogate. * iconv/gconv_simple.c (BODY): Likewise. * iconv/tst-iconv7.c: New file. * iconv/Makefile (tests): Add tst-iconv7. diff --git a/iconv/Makefile b/iconv/Makefile index c2299c9..30c8e83 100644 --- a/iconv/Makefile +++ b/iconv/Makefile @@ -42,7 +42,8 @@ CFLAGS-charmap.c = -DCHARMAP_PATH='"$(i18ndir)/charmaps"' \ CFLAGS-linereader.c = -DNO_TRANSLITERATION CFLAGS-simple-hash.c = -I../locale -tests = tst-iconv1 tst-iconv2 tst-iconv3 tst-iconv4 tst-iconv5 tst-iconv6 +tests = tst-iconv1 tst-iconv2 tst-iconv3 tst-iconv4 tst-iconv5 tst-iconv6 \ + tst-iconv7 others = iconv_prog iconvconfig install-others-programs = $(inst_bindir)/iconv diff --git a/iconv/gconv_simple.c b/iconv/gconv_simple.c index f66bf34..e5284e4 100644 --- a/iconv/gconv_simple.c +++ b/iconv/gconv_simple.c @@ -892,7 +892,8 @@ ucs4le_internal_loop_single (struct __gconv_step *step, if (__glibc_likely (wc < 0x80)) \ /* It's an one byte sequence. */ \ *outptr++ = (unsigned char) wc; \ - else if (__glibc_likely (wc <= 0x7fffffff)) \ + else if (__glibc_likely (wc <= 0x7fffffff \ + && (wc < 0xd800 || wc > 0xdfff))) \ { \ size_t step; \ unsigned char *start; \ diff --git a/iconv/tst-iconv7.c b/iconv/tst-iconv7.c new file mode 100644 index 0000000..fc2e33e --- /dev/null +++ b/iconv/tst-iconv7.c @@ -0,0 +1,263 @@ +/* Testing UTF conversions with UTF16 surrogates as input. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include + +static int +run_conversion (const char *from, const char *to, char *inbuf, size_t inbuflen + , int exp_errno, int line) +{ + char outbuf[16]; + iconv_t cd; + char *inptr; + size_t inlen; + char *outptr; + size_t outlen; + size_t n; + int e; + int fails = 0; + + cd = iconv_open (to, from); + if (cd == (iconv_t) -1) + { + printf ("line %d: cannot convert from %s to %s: %m\n", line, from, to); + return 1; + } + + inptr = (char *) inbuf; + inlen = inbuflen; + outptr = outbuf; + outlen = sizeof (outbuf); + + errno = 0; + n = iconv (cd, &inptr, &inlen, &outptr, &outlen); + e = errno; + + if (exp_errno == 0) + { + if (n == (size_t) -1) + { + puts ("n should be >= 0, but n == -1"); + fails ++; + } + + if (e != 0) + { + printf ("errno should be 0: 'Success', but errno == %d: '%s'\n" + , e, strerror(e)); + fails ++; + } + } + else + { + if (n != (size_t) -1) + { + printf ("n should be -1, but n == %zd\n", n); + fails ++; + } + + if (e != exp_errno) + { + printf ("errno should be %d: '%s', but errno == %d: '%s'\n" + , exp_errno, strerror (exp_errno), e, strerror (e)); + fails ++; + } + } + + iconv_close (cd); + + if (fails > 0) + { + printf ("Errors in line %d while converting %s to %s.\n\n" + , line, from, to); + } + + return fails; +} + +static int +do_test (void) +{ + int fails = 0; + char buf[4]; + + /* This test runs iconv() with UTF character in range of an UTF16 surrogate. + UTF-16 high surrogate is in range 0xD800..0xDBFF and + UTF-16 low surrogate is in range 0xDC00..0xDFFF. + Converting from or to UTF-xx has to report errors in those cases. + In UTF-16, surrogate pairs with a high surrogate in front of a low + surrogate is valid. */ + + /* Use RUN_UCS4_UTF32_INPUT to test conversion ... + + ... from INTERNAL to UTF-xx[LE|BE]: + Converting from UCS4 to UTF-xx[LE|BE] first converts UCS4 to INTERNAL + without checking for UTF-16 surrogate values + and then converts from INTERNAL to UTF-xx[LE|BE]. + The latter conversion has to report an error in those cases. + + ... from UTF-32[LE|BE] to INTERNAL: + Converting directly from UTF-32LE to UTF-8|16 is needed, + because e.g. s390x has iconv-modules which converts directly. */ +#define RUN_UCS4_UTF32_INPUT(b0, b1, b2, b3, err, line) \ + buf[0] = b0; \ + buf[1] = b1; \ + buf[2] = b2; \ + buf[3] = b3; \ + fails += run_conversion ("UCS4", "UTF-8", buf, 4, err, line); \ + fails += run_conversion ("UCS4", "UTF-16LE", buf, 4, err, line); \ + fails += run_conversion ("UCS4", "UTF-16BE", buf, 4, err, line); \ + fails += run_conversion ("UCS4", "UTF-32LE", buf, 4, err, line); \ + fails += run_conversion ("UCS4", "UTF-32BE", buf, 4, err, line); \ + fails += run_conversion ("UTF-32BE", "WCHAR_T", buf, 4, err, line); \ + fails += run_conversion ("UTF-32BE", "UTF-8", buf, 4, err, line); \ + fails += run_conversion ("UTF-32BE", "UTF-16LE", buf, 4, err, line); \ + fails += run_conversion ("UTF-32BE", "UTF-16BE", buf, 4, err, line); \ + buf[0] = b3; \ + buf[1] = b2; \ + buf[2] = b1; \ + buf[3] = b0; \ + fails += run_conversion ("UTF-32LE", "WCHAR_T", buf, 4, err, line); \ + fails += run_conversion ("UTF-32LE", "UTF-8", buf, 4, err, line); \ + fails += run_conversion ("UTF-32LE", "UTF-16LE", buf, 4, err, line); \ + fails += run_conversion ("UTF-32LE", "UTF-16BE", buf, 4, err, line); + + /* Use UCS4/UTF32 input of 0xD7FF. */ + RUN_UCS4_UTF32_INPUT (0x0, 0x0, 0xD7, 0xFF, 0, __LINE__); + + /* Use UCS4/UTF32 input of 0xD800. */ + RUN_UCS4_UTF32_INPUT (0x0, 0x0, 0xD8, 0x00, EILSEQ, __LINE__); + + /* Use UCS4/UTF32 input of 0xDBFF. */ + RUN_UCS4_UTF32_INPUT (0x0, 0x0, 0xDB, 0xFF, EILSEQ, __LINE__); + + /* Use UCS4/UTF32 input of 0xDC00. */ + RUN_UCS4_UTF32_INPUT (0x0, 0x0, 0xDC, 0x00, EILSEQ, __LINE__); + + /* Use UCS4/UTF32 input of 0xDFFF. */ + RUN_UCS4_UTF32_INPUT (0x0, 0x0, 0xDF, 0xFF, EILSEQ, __LINE__); + + /* Use UCS4/UTF32 input of 0xE000. */ + RUN_UCS4_UTF32_INPUT (0x0, 0x0, 0xE0, 0x00, 0, __LINE__); + + + /* Use RUN_UTF16_INPUT to test conversion from UTF16[LE|BE] to INTERNAL. + Converting directly from UTF-16 to UTF-8|32 is needed, + because e.g. s390x has iconv-modules which converts directly. + Use len == 2 or 4 to specify one or two UTF-16 characters. */ +#define RUN_UTF16_INPUT(b0, b1, b2, b3, len, err, line) \ + buf[0] = b0; \ + buf[1] = b1; \ + buf[2] = b2; \ + buf[3] = b3; \ + fails += run_conversion ("UTF-16BE", "WCHAR_T", buf, len, err, line); \ + fails += run_conversion ("UTF-16BE", "UTF-8", buf, len, err, line); \ + fails += run_conversion ("UTF-16BE", "UTF-32LE", buf, len, err, line); \ + fails += run_conversion ("UTF-16BE", "UTF-32BE", buf, len, err, line); \ + buf[0] = b1; \ + buf[1] = b0; \ + buf[2] = b3; \ + buf[3] = b2; \ + fails += run_conversion ("UTF-16LE", "WCHAR_T", buf, len, err, line); \ + fails += run_conversion ("UTF-16LE", "UTF-8", buf, len, err, line); \ + fails += run_conversion ("UTF-16LE", "UTF-32LE", buf, len, err, line); \ + fails += run_conversion ("UTF-16LE", "UTF-32BE", buf, len, err, line); + + /* Use UTF16 input of 0xD7FF. */ + RUN_UTF16_INPUT (0xD7, 0xFF, 0xD7, 0xFF, 4, 0, __LINE__); + + /* Use [single] UTF16 high surrogate 0xD800 [with a valid character behind]. + And check an UTF16 surrogate pair [without valid low surrogate]. */ + RUN_UTF16_INPUT (0xD8, 0x0, 0x0, 0x0, 2, EINVAL, __LINE__); + RUN_UTF16_INPUT (0xD8, 0x0, 0xD7, 0xFF, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xD8, 0x0, 0xD8, 0x0, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xD8, 0x0, 0xE0, 0x0, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xD8, 0x0, 0xDC, 0x0, 4, 0, __LINE__); + + /* Use [single] UTF16 high surrogate 0xDBFF [with a valid character behind]. + And check an UTF16 surrogate pair [without valid low surrogate]. */ + RUN_UTF16_INPUT (0xDB, 0xFF, 0x0, 0x0, 2, EINVAL, __LINE__); + RUN_UTF16_INPUT (0xDB, 0xFF, 0xD7, 0xFF, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDB, 0xFF, 0xDB, 0xFF, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDB, 0xFF, 0xE0, 0x0, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDB, 0xFF, 0xDF, 0xFF, 4, 0, __LINE__); + + /* Use single UTF16 low surrogate 0xDC00 [with a valid character behind]. + And check an UTF16 surrogate pair [without valid high surrogate]. */ + RUN_UTF16_INPUT (0xDC, 0x0, 0x0, 0x0, 2, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDC, 0x0, 0xD7, 0xFF, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xD8, 0x0, 0xDC, 0x0, 4, 0, __LINE__); + RUN_UTF16_INPUT (0xD7, 0xFF, 0xDC, 0x0, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDC, 0x0, 0xDC, 0x0, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xE0, 0x0, 0xDC, 0x0, 4, EILSEQ, __LINE__); + + /* Use single UTF16 low surrogate 0xDFFF [with a valid character behind]. + And check an UTF16 surrogate pair [without valid high surrogate]. */ + RUN_UTF16_INPUT (0xDF, 0xFF, 0x0, 0x0, 2, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDF, 0xFF, 0xD7, 0xFF, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDB, 0xFF, 0xDF, 0xFF, 4, 0, __LINE__); + RUN_UTF16_INPUT (0xD7, 0xFF, 0xDF, 0xFF, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xDF, 0xFF, 0xDF, 0xFF, 4, EILSEQ, __LINE__); + RUN_UTF16_INPUT (0xE0, 0x0, 0xDF, 0xFF, 4, EILSEQ, __LINE__); + + /* Use UCS4/UTF32 input of 0xE000. */ + RUN_UTF16_INPUT (0xE0, 0x0, 0xE0, 0x0, 4, 0, __LINE__); + + + /* Use RUN_UTF8_3BYTE_INPUT to test conversion from UTF-8 to INTERNAL. + Converting directly from UTF-8 to UTF-16|32 is needed, + because e.g. s390x has iconv-modules which converts directly. */ +#define RUN_UTF8_3BYTE_INPUT(b0, b1, b2, err, line) \ + buf[0] = b0; \ + buf[1] = b1; \ + buf[2] = b2; \ + fails += run_conversion ("UTF-8", "WCHAR_T", buf, 3, err, line); \ + fails += run_conversion ("UTF-8", "UTF-16LE", buf, 3, err, line); \ + fails += run_conversion ("UTF-8", "UTF-16BE", buf, 3, err, line); \ + fails += run_conversion ("UTF-8", "UTF-32LE", buf, 3, err, line); \ + fails += run_conversion ("UTF-8", "UTF-32BE", buf, 3, err, line); + + /* Use UTF-8 input of 0xD7FF. */ + RUN_UTF8_3BYTE_INPUT (0xED, 0x9F, 0xBF, 0, __LINE__); + + /* Use UTF-8 input of 0xD800. */ + RUN_UTF8_3BYTE_INPUT (0xED, 0xA0, 0x80, EILSEQ, __LINE__); + + /* Use UTF-8 input of 0xDBFF. */ + RUN_UTF8_3BYTE_INPUT (0xED, 0xAF, 0xBF, EILSEQ, __LINE__); + + /* Use UTF-8 input of 0xDC00. */ + RUN_UTF8_3BYTE_INPUT (0xED, 0xB0, 0x80, EILSEQ, __LINE__); + + /* Use UTF-8 input of 0xDFFF. */ + RUN_UTF8_3BYTE_INPUT (0xED, 0xBF, 0xBF, EILSEQ, __LINE__); + + /* Use UTF-8 input of 0xF000. */ + RUN_UTF8_3BYTE_INPUT (0xEF, 0x80, 0x80, 0, __LINE__); + + return fails > 0 ? EXIT_FAILURE : EXIT_SUCCESS; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/iconvdata/utf-16.c b/iconvdata/utf-16.c index 2d74a13..dbbcd6d 100644 --- a/iconvdata/utf-16.c +++ b/iconvdata/utf-16.c @@ -295,6 +295,12 @@ gconv_end (struct __gconv_step *data) { \ uint16_t u2; \ \ + if (__glibc_unlikely (u1 >= 0xdc00)) \ + { \ + /* This is no valid first word for a surrogate. */ \ + STANDARD_FROM_LOOP_ERR_HANDLER (2); \ + } \ + \ /* It's a surrogate character. At least the first word says \ it is. */ \ if (__glibc_unlikely (inptr + 4 > inend)) \ @@ -329,6 +335,12 @@ gconv_end (struct __gconv_step *data) } \ else \ { \ + if (__glibc_unlikely (u1 >= 0xdc00)) \ + { \ + /* This is no valid first word for a surrogate. */ \ + STANDARD_FROM_LOOP_ERR_HANDLER (2); \ + } \ + \ /* It's a surrogate character. At least the first word says \ it is. */ \ if (__glibc_unlikely (inptr + 4 > inend)) \ diff --git a/iconvdata/utf-32.c b/iconvdata/utf-32.c index 0d6fe30..25f6fc6 100644 --- a/iconvdata/utf-32.c +++ b/iconvdata/utf-32.c @@ -239,7 +239,7 @@ gconv_end (struct __gconv_step *data) if (swap) \ u1 = bswap_32 (u1); \ \ - if (__glibc_unlikely (u1 >= 0x110000)) \ + if (__glibc_unlikely (u1 >= 0x110000 || (u1 >= 0xd800 && u1 < 0xe000))) \ { \ /* This is illegal. */ \ STANDARD_FROM_LOOP_ERR_HANDLER (4); \ -- 2.3.0