This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 14/14] Fix UTF-16 surrogate handling.
- From: Stefan Liebler <stli at linux dot vnet dot ibm dot com>
- To: libc-alpha at sourceware dot org
- Date: Thu, 25 Feb 2016 09:58:21 +0100
- Subject: Re: [PATCH 14/14] Fix UTF-16 surrogate handling.
- Authentication-results: sourceware.org; auth=none
- References: <1456219278-5258-1-git-send-email-stli at linux dot vnet dot ibm dot com> <1456219278-5258-15-git-send-email-stli at linux dot vnet dot ibm dot com> <alpine dot DEB dot 2 dot 10 dot 1602231742030 dot 17168 at digraph dot polyomino dot org dot uk>
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
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <inttypes.h>
+#include <iconv.h>
+#include <byteswap.h>
+
+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