Re: [PATCH v3 3/3] iconvdata: Test case for bug 34556, bug 34568

On 8/27/26 7:58 AM, Florian Weimer wrote:
> Assisted-by: LLM

LGTM.

Reviewed-by: Carlos O'Donell <[email protected]>

> ---
> v3: Split into separate bugs with distinct CVE IDs, as requested.
>   iconvdata/Makefile                |   5 +-
>   iconvdata/tst-jisx0213-progress.c | 124 ++++++++++++++++++++++++++++++
>   2 files changed, 128 insertions(+), 1 deletion(-)
>   create mode 100644 iconvdata/tst-jisx0213-progress.c
> 
> diff --git a/iconvdata/Makefile b/iconvdata/Makefile
> index fbb0067302..ba2eec2b48 100644
> --- a/iconvdata/Makefile
> +++ b/iconvdata/Makefile
> @@ -76,7 +76,8 @@ 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-iconv12 tst-iconv-big5-hkscs-to-2ucs4 \
>   	bug-iconv13 bug-iconv14 bug-iconv15 \
> -	tst-iconv-iso-2022-cn-ext tst-bug33980
> +	tst-iconv-iso-2022-cn-ext tst-bug33980 \
> +	tst-jisx0213-progress

OK. Adds test.

>   ifeq ($(have-thread-library),yes)
>   tests += bug-iconv3
>   endif
> @@ -335,6 +336,8 @@ $(objpfx)tst-iconv-iso-2022-cn-ext.out: $(addprefix $(objpfx), $(gconv-modules))
>   					$(addprefix $(objpfx),$(modules.so))
>   $(objpfx)tst-bug33980.out: $(addprefix $(objpfx), $(gconv-modules)) \
>   			   $(addprefix $(objpfx),$(modules.so))
> +$(objpfx)tst-jisx0213-progress.out: \
> +  $(addprefix $(objpfx), $(gconv-modules)) $(addprefix $(objpfx),$(modules.so))

OK. Depends on gconv modules.

>   
>   $(objpfx)iconv-test.out: run-iconv-test.sh \
>   			 $(addprefix $(objpfx), $(gconv-modules)) \
> diff --git a/iconvdata/tst-jisx0213-progress.c b/iconvdata/tst-jisx0213-progress.c
> new file mode 100644
> index 0000000000..7b2073be1f
> --- /dev/null
> +++ b/iconvdata/tst-jisx0213-progress.c
> @@ -0,0 +1,124 @@
> +/* Test JISX0213 combining character conversion progress (bug 34556, bug 34568).

OK. Both bugs.

> +   Copyright (C) 2026 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* Certain JISX0213 byte sequences map to a combining sequence, for
> +   example U+304B (HIRAGANA LETTER KA) followed by U+309A (COMBINING
> +   SEMI-VOICED SOUND MARK).  When converting to internal encoding
> +   (actually UTF-32) with a small output buffer, the first code point
> +   is emitted and the second is queued in the converter state.  This
> +   test verifies that the queued code point is consumed exactly once
> +   on retry, so that the conversion makes progress and terminates.  */

OK. Agreed.

> +
> +#include <errno.h>
> +#include <iconv.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <support/check.h>
> +#include <support/support.h>
> +
> +static void
> +test_one (const char *charset, const char *input, size_t outbufsize)
> +{
> +  printf ("info: %s: testing output buffer size %zu\n", charset, outbufsize);
> +
> +  /* Expected UTF-32 output.  */
> +  static const wchar_t expected[] = { 0x304b, 0x309a, 'A' };

OK. Sequence with A at the end.

> +
> +  /* Use WCHAR_T encoding to avoid the BOM.  */
> +  iconv_t cd = iconv_open ("WCHAR_T", charset);
> +  TEST_VERIFY_EXIT (cd != (iconv_t) -1);
> +
> +  char result[64];
> +  size_t result_len = 0;
> +
> +  char *inptr = (char *) input;
> +  size_t inleft = strlen (input);
> +
> +  char outbuf[64];
> +
> +  int iterations = 0;
> +  while (inleft > 0)

OK. Keep going while we have input to process.

> +    {
> +      char *outptr = outbuf;
> +      size_t outleft = outbufsize;
> +      size_t inleft_before = inleft;
> +
> +      size_t ret = iconv (cd, &inptr, &inleft, &outptr, &outleft);
> +      size_t produced = outptr - outbuf;
> +
> +      TEST_VERIFY_EXIT (result_len + produced <= sizeof (result));
> +      memcpy (result + result_len, outbuf, produced);
> +      result_len += produced;
> +
> +      if (ret == (size_t) -1 && errno == E2BIG)
> +	{
> +	  if (produced == 0 && inleft == inleft_before)
> +	    {
> +	      /* Output buffer too small for a single code point.  */
> +	      TEST_VERIFY_EXIT (outbufsize < 4);
> +	      break;
> +	    }
> +	  /* Bound iterations to detect non-progress bugs.  */
> +	  if (++iterations < 10)
> +	    continue;
> +	  else
> +	    {
> +	      FAIL ("%s: no progress", charset);
> +	      goto out;
> +	    }
> +	}
> +      if (ret == (size_t) -1)
> +	FAIL_EXIT1 ("outbufsize %zu: iconv: %m", outbufsize);
> +      break;
> +    }
> +
> +  /* Flush pending converter state.  */
> +  {
> +    char *outptr = outbuf;
> +    size_t outleft = outbufsize;
> +    size_t ret = iconv (cd, NULL, NULL, &outptr, &outleft);
> +    TEST_VERIFY (ret == 0);
> +    size_t produced = outptr - outbuf;
> +    memcpy (result + result_len, outbuf, produced);
> +    result_len += produced;
> +  }
> +
> +  if (outbufsize >= 4)
> +    {
> +      TEST_COMPARE (inleft, 0);
> +      TEST_COMPARE_BLOB (result, result_len,
> +			 expected, sizeof (expected));

OK. Make sure we got back exactly what we expected.

> +    }
> +
> + out:
> +  TEST_VERIFY_EXIT (iconv_close (cd) == 0);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  for (size_t outbufsize = 1; outbufsize <= 16; outbufsize++)

OK. Iterate buffer size.

> +    {
> +      test_one ("EUC-JISX0213", "\244\367A", outbufsize);
> +      test_one ("SHIFT_JISX0213", "\202\365A", outbufsize);

OK. Test both inputs that produce the same outputs.

> +    }
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>


-- 
Cheers,
Carlos.