Detect bad input for types xid, xid8, and cid.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 27 Dec 2022 16:40:01 +0000 (11:40 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 27 Dec 2022 16:40:01 +0000 (11:40 -0500)
Historically these input functions just called strtoul or strtoull
and returned the result, with no error detection whatever.  Upgrade
them to reject garbage input and out-of-range values, similarly to
our other numeric input routines.

To share the code for this with type oid, adjust the existing
"oidin_subr" to be agnostic about the SQL name of the type it is
handling, and move it to numutils.c; then clone it for 64-bit types.

Because the xid types previously accepted hex and octal input by
reason of calling strtoul[l] with third argument zero, I made the
common subroutine do that too, with the consequence that type oid
now also accepts hex and octal input.  In view of 6fcda9aba, that
seems like a good thing.

While at it, simplify the existing over-complicated handling of
syntax errors from strtoul: we only need one ereturn not three.

Discussion: https://postgr.es/m/3526121.1672000729@sss.pgh.pa.us

src/backend/utils/adt/numutils.c
src/backend/utils/adt/oid.c
src/backend/utils/adt/xid.c
src/include/utils/builtins.h
src/test/regress/expected/xid.out
src/test/regress/sql/xid.sql

index 7cded73e6e66f6b002cf0c0e4456f347a12d28f6..1e1a982b2fd89f8fa3dc0a2d3faed0830da56ade 100644 (file)
@@ -477,6 +477,156 @@ invalid_syntax:
                    "bigint", s)));
 }
 
+/*
+ * Convert input string to an unsigned 32 bit integer.
+ *
+ * Allows any number of leading or trailing whitespace characters.
+ *
+ * If endloc isn't NULL, store a pointer to the rest of the string there,
+ * so that caller can parse the rest.  Otherwise, it's an error if anything
+ * but whitespace follows.
+ *
+ * typname is what is reported in error messges.
+ *
+ * If escontext points to an ErrorSaveContext node, that is filled instead
+ * of throwing an error; the caller must check SOFT_ERROR_OCCURRED()
+ * to detect errors.
+ */
+uint32
+uint32in_subr(const char *s, char **endloc,
+             const char *typname, Node *escontext)
+{
+   uint32      result;
+   unsigned long cvt;
+   char       *endptr;
+
+   errno = 0;
+   cvt = strtoul(s, &endptr, 0);
+
+   /*
+    * strtoul() normally only sets ERANGE.  On some systems it may also set
+    * EINVAL, which simply means it couldn't parse the input string.  Be sure
+    * to report that the same way as the standard error indication (that
+    * endptr == s).
+    */
+   if ((errno && errno != ERANGE) || endptr == s)
+       ereturn(escontext, 0,
+               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                errmsg("invalid input syntax for type %s: \"%s\"",
+                       typname, s)));
+
+   if (errno == ERANGE)
+       ereturn(escontext, 0,
+               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                errmsg("value \"%s\" is out of range for type %s",
+                       s, typname)));
+
+   if (endloc)
+   {
+       /* caller wants to deal with rest of string */
+       *endloc = endptr;
+   }
+   else
+   {
+       /* allow only whitespace after number */
+       while (*endptr && isspace((unsigned char) *endptr))
+           endptr++;
+       if (*endptr)
+           ereturn(escontext, 0,
+                   (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                    errmsg("invalid input syntax for type %s: \"%s\"",
+                           typname, s)));
+   }
+
+   result = (uint32) cvt;
+
+   /*
+    * Cope with possibility that unsigned long is wider than uint32, in which
+    * case strtoul will not raise an error for some values that are out of
+    * the range of uint32.
+    *
+    * For backwards compatibility, we want to accept inputs that are given
+    * with a minus sign, so allow the input value if it matches after either
+    * signed or unsigned extension to long.
+    *
+    * To ensure consistent results on 32-bit and 64-bit platforms, make sure
+    * the error message is the same as if strtoul() had returned ERANGE.
+    */
+#if PG_UINT32_MAX != ULONG_MAX
+   if (cvt != (unsigned long) result &&
+       cvt != (unsigned long) ((int) result))
+       ereturn(escontext, 0,
+               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                errmsg("value \"%s\" is out of range for type %s",
+                       s, typname)));
+#endif
+
+   return result;
+}
+
+/*
+ * Convert input string to an unsigned 64 bit integer.
+ *
+ * Allows any number of leading or trailing whitespace characters.
+ *
+ * If endloc isn't NULL, store a pointer to the rest of the string there,
+ * so that caller can parse the rest.  Otherwise, it's an error if anything
+ * but whitespace follows.
+ *
+ * typname is what is reported in error messges.
+ *
+ * If escontext points to an ErrorSaveContext node, that is filled instead
+ * of throwing an error; the caller must check SOFT_ERROR_OCCURRED()
+ * to detect errors.
+ */
+uint64
+uint64in_subr(const char *s, char **endloc,
+             const char *typname, Node *escontext)
+{
+   uint64      result;
+   char       *endptr;
+
+   errno = 0;
+   result = strtou64(s, &endptr, 0);
+
+   /*
+    * strtoul[l] normally only sets ERANGE.  On some systems it may also set
+    * EINVAL, which simply means it couldn't parse the input string.  Be sure
+    * to report that the same way as the standard error indication (that
+    * endptr == s).
+    */
+   if ((errno && errno != ERANGE) || endptr == s)
+       ereturn(escontext, 0,
+               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                errmsg("invalid input syntax for type %s: \"%s\"",
+                       typname, s)));
+
+   if (errno == ERANGE)
+       ereturn(escontext, 0,
+               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                errmsg("value \"%s\" is out of range for type %s",
+                       s, typname)));
+
+   if (endloc)
+   {
+       /* caller wants to deal with rest of string */
+       *endloc = endptr;
+   }
+   else
+   {
+       /* allow only whitespace after number */
+       while (*endptr && isspace((unsigned char) *endptr))
+           endptr++;
+       if (*endptr)
+           ereturn(escontext, 0,
+                   (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                    errmsg("invalid input syntax for type %s: \"%s\"",
+                           typname, s)));
+   }
+
+   return result;
+}
+
 /*
  * pg_itoa: converts a signed 16-bit integer to its string representation
  * and returns strlen(a).
index 9d382b5cb7c151208a1a8bedf7dc61da3010c717..6b70b774d58e5de71feed90ba47387e309a6e21d 100644 (file)
  *  USER I/O ROUTINES                                                       *
  *****************************************************************************/
 
