From 58ff4a0a08679356e502dfcbbc50e9bec94b77b1 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 6 Aug 2026 15:39:56 +0900 Subject: [PATCH] Tighten tid input parsing with strtoul() to reject empty fields strtoul() informs that in the case of no conversion, "endptr" is set equal to the location of the input pointer, which is something that tidin() has never checked. This allowed inputs like "(,)", "(1,)" or "(,1)" to be accepted on some platforms, like Linux where "(1,)" implied "(1,0)", but rejected on others like macos where "(1,)" is rejected, due to the fact that errno may not be set. This change may cause some issues as some tid values will now be rejected, but at least we get the consistent strtoul() experience. If it proves to be a pain for some, it could always be reverted. Another point of this change is that we get one step closer to a possible common strtoul() where the error handling could be unified in core, with all the errors handled the same way for errno and no input parsed, at least. Reported-by: Michael Malis Author: Tristan Partin Discussion: https://postgr.es/m/19584-e60c446ba6f57c9c@postgresql.org Discussion: https://postgr.es/m/DKBS2Z9CGARC.2T07O6TJYSE8B@partin.io --- src/backend/utils/adt/tid.c | 5 ++--- src/test/regress/expected/tid.out | 8 ++++++++ src/test/regress/sql/tid.sql | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c index 9257886f1da..a97873f91ba 100644 --- a/src/backend/utils/adt/tid.c +++ b/src/backend/utils/adt/tid.c @@ -73,7 +73,7 @@ tidin(PG_FUNCTION_ARGS) errno = 0; cvt = strtoul(coord[0], &badp, 10); - if (errno || *badp != DELIM) + if (badp == coord[0] || errno || *badp != DELIM) ereturn(escontext, (Datum) 0, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", @@ -95,8 +95,7 @@ tidin(PG_FUNCTION_ARGS) #endif cvt = strtoul(coord[1], &badp, 10); - if (errno || *badp != RDELIM || - cvt > USHRT_MAX) + if (badp == coord[1] || errno || *badp != RDELIM || cvt > USHRT_MAX) ereturn(escontext, (Datum) 0, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", diff --git a/src/test/regress/expected/tid.out b/src/test/regress/expected/tid.out index 3497a77688b..45cc61fb1ca 100644 --- a/src/test/regress/expected/tid.out +++ b/src/test/regress/expected/tid.out @@ -17,6 +17,14 @@ SELECT '(1,65536)'::tid; -- error ERROR: invalid input syntax for type tid: "(1,65536)" LINE 1: SELECT '(1,65536)'::tid; ^ +SELECT '(,1)'::tid; -- error +ERROR: invalid input syntax for type tid: "(,1)" +LINE 1: SELECT '(,1)'::tid; + ^ +SELECT '(1,)'::tid; -- error +ERROR: invalid input syntax for type tid: "(1,)" +LINE 1: SELECT '(1,)'::tid; + ^ -- Also try it with non-error-throwing API SELECT pg_input_is_valid('(0)', 'tid'); pg_input_is_valid diff --git a/src/test/regress/sql/tid.sql b/src/test/regress/sql/tid.sql index c0a70be5cbd..51d00b92074 100644 --- a/src/test/regress/sql/tid.sql +++ b/src/test/regress/sql/tid.sql @@ -8,6 +8,8 @@ SELECT SELECT '(4294967296,1)'::tid; -- error SELECT '(1,65536)'::tid; -- error +SELECT '(,1)'::tid; -- error +SELECT '(1,)'::tid; -- error -- Also try it with non-error-throwing API SELECT pg_input_is_valid('(0)', 'tid');