Hi Michael

On 16.09.23 06:18, Michael Paquier wrote:
That was the idea. I forgot about strpos(), but if you do that, do we
actually need a function in core to achieve that?
I guess it depends who you ask :) I personally think it would be a good addition to the view, as it would provide a more comprehensive look into the hba file. Yes, the fact that it could possibly be written in SQL sounds silly, but it's IMHO still relevant to have it by default.
There are a few fancy cases with the SQL function you have sent, actually.. strpos() would grep the first '#' character, ignoring quoted areas.

Yes, you're totally right. I didn't take into account any token surrounded by double quotes containing #.

v3 attached addresses this issue.

From the following hba:

 host db jim 192.168.10.1/32 md5 # foo
 host db jim 192.168.10.2/32 md5 #bar
 host db jim 192.168.10.3/32 md5 #     #foo#
 host "a#db" "a#user" 192.168.10.4/32 md5 # fancy #hba entry

We can get these records from the view:

 SELECT type, database, user_name, address, comment
 FROM pg_hba_file_rules
 WHERE address ~~ '192.168.10.%';

 type | database | user_name | address    |     comment
------+----------+-----------+--------------+------------------
 host | {db}     | {jim}     | 192.168.10.1 | foo
 host | {db}     | {jim}     | 192.168.10.2 | bar
 host | {db}     | {jim}     | 192.168.10.3 | #foo#
 host | {a#db}   | {a#user}  | 192.168.10.4 | fancy #hba entry


I am still struggling to find a way to enable this function in separated path without having to read the conf file multiple times, or writing too much redundant code. How many other conf files do you think would profit from this feature?

Jim
From 47f55bab0a8e8af286e6be2f40d218f25a5066c9 Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jo...@uni-muenster.de>
Date: Wed, 20 Sep 2023 00:04:05 +0200
Subject: [PATCH v3] Add inline comments to the pg_hba_file_rules view

This patch proposes the column "comment" to the pg_hba_file_rules view.
It basically parses the inline comment (if any) of a valid pg_hba.conf
entry and displays it in the new column. The patch slightly changes the
test 004_file_inclusion.pl to accomodate the new column and the hba comments.
The view's documentation at system-views.sgml is extended accordingly.

The function GetInlineComment() was added to conffiles.c to avoid adding more
complexity directly to hba.c. It also enables this feature to be used by other
configuration files, if necessary.
---
 doc/src/sgml/system-views.sgml                |  9 +++
 src/backend/utils/adt/hbafuncs.c              | 11 ++-
 src/backend/utils/misc/conffiles.c            | 41 ++++++++++
 src/include/catalog/pg_proc.dat               |  6 +-
 src/include/libpq/hba.h                       |  1 +
 src/include/utils/conffiles.h                 |  1 +
 .../authentication/t/004_file_inclusion.pl    | 74 +++++++++++++++++--
 src/test/regress/expected/rules.out           |  3 +-
 8 files changed, 135 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 2b35c2f91b..68f9857de0 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -1090,6 +1090,15 @@
       </para></entry>
      </row>
 
+    <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>comment</structfield> <type>text</type>
+      </para>
+      <para>
+       Text after the first <literal>#</literal> comment character in the end of a valid <literal>pg_hba.conf</literal> entry, if any
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>error</structfield> <type>text</type>
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 73d3ad1dad..929678e97e 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -22,6 +22,7 @@
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/guc.h"
+#include "utils/conffiles.h"
 
 
 static ArrayType *get_hba_options(HbaLine *hba);
@@ -159,7 +160,7 @@ get_hba_options(HbaLine *hba)
 }
 
 /* Number of columns in pg_hba_file_rules view */
