Re: Fix RADIUS error reporting in hba file parsing

[Magnus Hagander]

On Thu, May 27, 2021 at 10:36 AM Peter Eisentraut
 wrote:
>
>
> 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.

LGTM. I agree that the extra code from removing the function is worth
it if it makes it better for translations.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

Fix RADIUS error reporting in hba file parsing

[Peter Eisentraut]

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.
From 063a8ae81d5ac33e8700bb0ea076d1499d36b655 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 27 May 2021 10:26:21 +0200
Subject: [PATCH] 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.
---
 src/backend/libpq/hba.c | 90 ++---
 1 file changed, 48 insertions(+), 42 deletions(-)

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 60767f2957..3be8778d21 100644
--- 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 confi