fix: not adding application_name on all URIs

CHANGELOG.md

@@ -21,6 +21,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #3404, Show extra information in the `PGRST121` (could not parse RAISE 'PGRST') error - @laurenceisla
    + Shows the failed MESSAGE or DETAIL in the `details` field
    + Shows the correct JSON format in the `hints` field
+ - #3340, Log when the LISTEN channel gets a notification - @steve-chavez
+ - #3184, Log full pg version to stderr on connection - @steve-chavez
 
 ### Fixed
 
@@ -32,14 +34,14 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #3237, Dump media handlers and timezones with --dump-schema - @wolfgangwalther
  - #3323, #3324, Don't hide error on LISTEN channel failure - @steve-chavez
  - #3330, Incorrect admin server `/ready` response on slow schema cache loads - @steve-chavez
- - #3340, Log when the LISTEN channel gets a notification - @steve-chavez
  - #3345, Fix in-database configuration values not loading for `pgrst.server_trace_header` and `pgrst.server_cors_allowed_origins` - @laurenceisla
  - #3361, Clarify the `PGRST204` (column not found) error message - @steve-chavez
  - #3373, Remove rejected mediatype `application/vnd.pgrst.object+json` from response - @taimoorzaeem
  - #3418, Fix OpenAPI not tagging a FK column correctly on O2O relationships - @laurenceisla
  - #3256, Fix wrong http status for pg error `42P17 infinite recursion` - @taimoorzaeem
  - #3404, Clarify the `PGRST121` (could not parse RAISE 'PGRST') error message - @laurenceisla
  - #3267, Fix wrong `503 Service Unavailable` on pg error `53400` - @taimoorzaeem
+ - #2985, Fix not adding `application_name` on all connection strings - @steve-chavez
 
 ### Deprecated
 

docs/postgrest.dict

@@ -157,7 +157,6 @@ stateful
 stdout
 supervisees
 SvelteKit
-syslog
 systemd
 todo
 todos

docs/references/connection_pool.rst

@@ -7,11 +7,6 @@ A connection pool is a cache of reusable database connections. It allows serving
 
 Minimizing connections is paramount to performance. Each PostgreSQL connection creates a process, having too many can exhaust available resources.
 
-Connection String
------------------
-
-For connecting to the database, the pool requires a connection string. You can configure it using :ref:`db-uri`.
-
 .. _pool_growth_limit:
 .. _dyn_conn_pool:
 
@@ -22,6 +17,26 @@ To conserve system resources, PostgREST uses a dynamic connection pool. This ena
 
 - If all the connections are being used, a new connection is added. The pool can grow until it reaches the :ref:`db-pool` size. Note that it’s pointless to set this higher than the ``max_connections`` setting in your database.
 - If a connection is unused for a period of time (:ref:`db-pool-max-idletime`), it will be released.
+- For connecting to the database, the :ref:`authenticator <roles>` role is used. You can configure this using :ref:`db-uri`.
+
+Connection Application Name
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+PostgREST sets the connection `application_name <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-FALLBACK-APPLICATION-NAME>`_ for all of its used connections.
+This is useful for PostgreSQL statistics and logs.
+
+For example, you can query `pg_stat_activity <https://www.postgresql.org/docs/current/monitoring-stats.html#MONITORING-PG-STAT-ACTIVITY-VIEW>`_ to get the PostgREST version:
+
+.. code-block:: postgres
+
+  select distinct usename, application_name
+  from pg_stat_activity
+  where usename = 'authenticator';
+
+     usename     |     application_name
+  ---------------+--------------------------
+   authenticator | PostgREST 12.1
+
 
 Connection lifetime
 -------------------

docs/references/observability.rst

@@ -20,33 +20,20 @@ PostgREST logs basic request information to ``stdout``, including the authentica
    127.0.0.1 - user [26/Jul/2021:01:56:38 -0500] "GET /clients HTTP/1.1" 200 - "" "curl/7.64.0"
    127.0.0.1 - anonymous [26/Jul/2021:01:56:48 -0500] "GET /unexistent HTTP/1.1" 404 - "" "curl/7.64.0"
 
-For diagnostic information about the server itself, PostgREST logs to ``stderr``.
+For diagnostic information about the server itself, PostgREST logs to ``stderr``. It includes the server version and also the version of the connected PostgreSQL.
 
 .. code::
 
