Re: Minor issues in .pgpass - Mailing list 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
betterto 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
detailedtesting.
 

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
choosea 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
whatI 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.akhtar@highgo.ca
SKYPE: engineeredvirus

The new status of this patch is: Waiting on Author