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: Tue, 22 Mar 2016 15:38:16 +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> <namfnd$mrp$2 at ger dot gmane dot org> <56EBFCED dot 3070003 at linux dot vnet dot ibm dot com>
On 03/18/2016 02:04 PM, Stefan Liebler wrote:
Is the previously attached test-case okay?
Hi,
here is one further update regarding the new test "iconv/tst-iconv7.c".
If "make check" is executed directly after building without the
installation step, then <build-dir>/iconvdata/gconv-modules hasn't been
generated yet and the system gconv-modules-file/iconv-modules are used
instead - if available.
The test needs gconv-modules and the UTF-16|32 modules as prerequirement.
Thus I've moved it to iconvdata/bug-iconv12.c and added
the prerequirements in iconvdata/Makefile.
Ok, to commit?
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.
* iconvdata/bug-iconv12.c: New file.
* iconvdata/Makefile (tests): Add bug-iconv12.
>From d3b5f8fe14a719dbf13e83ee3946395c73c55766 Mon Sep 17 00:00:00 2001
From: Stefan Liebler <stli@linux.vnet.ibm.com>
Date: Tue, 23 Feb 2016 09:27:46 +0100
Subject: [PATCH 14/14] Fix UTF-16 surrogate handling. [BZ #19727]
According to the latest Unicode standard, a conversion from/to UTF-xx has
to report an error if the character value is in range of an utf16 surrogate
(0xd800..0xdfff). See https://sourceware.org/ml/libc-help/2015-12/msg00015.html.
Thus this patch fixes this behaviour for converting from utf32 to internal and
from internal to utf8.
Furthermore the conversion from utf16 to internal does not report an error if the
input-stream consists of two low-surrogate values. If an uint16_t value is in the
range of 0xd800 .. 0xdfff, the next uint16_t value is checked, if it is in the
range of a low surrogate (0xdc00 .. 0xdfff). Afterwards these two uint16_t
values are interpreted as a high- and low-surrogates pair. But there is no test
if the first uint16_t value is really in the range of a high-surrogate
(0xd800 .. 0xdbff). If there would be two uint16_t values in the range of a low
surrogate, then they will be treated as a valid high- and low-surrogates pair.
This patch adds this test.
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.
* iconvdata/bug-iconv12.c: New file.
* iconvdata/Makefile (tests): Add bug-iconv12.
rename test
---
iconv/gconv_simple.c | 3 +-
iconvdata/Makefile | 4 +-
iconvdata/bug-iconv12.c | 263 ++++++++++++++++++++++++++++++++++++++++++++++++
iconvdata/utf-16.c | 12 +++
iconvdata/utf-32.c | 2 +-
5 files changed, 281 insertions(+), 3 deletions(-)
create mode 100644 iconvdata/bug-iconv12.c
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/iconvdata/Makefile b/iconvdata/Makefile
index 1ac1a5c..8e59dd6 100644
--- a/iconvdata/Makefile
+++ b/iconvdata/Makefile
@@ -68,7 +68,7 @@ modules.so := $(addsuffix .so, $(modules))
ifeq (yes,$(build-shared))
tests = bug-iconv1 bug-iconv2 tst-loading tst-e2big tst-iconv4 bug-iconv4 \
tst-iconv6 bug-iconv5 bug-iconv6 tst-iconv7 bug-iconv8 bug-iconv9 \
- bug-iconv10 bug-iconv11
+ bug-iconv10 bug-iconv11 bug-iconv12
ifeq ($(have-thread-library),yes)
tests += bug-iconv3
endif
@@ -309,6 +309,8 @@ $(objpfx)tst-iconv7.out: $(objpfx)gconv-modules \
$(addprefix $(objpfx),$(modules.so))
$(objpfx)bug-iconv10.out: $(objpfx)gconv-modules \
$(addprefix $(objpfx),$(modules.so))
+$(objpfx)bug-iconv12.out: $(objpfx)gconv-modules \
+ $(addprefix $(objpfx),$(modules.so))
$(objpfx)iconv-test.out: run-iconv-test.sh $(objpfx)gconv-modules \
$(addprefix $(objpfx),$(modules.so)) \
diff --git a/iconvdata/bug-iconv12.c b/iconvdata/bug-iconv12.c
new file mode 100644
index 0000000..8c748e8
--- /dev/null
+++ b/iconvdata/bug-iconv12.c
@@ -0,0 +1,263 @@
+/* bug 19727: 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