-#define NUM_PG_HBA_FILE_RULES_ATTS	 11
+#define NUM_PG_HBA_FILE_RULES_ATTS	 12
 
 /*
  * fill_hba_line
@@ -191,6 +192,7 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
 	const char *addrstr;
 	const char *maskstr;
 	ArrayType  *options;
+	char       *comment;
 
 	Assert(tupdesc->natts == NUM_PG_HBA_FILE_RULES_ATTS);
 
@@ -346,6 +348,13 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
 			values[index++] = PointerGetDatum(options);
 		else
 			nulls[index++] = true;
+
+		/* comments */
+		comment = GetInlineComment(hba->rawline);
+		if(comment)
+			values[index++] = CStringGetTextDatum(comment);
+		else
+			nulls[index++] = true;
 	}
 	else
 	{
diff --git a/src/backend/utils/misc/conffiles.c b/src/backend/utils/misc/conffiles.c
index 376a5c885b..feda49bf31 100644
--- a/src/backend/utils/misc/conffiles.c
+++ b/src/backend/utils/misc/conffiles.c
@@ -25,6 +25,47 @@
 #include "storage/fd.h"
 #include "utils/conffiles.h"
 
+/*
+ * GetInlineComment
+ *
+ * This function returns comments of a given config file line,
+ * if there is any. A comment is any text after the # character,
+ * as long as it is not located after a opeining '"'.
+ */
+char *
+GetInlineComment(char *line)
+{
+
+	size_t nq = 0;
+	char *comment;
+
+	for (int i = 0; i < strlen(line); i++) {
+
+		if (line[i] == '"')
+			nq++;
+		else if (line[i] == '#' && nq % 2 == 0)
+		{
+			size_t len = strlen(line);
+			comment = (char *) palloc(len * sizeof(char *));
+			memcpy(comment,&line[i+1],len-i);
+
+			/* trim leading and trailing whitespaces */
+			while (*comment && isspace((unsigned char) *comment))
+				comment++;
+			len = strlen(comment);
+			while (len > 0 && isspace((unsigned char) comment[len - 1]))
+				len--;
+			comment[len] = '\0';
+
+			return comment;
+		}
+
+	}
+
+	return NULL;
+
+ }
+
 /*
  * AbsoluteConfigLocation
  *
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9805bc6118..360f71e8ef 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6244,9 +6244,9 @@
 { oid => '3401', descr => 'show pg_hba.conf rules',
   proname => 'pg_hba_file_rules', prorows => '1000', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => '',
-  proallargtypes => '{int4,text,int4,text,_text,_text,text,text,text,_text,text}',
-  proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
-  proargnames => '{rule_number,file_name,line_number,type,database,user_name,address,netmask,auth_method,options,error}',
+  proallargtypes => '{int4,text,int4,text,_text,_text,text,text,text,_text,text,text}',
+  proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{rule_number,file_name,line_number,type,database,user_name,address,netmask,auth_method,options,comment,error}',
   prosrc => 'pg_hba_file_rules' },
 { oid => '6250', descr => 'show pg_ident.conf mappings',
   proname => 'pg_ident_file_mappings', prorows => '1000', proretset => 't',
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 189f6d0df2..5cb7224712 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -135,6 +135,7 @@ typedef struct HbaLine
 	char	   *radiusidentifiers_s;
 	List	   *radiusports;
 	char	   *radiusports_s;
+	char	   *comments;
 } HbaLine;
 
 typedef struct IdentLine
diff --git a/src/include/utils/conffiles.h b/src/include/utils/conffiles.h
index e3868fbc23..b60adae5fa 100644
--- a/src/include/utils/conffiles.h
+++ b/src/include/utils/conffiles.h
@@ -23,5 +23,6 @@ extern char **GetConfFilesInDir(const char *includedir,
 								const char *calling_file,
 								int elevel, int *num_filenames,
 								char **err_msg);
+extern char *GetInlineComment(char *line);
 
 #endif							/* CONFFILES_H */
diff --git a/src/test/authentication/t/004_file_inclusion.pl b/src/test/authentication/t/004_file_inclusion.pl
index 55d28ad586..199c824e33 100644
--- a/src/test/authentication/t/004_file_inclusion.pl
+++ b/src/test/authentication/t/004_file_inclusion.pl
@@ -63,8 +63,56 @@ sub add_hba_line
 	# Increment pg_hba_file_rules.rule_number and save it.
 	$globline = ++$line_counters{'hba_rule'};
 
+	# It is possible to have the character '#' in several elements of
+	# a hba entry if they're wrapped with double quotes. So we first check
+	# if a '#' comes after an opening '"', and if so we ignore it. If a '#'
+	# is located after a closing '"' or if there is no previous '"' in the
+	# string, we consider it as the beginning of an inline comment.
+
+	my $comment_position = -1;
+	my $nq = 0;
+
+	for my $i (0..length($entry)-1){
+
+		my $char = substr($entry, $i, 1);
+
+		if($char eq '"')
+		{
+			$nq = $nq + 1;
+		} elsif ($char eq '#' && $nq % 2 == 0)
+		{
+			$comment_position = $i;
+			last;
+		}
+
+	}
+
+	# In case the pg_hba entry has a comment, we store it in $comment
+	# and remove it from $entry. The content of $comment will be pushed
+	# into @tokens separataely. This avoids having whitespaces in $comment
+	# being interpreted as element separator by split(). The leading
+	# and trailing whitespaces in $comment are removed to reproduce the
+	# behaviour of GetInlineComment(char *line).
+
+	my $comment;
+
+	if($comment_position != -1)
+	{
+		$comment = substr($entry, $comment_position+1);
+		$comment =~ s/^\s+|\s+$//g;
+		$entry = substr($entry, 0, $comment_position);
+	}
+
 	# Generate the expected pg_hba_file_rules line
 	@tokens = split(/ /, $entry);
+
+	# Remove surrounding double quotes.
+	foreach (@tokens)
+	{
+		$_ =~ s/^"|"$//g;
+	}
+
+	# Wrapping database and user name with {}
 	$tokens[1] = '{' . $tokens[1] . '}';    # database
 	$tokens[2] = '{' . $tokens[2] . '}';    # user_name
 
@@ -72,6 +120,16 @@ sub add_hba_line
 	push @tokens, '';
 	push @tokens, '';
 
+	# Append comment, if any. Otherwise append empty string
+	if(not defined $comment)
+	{
+		push @tokens, '';
+	}
+	else
+	{
+		push @tokens, $comment;
+	}
+
 	# Final line expected, output of the SQL query.
 	$line = "";
 	$line .= "\n" if ($globline > 1);
@@ -167,15 +225,18 @@ mkdir("$data_dir/hba_inc_if");
 mkdir("$data_dir/hba_pos");
 
 # First, make sure that we will always be able to connect.
-$hba_expected .= add_hba_line($node, "$hba_file", 'local all all trust');
+$hba_expected .= add_hba_line($node, "$hba_file", 'local all all trust    #   # First, make sure that we will always be able to connect.   ');
+
+# Add database and user names containing # by wrapping them with double quotes.
+$hba_expected .= add_hba_line($node, "$hba_file", 'local "fancy#dbname" "fancy#username" trust# Add database and user names containing # by wrapping them with double quotes.');
 
 # "include".  Note that as $hba_file is located in $data_dir/subdir1,
 # pg_hba_pre.conf is located at the root of the data directory.
 $hba_expected .=
-  add_hba_line($node, "$hba_file", "include ../pg_hba_pre.conf");
+  add_hba_line($node, "$hba_file", "include ../pg_hba_pre.conf#foo");
 $hba_expected .=
   add_hba_line($node, 'pg_hba_pre.conf', "local pre all reject");
-$hba_expected .= add_hba_line($node, "$hba_file", "local all all reject");
+$hba_expected .= add_hba_line($node, "$hba_file", 'local all "all" reject #bar');
 add_hba_line($node, "$hba_file", "include ../hba_pos/pg_hba_pos.conf");
 $hba_expected .=
   add_hba_line($node, 'hba_pos/pg_hba_pos.conf', "local pos all reject");
@@ -184,7 +245,7 @@ $hba_expected .=
 $hba_expected .=
   add_hba_line($node, 'hba_pos/pg_hba_pos.conf', "include pg_hba_pos2.conf");
 $hba_expected .=
-  add_hba_line($node, 'hba_pos/pg_hba_pos2.conf', "local pos2 all reject");
+  add_hba_line($node, 'hba_pos/pg_hba_pos2.conf', "local pos2 all reject #bar!#");
 $hba_expected .=
   add_hba_line($node, 'hba_pos/pg_hba_pos2.conf', "local pos3 all reject");
 
@@ -216,7 +277,7 @@ $hba_expected .= "\n"
   . $line_counters{'hba_rule'} . "|"
   . basename($hba_file) . "|"
   . $line_counters{$hba_file}
-  . '|local|{db1,db3}|{all}|reject||';
+  . '|local|{db1,db3}|{all}|reject|||';
 
 note "Generating ident structure with include directives";
 
@@ -279,7 +340,8 @@ my $contents = $node->safe_psql(
   user_name,
   auth_method,
   options,
-  error
+  error,
+  comment
  FROM pg_hba_file_rules ORDER BY rule_number;));
 is($contents, $hba_expected, 'check contents of pg_hba_file_rules');
 
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 5058be5411..6a6c6f9edd 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1348,8 +1348,9 @@ pg_hba_file_rules| SELECT rule_number,
     netmask,
     auth_method,
     options,
+    comment,
     error
-   FROM pg_hba_file_rules() a(rule_number, file_name, line_number, type, database, user_name, address, netmask, auth_method, options, error);
+   FROM pg_hba_file_rules() a(rule_number, file_name, line_number, type, database, user_name, address, netmask, auth_method, options, comment, error);
 pg_ident_file_mappings| SELECT map_number,
     file_name,
     line_number,
-- 
2.34.1

Reply via email to