On Wed, Mar 18, 2026 at 06:34:03PM +0700, John Naylor wrote: > Seems sensible to me. I only have minor nitpicks:
Thanks for the review of the module. > +operation for a single byte as well as a range of these, acting as thin > +wrappers standing on top of pg_saslprep(). > > It's more natural to say "wrappers around", at least that's what comes to me. Fixed. > + if (unlikely(utf8_len == 0)) > > The exceptional path only has two lines of code, so it's unclear what > this hint is trying to do. This module isn't run by default anyway Removed that. > + MemSet(nulls, false, sizeof(nulls)); > > Regular "memset" with a 4-byte constant input is easily inline-able by > the compiler, and I think we should use our homegrown implementation > only when there is a specific reason for it. (I know there are many > dozens of uses without a reason already, but...) Removed that. > -is($result, 'U+0000|SUCCESS|\x00|\x', "Only nul authorized for all > valid UTF8 codepoints"); > +is($result, '', "No empty or NULL values for all valid UTF8 codepoints"); > > I don't quite understand "only nul authorized..." -- I understand the > explanation in your email, but I having difficulty with the way it's > phrased here. (Although it'll be moot if we go ahead with 0002) Yes, still better to keep the state of the tree cleaner at all times, especially if 0002 gets reverted. I have used a simpler "valid codepoints returning an empty password". Applied the result for the module, to have at least the coverage part. The last piece is refreshed, and attached for now. -- Michael
From a806e48445ee7d9d75dbe70e0da76b703650faa4 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Thu, 19 Mar 2026 13:18:28 +0900 Subject: [PATCH v3] Make implementation of SASLprep compliant for ASCII characters --- src/common/saslprep.c | 12 ---- .../test_saslprep/expected/test_saslprep.out | 66 +++++++++---------- .../test_saslprep/t/001_saslprep_ranges.pl | 4 +- 3 files changed, 34 insertions(+), 48 deletions(-) diff --git a/src/common/saslprep.c b/src/common/saslprep.c index 2ad2cefc14fb..38d50dd823c4 100644 --- a/src/common/saslprep.c +++ b/src/common/saslprep.c @@ -1061,18 +1061,6 @@ pg_saslprep(const char *input, char **output) /* Ensure we return *output as NULL on failure */ *output = NULL; - /* - * Quick check if the input is pure ASCII. An ASCII string requires no - * further processing. - */ - if (pg_is_ascii(input)) - { - *output = STRDUP(input); - if (!(*output)) - goto oom; - return SASLPREP_SUCCESS; - } - /* * Convert the input from UTF-8 to an array of Unicode codepoints. * diff --git a/src/test/modules/test_saslprep/expected/test_saslprep.out b/src/test/modules/test_saslprep/expected/test_saslprep.out index f72dbffa0a11..92f93365343e 100644 --- a/src/test/modules/test_saslprep/expected/test_saslprep.out +++ b/src/test/modules/test_saslprep/expected/test_saslprep.out @@ -19,38 +19,38 @@ SELECT FROM generate_series(0,127) AS a; dat | byt | saslprep ----------+------+------------------- - <NUL> | \x00 | ("\\x",SUCCESS) - <CTL_1> | \x01 | ("\\x01",SUCCESS) - <CTL_2> | \x02 | ("\\x02",SUCCESS) - <CTL_3> | \x03 | ("\\x03",SUCCESS) - <CTL_4> | \x04 | ("\\x04",SUCCESS) - <CTL_5> | \x05 | ("\\x05",SUCCESS) - <CTL_6> | \x06 | ("\\x06",SUCCESS) - <CTL_7> | \x07 | ("\\x07",SUCCESS) - <CTL_8> | \x08 | ("\\x08",SUCCESS) - <CTL_9> | \x09 | ("\\x09",SUCCESS) - <CTL_10> | \x0a | ("\\x0a",SUCCESS) - <CTL_11> | \x0b | ("\\x0b",SUCCESS) - <CTL_12> | \x0c | ("\\x0c",SUCCESS) - <CTL_13> | \x0d | ("\\x0d",SUCCESS) - <CTL_14> | \x0e | ("\\x0e",SUCCESS) - <CTL_15> | \x0f | ("\\x0f",SUCCESS) - <CTL_16> | \x10 | ("\\x10",SUCCESS) - <CTL_17> | \x11 | ("\\x11",SUCCESS) - <CTL_18> | \x12 | ("\\x12",SUCCESS) - <CTL_19> | \x13 | ("\\x13",SUCCESS) - <CTL_20> | \x14 | ("\\x14",SUCCESS) - <CTL_21> | \x15 | ("\\x15",SUCCESS) - <CTL_22> | \x16 | ("\\x16",SUCCESS) - <CTL_23> | \x17 | ("\\x17",SUCCESS) - <CTL_24> | \x18 | ("\\x18",SUCCESS) - <CTL_25> | \x19 | ("\\x19",SUCCESS) - <CTL_26> | \x1a | ("\\x1a",SUCCESS) - <CTL_27> | \x1b | ("\\x1b",SUCCESS) - <CTL_28> | \x1c | ("\\x1c",SUCCESS) - <CTL_29> | \x1d | ("\\x1d",SUCCESS) - <CTL_30> | \x1e | ("\\x1e",SUCCESS) - <CTL_31> | \x1f | ("\\x1f",SUCCESS) + <NUL> | \x00 | (,PROHIBITED) + <CTL_1> | \x01 | (,PROHIBITED) + <CTL_2> | \x02 | (,PROHIBITED) + <CTL_3> | \x03 | (,PROHIBITED) + <CTL_4> | \x04 | (,PROHIBITED) + <CTL_5> | \x05 | (,PROHIBITED) + <CTL_6> | \x06 | (,PROHIBITED) + <CTL_7> | \x07 | (,PROHIBITED) + <CTL_8> | \x08 | (,PROHIBITED) + <CTL_9> | \x09 | (,PROHIBITED) + <CTL_10> | \x0a | (,PROHIBITED) + <CTL_11> | \x0b | (,PROHIBITED) + <CTL_12> | \x0c | (,PROHIBITED) + <CTL_13> | \x0d | (,PROHIBITED) + <CTL_14> | \x0e | (,PROHIBITED) + <CTL_15> | \x0f | (,PROHIBITED) + <CTL_16> | \x10 | (,PROHIBITED) + <CTL_17> | \x11 | (,PROHIBITED) + <CTL_18> | \x12 | (,PROHIBITED) + <CTL_19> | \x13 | (,PROHIBITED) + <CTL_20> | \x14 | (,PROHIBITED) + <CTL_21> | \x15 | (,PROHIBITED) + <CTL_22> | \x16 | (,PROHIBITED) + <CTL_23> | \x17 | (,PROHIBITED) + <CTL_24> | \x18 | (,PROHIBITED) + <CTL_25> | \x19 | (,PROHIBITED) + <CTL_26> | \x1a | (,PROHIBITED) + <CTL_27> | \x1b | (,PROHIBITED) + <CTL_28> | \x1c | (,PROHIBITED) + <CTL_29> | \x1d | (,PROHIBITED) + <CTL_30> | \x1e | (,PROHIBITED) + <CTL_31> | \x1f | (,PROHIBITED) | \x20 | ("\\x20",SUCCESS) ! | \x21 | ("\\x21",SUCCESS) " | \x22 | ("\\x22",SUCCESS) @@ -146,7 +146,7 @@ SELECT | | \x7c | ("\\x7c",SUCCESS) } | \x7d | ("\\x7d",SUCCESS) ~ | \x7e | ("\\x7e",SUCCESS) - <DEL> | \x7f | ("\\x7f",SUCCESS) + <DEL> | \x7f | (,PROHIBITED) (128 rows) DROP EXTENSION test_saslprep; diff --git a/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl b/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl index b353017c0651..4a7cb5aaa588 100644 --- a/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl +++ b/src/test/modules/test_saslprep/t/001_saslprep_ranges.pl @@ -25,14 +25,12 @@ $node->safe_psql('postgres', 'CREATE EXTENSION test_saslprep;'); # Among all the valid UTF-8 codepoint ranges, our implementation of # SASLprep should never return an empty password if the operation is # considered a success. -# The only exception is currently the nul character, prohibited in -# input of CREATE/ALTER ROLE. my $result = $node->safe_psql( 'postgres', qq[SELECT * FROM test_saslprep_ranges() WHERE status = 'SUCCESS' AND res IN (NULL, '') ]); -is($result, 'U+0000|SUCCESS|\x00|\x', "valid codepoints returning an empty password"); +is($result, '', "valid codepoints returning an empty password"); $node->stop; done_testing(); -- 2.53.0
signature.asc
Description: PGP signature