-/*
- * Parse a single OID and return its value.
- *
- * If endloc isn't NULL, store a pointer to the rest of the string there,
- * so that caller can parse the rest.  Otherwise, it's an error if anything
- * but whitespace follows.
- *
- * If escontext points to an ErrorSaveContext node, that is filled instead
- * of throwing an error; the caller must check SOFT_ERROR_OCCURRED()
- * to detect errors.
- */
-static Oid
-oidin_subr(const char *s, char **endloc, Node *escontext)
-{
-   unsigned long cvt;
-   char       *endptr;
-   Oid         result;
-
-   if (*s == '\0')
-       ereturn(escontext, InvalidOid,
-               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-                errmsg("invalid input syntax for type %s: \"%s\"",
-                       "oid", s)));
-
-   errno = 0;
-   cvt = strtoul(s, &endptr, 10);
-
-   /*
-    * strtoul() normally only sets ERANGE.  On some systems it also may set
-    * EINVAL, which simply means it couldn't parse the input string. This is
-    * handled by the second "if" consistent across platforms.
-    */
-   if (errno && errno != ERANGE && errno != EINVAL)
-       ereturn(escontext, InvalidOid,
-               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-                errmsg("invalid input syntax for type %s: \"%s\"",
-                       "oid", s)));
-
-   if (endptr == s && *s != '\0')
-       ereturn(escontext, InvalidOid,
-               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-                errmsg("invalid input syntax for type %s: \"%s\"",
-                       "oid", s)));
-
-   if (errno == ERANGE)
-       ereturn(escontext, InvalidOid,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("value \"%s\" is out of range for type %s",
-                       s, "oid")));
-
-   if (endloc)
-   {
-       /* caller wants to deal with rest of string */
-       *endloc = endptr;
-   }
-   else
-   {
-       /* allow only whitespace after number */
-       while (*endptr && isspace((unsigned char) *endptr))
-           endptr++;
-       if (*endptr)
-           ereturn(escontext, InvalidOid,
-                   (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-                    errmsg("invalid input syntax for type %s: \"%s\"",
-                           "oid", s)));
-   }
-
-   result = (Oid) cvt;
-
-   /*
-    * Cope with possibility that unsigned long is wider than Oid, in which
-    * case strtoul will not raise an error for some values that are out of
-    * the range of Oid.
-    *
-    * For backwards compatibility, we want to accept inputs that are given
-    * with a minus sign, so allow the input value if it matches after either
-    * signed or unsigned extension to long.
-    *
-    * To ensure consistent results on 32-bit and 64-bit platforms, make sure
-    * the error message is the same as if strtoul() had returned ERANGE.
-    */
-#if OID_MAX != ULONG_MAX
-   if (cvt != (unsigned long) result &&
-       cvt != (unsigned long) ((int) result))
-       ereturn(escontext, InvalidOid,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("value \"%s\" is out of range for type %s",
-                       s, "oid")));
-#endif
-
-   return result;
-}
-
 Datum
 oidin(PG_FUNCTION_ARGS)
 {
    char       *s = PG_GETARG_CSTRING(0);
    Oid         result;
 
-   result = oidin_subr(s, NULL, fcinfo->context);
+   result = uint32in_subr(s, NULL, "oid", fcinfo->context);
    PG_RETURN_OID(result);
 }
 
