From af1593f952fc338f9085422438ba9fd7e14bda8c Mon Sep 17 00:00:00 2001
From: Ryan Senior <ryan@metabase.com>
Date: Fri, 9 Nov 2018 11:27:54 -0600
Subject: [PATCH] Fix unspecified PG database connection strings
This issue can occur if the `MB_DB_DBNAME` variable has not been
specified. The `:dbname` key has a nil value which causes Ring's
`form-encode` function to fail. This commit explicitly removes the
`:dbname` as it's extra anyway and will also remove map entries that
have a nil key or value before encoding it.
---
src/metabase/db/spec.clj | 9 ++++++---
test/metabase/db/spec_test.clj | 36 ++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 3 deletions(-)
create mode 100644 test/metabase/db/spec_test.clj
diff --git a/src/metabase/db/spec.clj b/src/metabase/db/spec.clj
index 68f9acd5a39..ba09f511f30 100644
--- a/src/metabase/db/spec.clj
+++ b/src/metabase/db/spec.clj
@@ -3,9 +3,9 @@
Only databases that are supported as application DBs should have functions in this namespace;
otherwise, similar functions are only needed by drivers, and belong in those namespaces."
(:require [clojure.string :as str]
+ [medley.core :as m]
[ring.util.codec :as codec]))
-
(defn h2
"Create a database specification for a h2 database. Opts should include a key
for :db which is the path to the database file."
@@ -18,11 +18,14 @@
(dissoc opts :db)))
(defn- remove-required-keys [db-spec & more-keys]
- (apply dissoc db-spec (concat [:host :port :db :user :password :additional-options]
+ (apply dissoc db-spec (concat [:host :port :db :dbname :user :password :additional-options]
more-keys)))
+(defn- purge-nil-values [m]
+ (m/remove-kv (fn [k v] (or (nil? k) (nil? v))) m))
+
(defn- make-subname [host port db extra-connection-params]
- (let [query-params (codec/form-encode extra-connection-params)]
+ (let [query-params (codec/form-encode (purge-nil-values extra-connection-params))]
(str "//" host ":" port "/" db (when-not (str/blank? query-params)
(str "?" query-params)))))
diff --git a/test/metabase/db/spec_test.clj b/test/metabase/db/spec_test.clj
new file mode 100644
index 00000000000..fcbe3584b34
--- /dev/null
+++ b/test/metabase/db/spec_test.clj
@@ -0,0 +1,36 @@
+(ns metabase.db.spec-test
+ (:require [expectations :refer :all]
+ [metabase.db.spec :refer :all]))
+
+(defn- default-pg-spec [db]
+ {:classname "org.postgresql.Driver", :subprotocol "postgresql",
+ :subname (format "//localhost:5432/%s?OpenSourceSubProtocolOverride=true" db)})
+
+;; Basic minimal config
+(expect
+ (default-pg-spec "metabase")
+ (postgres {:host "localhost"
+ :port 5432
+ :db "metabase"}))
+
+;; Users that don't specify a `:dbname` (and thus no `:db`) will use the user's default, we should allow that
+(expect
+ (assoc (default-pg-spec "") :dbname nil)
+ (postgres {:host "localhost"
+ :port 5432
+ :dbname nil
+ :db nil}))
+
+;; We should be tolerant of other random nil values sneaking through
+(expect
+ (assoc (default-pg-spec "") :dbname nil, :somethingrandom nil)
+ (postgres {:host "localhost"
+ :port 5432
+ :dbname nil
+ :db nil
+ :somethingrandom nil}))
+
+;; Not specifying any of the values results in defaults
+(expect
+ (default-pg-spec "")
+ (postgres {}))
--
GitLab