-   12/Jun/2021:17:47:39 -0500: Starting PostgREST 11.1.0...
-   12/Jun/2021:17:47:39 -0500: Attempting to connect to the database...
-   12/Jun/2021:17:47:39 -0500: Listening on port 3000
-   12/Jun/2021:17:47:39 -0500: Connection successful
-   12/Jun/2021:17:47:39 -0500: Config re-loaded
-   12/Jun/2021:17:47:40 -0500: Schema cache loaded
-
-.. note::
-
-   When running it in an SSH session you must detach it from stdout or it will be terminated when the session closes. The easiest technique is redirecting the output to a log file or to the syslog:
-
-   .. code-block:: bash
-
-     ssh [email protected] \
-       'postgrest foo.conf </dev/null >/var/log/postgrest.log 2>&1 &'
-
-     # another option is to pipe the output into "logger -t postgrest"
-
-Currently PostgREST doesn't log the SQL commands executed against the underlying database.
+   06/May/2024:08:16:11 -0500: Starting PostgREST 12.1...
+   06/May/2024:08:16:11 -0500: Attempting to connect to the database...
+   06/May/2024:08:16:11 -0500: Successfully connected to PostgreSQL 14.10 (Ubuntu 14.10-0ubuntu0.22.04.1) on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0, 64-bit
+   06/May/2024:08:16:11 -0500: Listening on port 3000
 
 Database Logs
 -------------
 
+Currently PostgREST doesn't log the SQL commands executed against the underlying database.
+
 To find the SQL operations, you can watch the database logs. By default PostgreSQL does not keep these logs, so you'll need to make the configuration changes below.
 
 Find :code:`postgresql.conf` inside your PostgreSQL data directory (to find that, issue the command :code:`show data_directory;`). Either find the settings scattered throughout the file and change them to the following values, or append this block of code to the end of the configuration file.
@@ -172,39 +159,17 @@ Max pool connections.
 Traces
 ======
 
-Server Version
---------------
-
-When debugging a problem it's important to verify the running PostgREST version. There are three ways to do this:
+Server Version Header
+---------------------
 
-- Look for the :code:`Server` HTTP response header that is returned on every request.
+When debugging a problem it's important to verify the running PostgREST version. For this you can look at the :code:`Server` HTTP response header that is returned on every request.
 
 .. code::
 
   HEAD /users HTTP/1.1
 
   Server: postgrest/11.0.1
 
-- Query ``application_name`` on `pg_stat_activity <https://www.postgresql.org/docs/current/monitoring-stats.html#MONITORING-PG-STAT-ACTIVITY-VIEW>`_.
-
-.. code-block:: postgres
-
-  select distinct application_name
-  from pg_stat_activity
-  where application_name ilike '%postgrest%';
-
-        application_name
-  ------------------------------
-  PostgREST 11.1.0
-
-.. important::
-
-  - The server sets the `fallback_application_name <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-FALLBACK-APPLICATION-NAME>`_ to the connection URI for this query to work. To override the value set ``application_name`` on the connection string.
-  - The version will only be set if it's a valid URI (`RFC 3986 <https://datatracker.ietf.org/doc/html/rfc3986>`_). This means any special characters must be urlencoded.
-  - The version will not be set if the connection string is in `keyword/value format <https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING-KEYWORD-VALUE>`_.
-
-- The ``stderr`` logs also contain the version, as noted on :ref:`pgrst_logging`.
-
 .. _trace_header:
 
 Trace Header
@@ -354,6 +319,7 @@ For example, to only allow requests from an IP address to get the execution plan
 
     const redirects = {
       '#health_check': 'health_check.html',
+      '#server-version': '#server-version-header',
     };
 
     let willRedirectTo = redirects[hash];

src/PostgREST/Config.hs

@@ -49,8 +49,7 @@ import Data.List.NonEmpty      (fromList, toList)
 import Data.Maybe              (fromJust)
 import Data.Scientific         (floatingOrInteger)
 import Network.URI             (escapeURIString,
-                                isUnescapedInURIComponent, parseURI,
-                                uriQuery)
+                                isUnescapedInURIComponent)
 import Numeric                 (readOct, showOct)
 import System.Environment      (getEnvironment)
 import System.Posix.Types      (FileMode)
