Fix RADIUS error reporting in hba file parsing

The RADIUS-related checks in parse_hba_line() did not respect elevel
and did not fill in *err_msg.  Also, verify_option_list_length()
pasted together error messages in an untranslatable way.  To fix the
latter, remove the function and do the error checking inline.  It's a
bit more verbose but only minimally longer, and it makes fixing the
first two issues straightforward.

Reviewed-by: Magnus Hagander <magnus@hagander.net>
Discussion: https://www.postgresql.org/message-id/flat/8381e425-8c23-99b3-15ec-3115001db1b2%40enterprisedb.com

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 60767f295722a3acba0f2fa07bb0a268a2389764..3be8778d21668ae2321cac94390cdbeb1bb18641 100644 (file)
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -144,8 +144,6 @@ static List *tokenize_inc_file(List *tokens, const char *outer_filename,
                               const char *inc_filename, int elevel, char **err_msg);
 static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
                               int elevel, char **err_msg);
-static bool verify_option_list_length(List *options, const char *optionname,
-                                     List *comparelist, const char *comparename, int line_num);
 static ArrayType *gethba_options(HbaLine *hba);
 static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
                          int lineno, HbaLine *hba, const char *err_msg);
@@ -1607,21 +1605,23 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 
        if (list_length(parsedline->radiusservers) < 1)
        {
-           ereport(LOG,
+           ereport(elevel,
                    (errcode(ERRCODE_CONFIG_FILE_ERROR),
                     errmsg("list of RADIUS servers cannot be empty"),
                     errcontext("line %d of configuration file \"%s\"",
                                line_num, HbaFileName)));
+           *err_msg = "list of RADIUS servers cannot be empty";
            return NULL;
        }
 
        if (list_length(parsedline->radiussecrets) < 1)
        {
-           ereport(LOG,
+           ereport(elevel,
                    (errcode(ERRCODE_CONFIG_FILE_ERROR),
                     errmsg("list of RADIUS secrets cannot be empty"),
                     errcontext("line %d of configuration file \"%s\"",
                                line_num, HbaFileName)));
+           *err_msg = "list of RADIUS secrets cannot be empty";
            return NULL;
        }
 
@@ -1630,24 +1630,53 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
         * but that's already checked above), 1 (use the same value
         * everywhere) or the same as the number of servers.
         */
-       if (!verify_option_list_length(parsedline->radiussecrets,
-                                      "RADIUS secrets",
-                                      parsedline->radiusservers,
-                                      "RADIUS servers",
-                                      line_num))
+       if (!(list_length(parsedline->radiussecrets) == 1 ||
+             list_length(parsedline->radiussecrets) == list_length(parsedline->radiusservers)))
+       {
+           ereport(elevel,
+                   (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                    errmsg("the number of RADIUS secrets (%d) must be 1 or the same as the number of RADIUS servers (%d)",
+                           list_length(parsedline->radiussecrets),
+                           list_length(parsedline->radiusservers)),
+                    errcontext("line %d of configuration file \"%s\"",
+                               line_num, HbaFileName)));
+           *err_msg = psprintf("the number of RADIUS secrets (%d) must be 1 or the same as the number of RADIUS servers (%d)",
+                               list_length(parsedline->radiussecrets),
+                               list_length(parsedline->radiusservers));
            return NULL;
-       if (!verify_option_list_length(parsedline->radiusports,
-                                      "RADIUS ports",
-                                      parsedline->radiusservers,
-                                      "RADIUS servers",
-                                      line_num))
+       }
+       if (!(list_length(parsedline->radiusports) == 0 ||
+             list_length(parsedline->radiusports) == 1 ||
+             list_length(parsedline->radiusports) == list_length(parsedline->radiusservers)))
+       {
+           ereport(elevel,
+                   (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                    errmsg("the number of RADIUS ports (%d) must be 1 or the same as the number of RADIUS servers (%d)",
+                           list_length(parsedline->radiusports),
+                           list_length(parsedline->radiusservers)),
+                    errcontext("line %d of configuration file \"%s\"",
+                               line_num, HbaFileName)));
+           *err_msg = psprintf("the number of RADIUS ports (%d) must be 1 or the same as the number of RADIUS servers (%d)",
+                               list_length(parsedline->radiusports),
+                               list_length(parsedline->radiusservers));
            return NULL;
-       if (!verify_option_list_length(parsedline->radiusidentifiers,
-                                      "RADIUS identifiers",
-                                      parsedline->radiusservers,
-                                      "RADIUS servers",
-                                      line_num))
+       }
+       if (!(list_length(parsedline->radiusidentifiers) == 0 ||
+             list_length(parsedline->radiusidentifiers) == 1 ||
+             list_length(parsedline->radiusidentifiers) == list_length(parsedline->radiusservers)))
+       {
+           ereport(elevel,
+                   (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                    errmsg("the number of RADIUS identifiers (%d) must be 1 or the same as the number of RADIUS servers (%d)",
+                           list_length(parsedline->radiusidentifiers),
+                           list_length(parsedline->radiusservers)),
+                    errcontext("line %d of configuration file \"%s\"",
+                               line_num, HbaFileName)));
+           *err_msg = psprintf("the number of RADIUS identifiers (%d) must be 1 or the same as the number of RADIUS servers (%d)",
+                               list_length(parsedline->radiusidentifiers),
+                               list_length(parsedline->radiusservers));
            return NULL;
+       }
    }
 
    /*
@@ -1662,29 +1691,6 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 }
 
 
-static bool
-verify_option_list_length(List *options, const char *optionname,
-                         List *comparelist, const char *comparename,
-                         int line_num)
-{
-   if (list_length(options) == 0 ||
-       list_length(options) == 1 ||
-       list_length(options) == list_length(comparelist))
-       return true;
-
-   ereport(LOG,
-           (errcode(ERRCODE_CONFIG_FILE_ERROR),
-            errmsg("the number of %s (%d) must be 1 or the same as the number of %s (%d)",
-                   optionname,
-                   list_length(options),
-                   comparename,
-                   list_length(comparelist)
-                   ),
-            errcontext("line %d of configuration file \"%s\"",
-                       line_num, HbaFileName)));
-   return false;
-}
-
 /*
  * Parse one name-value pair as an authentication option into the given
  * HbaLine.  Return true if we successfully parse the option, false if we