Replace some strtok() with strsep()

strtok() considers adjacent delimiters to be one delimiter, which is
arguably the wrong behavior in some cases.  Replace with strsep(),
which has the right behavior: Adjacent delimiters create an empty
token.

Affected by this are parsing of:

- Stored SCRAM secrets
  ("SCRAM-SHA-256$:$:")

- ICU collation attributes
  ("und@colStrength=primary;colCaseLevel=yes") for ICU older than
  version 54

- PG_COLORS environment variable
  ("error=01;31:warning=01;35:note=01;36:locus=01")

- pg_regress command-line options with comma-separated list arguments
  (--dbname, --create-role) (currently only used pg_regress_ecpg)

Reviewed-by: Kyotaro Horiguchi 
Reviewed-by: David Steele 
Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org

diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 416195991486de55f76db1bd3c1fe5d367a1cd29..03ddddc3c27ab4dfdc45a2c17eb7ece877649648 100644 (file)
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -608,16 +608,15 @@ parse_scram_secret(const char *secret, int *iterations,
     * SCRAM-SHA-256$:$:
     */
    v = pstrdup(secret);
-   if ((scheme_str = strtok(v, "$")) == NULL)
+   if ((scheme_str = strsep(&v, "$")) == NULL)
        goto invalid_secret;
-   if ((iterations_str = strtok(NULL, ":")) == NULL)
+   if ((iterations_str = strsep(&v, ":")) == NULL)
        goto invalid_secret;
-   if ((salt_str = strtok(NULL, "$")) == NULL)
+   if ((salt_str = strsep(&v, "$")) == NULL)
        goto invalid_secret;
-   if ((storedkey_str = strtok(NULL, ":")) == NULL)
-       goto invalid_secret;
-   if ((serverkey_str = strtok(NULL, "")) == NULL)
+   if ((storedkey_str = strsep(&v, ":")) == NULL)
        goto invalid_secret;
+   serverkey_str = v;
 
    /* Parse the fields */
    if (strcmp(scheme_str, "SCRAM-SHA-256") != 0)

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 2673bafe60af24eee34c44ccb8f8d4b30cd0c7f1..38c40a40489767cbc5682e1b2ccbad00d5067ecf 100644 (file)
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -2813,6 +2813,7 @@ icu_set_collation_attributes(UCollator *collator, const char *loc,
    char       *icu_locale_id;
    char       *lower_str;
    char       *str;
+   char       *token;
 
    /*
     * The input locale may be a BCP 47 language tag, e.g.
@@ -2838,7 +2839,7 @@ icu_set_collation_attributes(UCollator *collator, const char *loc,
        return;
    str++;
 
-   for (char *token = strtok(str, ";"); token; token = strtok(NULL, ";"))
+   while ((token = strsep(&str, ";")))
    {
        char       *e = strchr(token, '=');
 

diff --git a/src/common/logging.c b/src/common/logging.c
index e9a02e3e46a51b114e4b30af7626e314476f6289..aedd1ae2d8c12c4825cac7440820f5787abb47d6 100644 (file)
--- a/src/common/logging.c
+++ b/src/common/logging.c
@@ -119,7 +119,9 @@ pg_logging_init(const char *argv0)
 
            if (colors)
            {
-               for (char *token = strtok(colors, ":"); token; token = strtok(NULL, ":"))
+               char       *token;
+
+               while ((token = strsep(&colors, ":")))
                {
                    char       *e = strchr(token, '=');
 

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 9ff0a2d65e2178b905eb56509dba40b8f5d7bf0b..69a0caffa47381503475c1dd7cf4bfcfefca9ec9 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -234,12 +234,11 @@ static void
 split_to_stringlist(const char *s, const char *delim, _stringlist **listhead)
 {
    char       *sc = pg_strdup(s);
-   char       *token = strtok(sc, delim);
+   char       *token;
 
-   while (token)
+   while ((token = strsep(&sc, delim)))
    {
        add_stringlist_item(listhead, token);
-       token = strtok(NULL, delim);
    }
    free(sc);
 }