@@ -502,19 +501,47 @@ readPGRSTEnvironment =
 -- "postgres:///postgres?host=server&port=5432&fallback_application_name=PostgREST%2011%271%260%40%23%24%25%2C.%3A%22%5B%5D%7B%7D%3F%2B%5E%28%29%3Dasdfqwer"
 --
 -- >>> addFallbackAppName ver "postgres://user:invalid_chars[]#@host:5432/postgres"
--- "postgres://user:invalid_chars[]#@host:5432/postgres"
+-- "postgres://user:invalid_chars[]#@host:5432/postgres?fallback_application_name=PostgREST%2011.1.0%20%285a04ec7%29"
 --
--- >>> addFallbackAppName ver "invalid_uri1=val1 invalid_uri2=val2"
--- "invalid_uri1=val1 invalid_uri2=val2"
+-- >>> addFallbackAppName ver "host=localhost port=5432 dbname=postgres"
+-- "host=localhost port=5432 dbname=postgres fallback_application_name='PostgREST 11.1.0 (5a04ec7)'"
+--
+-- >>> addFallbackAppName strangeVer "host=localhost port=5432 dbname=postgres"
+-- "host=localhost port=5432 dbname=postgres fallback_application_name='PostgREST 11\\'1&0@#$%,.:\"[]{}?+^()=asdfqwer'"
+--
+-- works with passwords containing `?`
+-- >>> addFallbackAppName ver "postgres://admin2:?pass?special?@localhost:5432/postgres"
+-- "postgres://admin2:?pass?special?@localhost:5432/postgres?fallback_application_name=PostgREST%2011.1.0%20%285a04ec7%29"
+--
+-- addFallbackAppName ver "postgresql://?dbname=postgres&host=/run/user/1000/postgrest/postgrest-with-postgresql-16-BuR/socket&user=some_protected_user&password=invalid_pass"
+-- "postgresql://?dbname=postgres&host=/run/user/1000/postgrest/postgrest-with-postgresql-16-BuR/socket&user=some_protected_user&password=invalid_pass&fallback_application_name=PostgREST%2011.1.0%20%285a04ec7%29"
+--
+-- addFallbackAppName ver "postgresql:///postgres?host=/run/user/1000/postgrest/postgrest-with-postgresql-16-BuR/socket&user=some_protected_user&password=invalid_pass"
+-- "postgresql:///postgres?host=/run/user/1000/postgrest/postgrest-with-postgresql-16-BuR/socket&user=some_protected_user&password=invalid_pass&fallback_application_name=PostgREST%2011.1.0%20%285a04ec7%29"
 addFallbackAppName :: ByteString -> Text -> Text
 addFallbackAppName version dbUri = dbUri <>
-  case uriQuery <$> parseURI (toS dbUri) of
-    -- Does not add the application name to key=val connection strings or invalid URIs
+  case pgConnString dbUri of
     Nothing  -> mempty
-    Just ""  -> "?" <> uriFmt
-    Just "?" -> uriFmt
-    _        -> "&" <> uriFmt
+    Just PGKeyVal -> " " <> keyValFmt
+    Just PGURI    -> case lookAtOptions dbUri of
+      (_, "")  -> "?" <> uriFmt
+      (_, "?") -> uriFmt
+      (_, _)   -> "&" <> uriFmt
   where
     uriFmt = pKeyWord <> toS (escapeURIString isUnescapedInURIComponent $ toS pgrstVer)
+    keyValFmt = pKeyWord <> "'" <> T.replace "'" "\\'" pgrstVer <> "'"
     pKeyWord = "fallback_application_name="
     pgrstVer = "PostgREST " <> T.decodeUtf8 version
+    lookAtOptions x =  T.breakOn "?" . snd $ T.breakOnEnd "@" x -- start from after `@` to not mess passwords that include `?`, see https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING-URIS
+
+data PGConnString = PGURI | PGKeyVal
+
+-- Uses same logic as libpq recognized_connection_string
+-- https://github.com/postgres/postgres/blob/5eafacd2797dc0b04a0bde25fbf26bf79903e7c2/src/interfaces/libpq/fe-connect.c#L5923-L5936
+pgConnString :: Text -> Maybe PGConnString
+pgConnString conn | uriDesignator `T.isPrefixOf` conn || shortUriDesignator `T.isPrefixOf` conn = Just PGURI
+                  | "=" `T.isInfixOf` conn                                                      = Just PGKeyVal
+                  | otherwise                                                                   = Nothing
+  where
+    uriDesignator = "postgresql://"
+    shortUriDesignator = "postgres://"