Re: Parsing errors in pg_hba.conf

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Tom Lane wrote:
>>> It would be seriously messy to try to make the parse-error code know
>>> about things like that.  Better would be to keep the GUC variable in
>>> existence always and have an assign hook to throw the error.
>
>> Um, no, it wouldn't :-) At least that's my impression. We're only
>> talking a bout the pg_hba code.
>
> Oh, sorry, for some reason I read this as postgresql.conf parsing.
> Too early in the morning, need more caffeine ;-)

:-) THat's what I was guessing.

Anyway, here's what I intended:

//Magnus

*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***************
*** 637,644 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
  #ifdef USE_SSL
              parsedline->conntype = ctHostSSL;
  #else
!             /* We don't accept this keyword at all if no SSL support */
!             goto hba_syntax;
  #endif
          }
  #ifdef USE_SSL
--- 637,649 ----
  #ifdef USE_SSL
              parsedline->conntype = ctHostSSL;
  #else
!             ereport(LOG,
!                     (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                      errmsg("hostssl not supported on this platform"),
!                      errhint("compile with --enable-ssl to use SSL connections"),
!                      errcontext("line %d of configuration file \"%s\"",
!                             line_num, HbaFileName)));
!             return false;
  #endif
          }
  #ifdef USE_SSL
***************
*** 654,671 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
          }
      } /* record type */
      else
!         goto hba_syntax;

      /* Get the database. */
      line_item = lnext(line_item);
      if (!line_item)
!         goto hba_syntax;
      parsedline->database = pstrdup(lfirst(line_item));

      /* Get the role. */
      line_item = lnext(line_item);
      if (!line_item)
!         goto hba_syntax;
      parsedline->role = pstrdup(lfirst(line_item));

      if (parsedline->conntype != ctLocal)
--- 659,698 ----
          }
      } /* record type */
      else
!     {
!         ereport(LOG,
!                 (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                  errmsg("invalid connection type \"%s\"",
!                         token),
!                  errcontext("line %d of configuration file \"%s\"",
!                         line_num, HbaFileName)));
!         return false;
!     }

      /* Get the database. */
      line_item = lnext(line_item);
      if (!line_item)
!     {
!         ereport(LOG,
!                 (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                  errmsg("end-of-line before database specification"),
!                  errcontext("line %d of configuration file \"%s\"",
!                         line_num, HbaFileName)));
!         return false;
!     }
      parsedline->database = pstrdup(lfirst(line_item));

      /* Get the role. */
      line_item = lnext(line_item);
      if (!line_item)
!     {
!         ereport(LOG,
!                 (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                  errmsg("end-of-line before role specification"),
!                  errcontext("line %d of configuration file \"%s\"",
!                         line_num, HbaFileName)));
!         return false;
!     }
      parsedline->role = pstrdup(lfirst(line_item));

      if (parsedline->conntype != ctLocal)
***************
*** 673,679 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
          /* Read the IP address field. (with or without CIDR netmask) */
          line_item = lnext(line_item);
          if (!line_item)
!             goto hba_syntax;
          token = pstrdup(lfirst(line_item));

          /* Check if it has a CIDR suffix and if so isolate it */
--- 700,713 ----
          /* Read the IP address field. (with or without CIDR netmask) */
          line_item = lnext(line_item);
          if (!line_item)
!         {
!             ereport(LOG,
!                     (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                      errmsg("end-of-line before ip address specification"),
!                      errcontext("line %d of configuration file \"%s\"",
!                             line_num, HbaFileName)));
!             return false;
!         }
          token = pstrdup(lfirst(line_item));

          /* Check if it has a CIDR suffix and if so isolate it */
***************
*** 718,731 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
          {
              if (pg_sockaddr_cidr_mask(&parsedline->mask, cidr_slash + 1,
                                        parsedline->addr.ss_family) < 0)
!                 goto hba_syntax;
          }
          else
          {
              /* Read the mask field. */
              line_item = lnext(line_item);
              if (!line_item)
!                 goto hba_syntax;
              token = lfirst(line_item);

              ret = pg_getaddrinfo_all(token, NULL, &hints, &gai_result);
--- 752,780 ----
          {
              if (pg_sockaddr_cidr_mask(&parsedline->mask, cidr_slash + 1,
                                        parsedline->addr.ss_family) < 0)
!             {
!                 ereport(LOG,
!                         (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                          errmsg("invalid CIDR mask in address \"%s\"",
!                                 token),
!                          errcontext("line %d of configuration file \"%s\"",
!                             line_num, HbaFileName)));
!                 return false;
!             }
          }
          else
          {
              /* Read the mask field. */
              line_item = lnext(line_item);
              if (!line_item)
!             {
!                 ereport(LOG,
!                         (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                          errmsg("end-of-line before netmask specification"),
!                          errcontext("line %d of configuration file \"%s\"",
!                                 line_num, HbaFileName)));
!                 return false;
!             }
              token = lfirst(line_item);

              ret = pg_getaddrinfo_all(token, NULL, &hints, &gai_result);
***************
*** 759,765 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
      /* Get the authentication method */
      line_item = lnext(line_item);
      if (!line_item)
!         goto hba_syntax;
      token = lfirst(line_item);

      unsupauth = NULL;
--- 808,821 ----
      /* Get the authentication method */
      line_item = lnext(line_item);
      if (!line_item)
!     {
!         ereport(LOG,
!                 (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                  errmsg("end-of-line before authentication method"),
!                  errcontext("line %d of configuration file \"%s\"",
!                         line_num, HbaFileName)));
!         return false;
!     }
      token = lfirst(line_item);

      unsupauth = NULL;
***************
*** 937,959 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
      }

      return true;
-
- hba_syntax:
-     if (line_item)
-         ereport(LOG,
-                 (errcode(ERRCODE_CONFIG_FILE_ERROR),
-                  errmsg("invalid token \"%s\"",
-                         (char *) lfirst(line_item)),
-                  errcontext("line %d of configuration file \"%s\"",
-                         line_num, HbaFileName)));
-     else
-         ereport(LOG,
-                 (errcode(ERRCODE_CONFIG_FILE_ERROR),
-                  errmsg("missing field at end of line"),
-                  errcontext("line %d of configuration file \"%s\"",
-                         line_num, HbaFileName)));
-
-     return false;
  }


--- 993,998 ----