pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Stop asserting that Unicode normalization consumes its whole input
3+ messages / 2 participants
[nested] [flat]

* Stop asserting that Unicode normalization consumes its whole input
@ 2026-09-18 17:00 Tristan Partin <tristan@partin.io>
  2026-09-22 04:45 ` Re: Stop asserting that Unicode normalization consumes its whole input John Naylor <johncnaylorls@gmail.com>
  0 siblings, 1 reply; 3+ messages in thread

From: Tristan Partin @ 2026-09-18 17:00 UTC (permalink / raw)
  To: pgsql-hackers <pgsql-hackers@lists.postgresql.org>

normalize() and IS NORMALIZED determine how many code points their input
holds with pg_mbstrlen_with_len(). They then decode exactly that many
code points, and assert afterward that this consumed the entire datum.
That invariant does not hold because pg_mbstrlen_with_len() stops at the
first of "limit" or a NUL.

Postgres doesn't allow text types to contain NUL, so reaching this
requires work, but src/test/regress/sql/encoding.sql already constructs
such values to pin down how the string functions behave on them. It just
never passed them to normalize() or IS NORMALIZED, which is why this
went unnoticed.

-- 
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

Attachments:

  [text/x-patch] v1-0001-Stop-asserting-that-Unicode-normalization-consume.patch (3.4K, ../../DLILPXVKR2GW.1KP2DGOPCP7P2@partin.io/2-v1-0001-Stop-asserting-that-Unicode-normalization-consume.patch)
  download | inline diff:
From d7a74c803a42a9df143d7eb56361aa8ec6f3b951 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@partin.io>
Date: Fri, 11 Sep 2026 05:37:13 +0000
Subject: [PATCH v1] Stop asserting that Unicode normalization consumes its
 whole input

normalize() and IS NORMALIZED determine how many code points their input
holds with pg_mbstrlen_with_len(). They then decode exactly that many
code points, and assert afterward that this consumed the entire datum.
That invariant does not hold because pg_mbstrlen_with_len() stops at the
first of "limit" or a NUL.

Postgres doesn't allow text types to contain NUL, so reaching this
requires work, but src/test/regress/sql/encoding.sql already constructs
such values to pin down how the string functions behave on them. It just
never passed them to normalize() or IS NORMALIZED, which is why this
went unnoticed.

Author: Tristan Partin <tristan@partin.io>
Signed-off-by: Tristan Partin <tristan@partin.io>
---
 src/backend/utils/adt/varlena.c        |  4 ++--
 src/test/regress/expected/encoding.out | 12 ++++++++++++
 src/test/regress/sql/encoding.sql      |  2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index f6a41e709ae..962b7857b7d 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -5537,7 +5537,7 @@ unicode_normalize_func(PG_FUNCTION_ARGS)
 		p += pg_utf_mblen(p);
 	}
 	input_chars[i] = (char32_t) '\0';
-	Assert((char *) p == VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input));
+	Assert((char *) p <= VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input));
 
 	/* action */
 	output_chars = unicode_normalize(form, input_chars);
@@ -5605,7 +5605,7 @@ unicode_is_normalized(PG_FUNCTION_ARGS)
 		p += pg_utf_mblen(p);
 	}
 	input_chars[i] = (char32_t) '\0';
-	Assert((char *) p == VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input));
+	Assert((char *) p <= VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input));
 
 	/* quick check (see UAX #15) */
 	quickcheck = unicode_is_normalized_quickcheck(form, input_chars);
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index 0bb72a1df6f..5c06d6daae4 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -120,6 +120,18 @@ SELECT regexp_replace(with_nul, '^caf(.)$', '\1') FROM regress_encoding;
  é
 (1 row)
 
+SELECT octet_length(normalize(with_nul)) FROM regress_encoding;
+ octet_length 
+--------------
+            5
+(1 row)
+
+SELECT with_nul IS NORMALIZED FROM regress_encoding;
+ is_normalized 
+---------------
+ t
+(1 row)
+
 -- NUL = character
 SELECT with_nul, reverse(with_nul), reverse(reverse(with_nul)) FROM regress_encoding;
  with_nul | reverse | reverse 
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index 26caa93a5d5..d2a649acaa2 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -57,6 +57,8 @@ SELECT substring(with_nul, 4, 1) FROM regress_encoding;
 SELECT substring(with_nul, 5, 1) FROM regress_encoding;
 SELECT convert_to(substring(with_nul, 5, 1), 'UTF8') FROM regress_encoding;
 SELECT regexp_replace(with_nul, '^caf(.)$', '\1') FROM regress_encoding;
+SELECT octet_length(normalize(with_nul)) FROM regress_encoding;
+SELECT with_nul IS NORMALIZED FROM regress_encoding;
 -- NUL = character
 SELECT with_nul, reverse(with_nul), reverse(reverse(with_nul)) FROM regress_encoding;
 
-- 
Tristan Partin
https://tristan.partin.io



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Stop asserting that Unicode normalization consumes its whole input
  2026-09-18 17:00 Stop asserting that Unicode normalization consumes its whole input Tristan Partin <tristan@partin.io>
@ 2026-09-22 04:45 ` John Naylor <johncnaylorls@gmail.com>
  2026-09-22 05:13   ` Re: Stop asserting that Unicode normalization consumes its whole input Tristan Partin <tristan@partin.io>
  0 siblings, 1 reply; 3+ messages in thread

From: John Naylor @ 2026-09-22 04:45 UTC (permalink / raw)
  To: Tristan Partin <tristan@partin.io>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>

On Sat, Sep 19, 2026 at 12:00 AM Tristan Partin <tristan@partin.io> wrote:
>
> normalize() and IS NORMALIZED determine how many code points their input
> holds with pg_mbstrlen_with_len(). They then decode exactly that many
> code points, and assert afterward that this consumed the entire datum.
> That invariant does not hold because pg_mbstrlen_with_len() stops at the
> first of "limit" or a NUL.

The limit here is VARSIZE_ANY_EXHDR(input) , which is the entire
length. This patch seems like it's weakening an assertion for a case
that shouldn't happen. (I haven't looked at the tests in encoding.sql
that force non-standard behavior, so I'm not sure what the context was
there...)

-- 
John Naylor
Amazon Web Services






^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Stop asserting that Unicode normalization consumes its whole input
  2026-09-18 17:00 Stop asserting that Unicode normalization consumes its whole input Tristan Partin <tristan@partin.io>
  2026-09-22 04:45 ` Re: Stop asserting that Unicode normalization consumes its whole input John Naylor <johncnaylorls@gmail.com>