@@ -218,7 +125,8 @@ oidvectorin(PG_FUNCTION_ARGS)
            oidString++;
        if (*oidString == '\0')
            break;
-       result->values[n] = oidin_subr(oidString, &oidString, escontext);
+       result->values[n] = uint32in_subr(oidString, &oidString,
+                                         "oid", escontext);
        if (SOFT_ERROR_OCCURRED(escontext))
            PG_RETURN_NULL();
    }
@@ -339,7 +247,8 @@ oidparse(Node *node)
             * constants by the lexer.  Accept these if they are valid OID
             * strings.
             */
-           return oidin_subr(castNode(Float, node)->fval, NULL, NULL);
+           return uint32in_subr(castNode(Float, node)->fval, NULL,
+                                "oid", NULL);
        default:
            elog(ERROR, "unrecognized node type: %d", (int) nodeTag(node));
    }
index e4b4952a281ee0edeee9e48cb97f44be311bb300..ade60281594365bfca090fa5ad8b87690c32bdb9 100644 (file)
@@ -31,8 +31,10 @@ Datum
 xidin(PG_FUNCTION_ARGS)
 {
    char       *str = PG_GETARG_CSTRING(0);
+   TransactionId result;
 
-   PG_RETURN_TRANSACTIONID((TransactionId) strtoul(str, NULL, 0));
+   result = uint32in_subr(str, NULL, "xid", fcinfo->context);
+   PG_RETURN_TRANSACTIONID(result);
 }
 
 Datum
@@ -183,8 +185,10 @@ Datum
 xid8in(PG_FUNCTION_ARGS)
 {
    char       *str = PG_GETARG_CSTRING(0);
+   uint64      result;
 
-   PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(strtou64(str, NULL, 0)));
+   result = uint64in_subr(str, NULL, "xid8", fcinfo->context);
+   PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(result));
 }
 
 Datum
@@ -321,8 +325,10 @@ Datum
 cidin(PG_FUNCTION_ARGS)
 {
    char       *str = PG_GETARG_CSTRING(0);
+   CommandId   result;
 
-   PG_RETURN_COMMANDID((CommandId) strtoul(str, NULL, 0));
+   result = uint32in_subr(str, NULL, "cid", fcinfo->context);
+   PG_RETURN_COMMANDID(result);
 }
 
 /*
index 15373ba68f75f449a076ad8d674db25baeb20134..a4a20b5a453bb383c1ce7ff58457c355eb4250d5 100644 (file)
@@ -51,6 +51,10 @@ extern int32 pg_strtoint32(const char *s);
 extern int32 pg_strtoint32_safe(const char *s, Node *escontext);
 extern int64 pg_strtoint64(const char *s);
 extern int64 pg_strtoint64_safe(const char *s, Node *escontext);
+extern uint32 uint32in_subr(const char *s, char **endloc,
+                           const char *typname, Node *escontext);
+extern uint64 uint64in_subr(const char *s, char **endloc,
+                           const char *typname, Node *escontext);
 extern int pg_itoa(int16 i, char *a);
 extern int pg_ultoa_n(uint32 value, char *a);
 extern int pg_ulltoa_n(uint64 value, char *a);
index c7b8d299c8479fba25595bb6fb2a7dd48258400a..e62f70194340ee85e2c32b07ceb09360284192b9 100644 (file)
@@ -13,29 +13,58 @@ select '010'::xid,
    8 |  42 | 4294967295 | 4294967295 |    8 |   42 | 18446744073709551615 | 18446744073709551615
 (1 row)
 
--- garbage values are not yet rejected (perhaps they should be)
+-- garbage values
 select ''::xid;
- xid 
------
-   0
+ERROR:  invalid input syntax for type xid: ""
+LINE 1: select ''::xid;
+               ^
+select 'asdf'::xid;
+ERROR:  invalid input syntax for type xid: "asdf"
+LINE 1: select 'asdf'::xid;
+               ^
+select ''::xid8;
+ERROR:  invalid input syntax for type xid8: ""
+LINE 1: select ''::xid8;
+               ^
+select 'asdf'::xid8;
+ERROR:  invalid input syntax for type xid8: "asdf"
+LINE 1: select 'asdf'::xid8;
+               ^
+-- Also try it with non-error-throwing API
+SELECT pg_input_is_valid('42', 'xid');
+ pg_input_is_valid 
+-------------------
+ t
 (1 row)
 
-select 'asdf'::xid;
xid 
------
-   0
+SELECT pg_input_is_valid('asdf', 'xid');
pg_input_is_valid 
+-------------------
+ f
 (1 row)
 
-select ''::xid8;
- xid8 
-------
-    0
+SELECT pg_input_error_message('0xffffffffff', 'xid');
+              pg_input_error_message               
+---------------------------------------------------
+ value "0xffffffffff" is out of range for type xid
 (1 row)
 
-select 'asdf'::xid8;
- xid8 
-------
-    0
+SELECT pg_input_is_valid('42', 'xid8');
+ pg_input_is_valid 
+-------------------
+ t
+(1 row)
+
+SELECT pg_input_is_valid('asdf', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+SELECT pg_input_error_message('0xffffffffffffffffffff', 'xid8');
+                    pg_input_error_message                    
+--------------------------------------------------------------
+ value "0xffffffffffffffffffff" is out of range for type xid8
 (1 row)
 
 -- equality
index 2289803681b5c9266190e0e00a52c0c4eb3abb68..b6996588ef641dd6452e70f1c4a9594a8fb9e004 100644 (file)
@@ -10,12 +10,20 @@ select '010'::xid,
       '0xffffffffffffffff'::xid8,
       '-1'::xid8;
 
--- garbage values are not yet rejected (perhaps they should be)
+-- garbage values
 select ''::xid;
 select 'asdf'::xid;
 select ''::xid8;
 select 'asdf'::xid8;
 
+-- Also try it with non-error-throwing API
+SELECT pg_input_is_valid('42', 'xid');
+SELECT pg_input_is_valid('asdf', 'xid');
+SELECT pg_input_error_message('0xffffffffff', 'xid');
+SELECT pg_input_is_valid('42', 'xid8');
+SELECT pg_input_is_valid('asdf', 'xid8');
+SELECT pg_input_error_message('0xffffffffffffffffffff', 'xid8');
+
 -- equality
 select '1'::xid = '1'::xid;
 select '1'::xid != '1'::xid;