config forbid same server-port and admin-server-port (#3559)

* fix: forbid same server-port and admin-server-port Forbids server-port and admin-server-port from being equal altogether, despite they might not conflict at all in case admin and app are bound to different addresses. Implemented as per the discussion at #3508 (comment)
PostgREST · Jul 3, 2024 · 06cbc4b · 06cbc4b
1 parent b9004ba
commit 06cbc4b
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2052, Dropped support for PostgreSQL 9.6 - @wolfgangwalther
  - #2052, Dropped support for PostgreSQL 10 - @wolfgangwalther
  - #2052, Dropped support for PostgreSQL 11 - @wolfgangwalther
+ - #3508, PostgREST now fails to start when `server-port` and `admin-server-port` config options are the same - @develop7
 
 ### Documentation
 

diff --git a/docs/references/configuration.rst b/docs/references/configuration.rst
@@ -174,7 +174,7 @@ admin-server-port
   **In-Database** `n/a`
   =============== =======================
 
-  Specifies the port for the :ref:`admin_server`.
+  Specifies the port for the :ref:`admin_server`. Cannot be equal to :ref:`server-port`.
 
 .. _app.settings.*:
 

diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs
@@ -220,9 +220,9 @@ readAppConfig dbSettings optPath prevDbUri roleSettings roleIsolationLvl = do
 
   case C.runParser (parser optPath env dbSettings roleSettings roleIsolationLvl) =<< mapLeft show conf of
     Left err ->
-      return . Left $ "Error in config " <> err
-    Right parsedConfig ->
-      Right <$> decodeLoadFiles parsedConfig
+      return . Left $ "Error in config: " <> err
+    Right config ->
+      Right <$> decodeLoadFiles config
   where
     -- Both C.ParseError and IOError are shown here
     loadConfig :: FilePath -> IO (Either SomeException C.Config)
@@ -279,14 +279,14 @@ parser optPath env dbSettings roleSettings roleIsolationLvl =
     <*> parseOpenAPIServerProxyURI "openapi-server-proxy-uri"
     <*> parseCORSAllowedOrigins "server-cors-allowed-origins"
     <*> (defaultServerHost <$> optString "server-host")
-    <*> (fromMaybe 3000 <$> optInt "server-port")
+    <*> parseServerPort "server-port"
     <*> (fmap (CI.mk . encodeUtf8) <$> optString "server-trace-header")
     <*> (fromMaybe False <$> optBool "server-timing-enabled")
     <*> (fmap T.unpack <$> optString "server-unix-socket")
     <*> parseSocketFileMode "server-unix-socket-mode"
     <*> (defaultServerHost <$> optWithAlias (optString "admin-server-host")
                                             (optString "server-host"))
-    <*> optInt "admin-server-port"
+    <*> parseAdminServerPort "admin-server-port"
     <*> pure roleSettings
     <*> pure roleIsolationLvl
     <*> optInt "internal-schema-cache-sleep"
@@ -298,6 +298,17 @@ parser optPath env dbSettings roleSettings roleIsolationLvl =
         fromEnv = M.mapKeys fromJust $ M.filterWithKey (\k _ -> isJust k) $ M.mapKeys normalize env
         normalize k = ("app.settings." <>) <$> T.stripPrefix "PGRST_APP_SETTINGS_" (toS k)
 
+    parseServerPort :: C.Key -> C.Parser C.Config Int
+    parseServerPort k = fromMaybe 3000 <$> optInt k
+
+    parseAdminServerPort :: C.Key -> C.Parser C.Config (Maybe Int)
+    parseAdminServerPort k = do
+      serverPort <- parseServerPort "server-port"
+      optInt k >>= \case
+        Nothing -> pure Nothing
+        Just asp | asp == serverPort -> fail "admin-server-port cannot be the same as server-port"
+                 | otherwise         -> pure $ Just asp
+
     parseSocketFileMode :: C.Key -> C.Parser C.Config FileMode
     parseSocketFileMode k =
       optString k >>= \case

diff --git a/test/io/test_cli.py b/test/io/test_cli.py
@@ -123,6 +123,15 @@ def test_cli(args, env, use_defaultenv, expect, defaultenv):
             assert expect in dump
 
 
+def test_server_port_and_admin_port_same_value(defaultenv):
+    "PostgREST should exit with an error message in output if server-port and admin-server-port are the same."
+    env = {**defaultenv, "PGRST_SERVER_PORT": "3000", "PGRST_ADMIN_SERVER_PORT": "3000"}
+
+    with pytest.raises(PostgrestError):
+        dump = cli(["--dump-config"], env=env).split("\n")
+        assert "admin-server-port cannot be the same as server-port" in dump
+
+
 @pytest.mark.parametrize(
     "expectedconfig",
     [