@ 2026-09-22 05:13   ` Tristan Partin <tristan@partin.io>
  0 siblings, 0 replies; 3+ messages in thread

From: Tristan Partin @ 2026-09-22 05:13 UTC (permalink / raw)
  To: John Naylor <johncnaylorls@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>

On Mon Sep 21, 2026 at 11:46 PM CDT, John Naylor wrote:
> On Sat, Sep 19, 2026 at 12:00 AM Tristan Partin <tristan@partin.io> wrote:
>>
>> normalize() and IS NORMALIZED determine how many code points their input
>> holds with pg_mbstrlen_with_len(). They then decode exactly that many
>> code points, and assert afterward that this consumed the entire datum.
>> That invariant does not hold because pg_mbstrlen_with_len() stops at the
>> first of "limit" or a NUL.
>
> The limit here is VARSIZE_ANY_EXHDR(input) , which is the entire
> length. This patch seems like it's weakening an assertion for a case
> that shouldn't happen. (I haven't looked at the tests in encoding.sql
> that force non-standard behavior, so I'm not sure what the context was
> there...)

Hey John,

In regress.c, we have the following function:

	/* Convert bytea to text without validation for corruption tests from SQL. */
	PG_FUNCTION_INFO_V1(test_bytea_to_text);
	Datum
	test_bytea_to_text(PG_FUNCTION_ARGS)
	{
	        PG_RETURN_TEXT_P(PG_GETARG_BYTEA_PP(0));
	}

In encoding.sql, we do things like the following:

	CREATE TABLE regress_encoding(good text, truncated text, with_nul text, truncated_with_nul text);
	INSERT INTO regress_encoding
	VALUES ('café',
	        'caf' || test_bytea_to_text('\xc3'),
	        'café' || test_bytea_to_text('\x00') || 'dcba',
	        'caf' || test_bytea_to_text('\xc300') || 'dcba');
	...
	-- NUL = terminator
	SELECT length(with_nul) FROM regress_encoding;
	SELECT substring(with_nul, 3, 1) FROM regress_encoding;
	SELECT substring(with_nul, 4, 1) FROM regress_encoding;
	SELECT substring(with_nul, 5, 1) FROM regress_encoding;
	SELECT convert_to(substring(with_nul, 5, 1), 'UTF8') FROM regress_encoding;
	SELECT regexp_replace(with_nul, '^caf(.)$', '\1') FROM regress_encoding;
	-- NUL = character
	SELECT with_nul, reverse(with_nul), reverse(reverse(with_nul)) FROM regress_encoding;

Output file:

	SELECT length(with_nul) FROM regress_encoding;
	 length
	--------
	      4
	(1 row)
	
	SELECT substring(with_nul, 3, 1) FROM regress_encoding;
	 substring
	-----------
	 f
	(1 row)
	
	SELECT substring(with_nul, 4, 1) FROM regress_encoding;
	 substring
	-----------
	 é
	(1 row)
	
	SELECT substring(with_nul, 5, 1) FROM regress_encoding;
	 substring
	-----------
	
	(1 row)
	
	SELECT convert_to(substring(with_nul, 5, 1), 'UTF8') FROM regress_encoding;
	 convert_to
	------------
	 \x
	(1 row)
	
	SELECT regexp_replace(with_nul, '^caf(.)$', '\1') FROM regress_encoding;
	 regexp_replace
	----------------
	 é
	(1 row)
	
	-- NUL = character
	SELECT with_nul, reverse(with_nul), reverse(reverse(with_nul)) FROM regress_encoding;
	 with_nul | reverse | reverse
	----------+---------+---------
	 café     | abcd    | café
	(1 row)

You can see that we are doing some shenanigans to test how text values
work in various string functions for corrupted values. We don't do the
same tests for normalize() and IS NORMALIZED, which is why I made the
change to <= instead of ==. Otherwise, builds with casserts enabled will
crash, which doesn't seem like what we intend to do given the array of
tests we do to make sure things still work.

I would say that you're right that this patch does weaken an assertion,
but it is something that Postgres tests for just to make sure things
continue to work.

-- 
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)






^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2026-09-22 05:13 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-18 17:00 Stop asserting that Unicode normalization consumes its whole input Tristan Partin <tristan@partin.io>
2026-09-22 04:45 ` John Naylor <johncnaylorls@gmail.com>
2026-09-22 05:13   ` Tristan Partin <tristan@partin.io>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox