Re: Minor issues in .pgpass

From: Hamid Akhtar <hamid(dot)akhtar(at)gmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Subject: Re: Minor issues in .pgpass
Date: 2020-02-28 15:46:18
Message-ID: 158290477891.14909.16506889684573597132.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested

First of all, this seems like fixing a valid issue, albeit, the probability of somebody messing is low, but it is still better to fix this problem.

I've not tested the patch in any detail, however, there are a couple of comments I have before I proceed on with detailed testing.

1. pgindent is showing a few issues with formatting. Please have a look and resolve those.
2. I think you can potentially use "len" variable instead of introducing "buflen" and "tmplen" variables. Also, I would choose a more appropriate name for "tmp" variable.

I believe if you move the following lines before the conditional statement and simply and change the if statement to "if (len >= sizeof(buf) - 1)", it will serve the purpose.
========================================
/* strip trailing newline and carriage return */
len = pg_strip_crlf(buf);

if (len == 0)
continue;
========================================

So, the patch should look like this in my opinion (ignore the formatting issues as this is just to give you an idea of what I mean):

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 408000a..6ca262f 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -6949,6 +6949,7 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
{
FILE *fp;
struct stat stat_buf;
+ int line_number = 0;

#define LINELEN NAMEDATALEN*5
char buf[LINELEN];
@@ -7018,10 +7019,40 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
if (fgets(buf, sizeof(buf), fp) == NULL)
break;

- /* strip trailing newline and carriage return */
- len = pg_strip_crlf(buf);
+ line_number++;

- if (len == 0)
+ /* strip trailing newline and carriage return */
+ len = pg_strip_crlf(buf);
+
+ if (len == 0)
+ continue;
+
+ if (len >= sizeof(buf) - 1)
+ {
+ char tmp[LINELEN];
+
+ /*
+ * Warn if this password setting line is too long,
+ * because it's unexpectedly truncated.
+ */
+ if (buf[0] != '#')
+ fprintf(stderr,
+ libpq_gettext("WARNING: line %d too long in password file \"%s\"\n"),
+ line_number, pgpassfile);
+
+ /* eat rest of the line */
+ while (!feof(fp) && !ferror(fp))
+ {
+ if (fgets(tmp, sizeof(tmp), fp) == NULL)
+ break;
+ len = strlen(tmp);
+ if (len < sizeof(tmp) -1 || tmp[len - 1] == '\n')
+ break;
+ }
+ }
+
+ /* ignore comments */
+ if (buf[0] == '#')

---
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL:+923335449950  EMAIL: mailto:hamid(dot)akhtar(at)highgo(dot)ca
SKYPE: engineeredvirus

The new status of this patch is: Waiting on Author

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2020-02-28 15:57:09 Re: Created feature for to_date() conversion using patterns 'YYYY-WW', 'YYYY-WW-D', 'YYYY-MM-W' and 'YYYY-MM-W-D'
Previous Message Tom Lane 2020-02-28 15:35:56 Re: Trying to pull up EXPR SubLinks