luciano/honeysql
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixes #316 by adding check on entity characters

Browse source 
Also record that documentation addresses #300, #309, #313, and #314.
This commit is contained in:
Sean Corfield 2021-04-09 23:41:59 -07:00
parent cf7e36a131
commit 88282ee258
6 changed files with 186 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGELOG.md
View file

@ -1,8 +1,10 @@
# Changes
* 2.0.next in progress
* Since Alpha 3, more documentation has been written and existing documentation clarified (addressing #300, #309, #313, #314).
* Fix #319 by ensuring `register-clause!` is idempotent.
* Fix #317 by dropping qualifiers in `:set` clauses (just like we do with `:insert` columns). Note that you can still use explicit _dotted_ names if you want table qualification.
* Fix #316 by disallowing entity names containing `;` (to avoid SQL injection risks).
* Fix #312 by adding `:raw` as a clause. There is no helper function equivalent (because it would be ambiguous whether you meant a function form -- `[:raw ..]` -- or a clause form -- `{:raw ..}`; and for the same reason, there is no `nest` helper function since that also works as a clause and as a function/special syntax).
* 2.0.0-alpha3 (for early testing; 2021-03-13)
@ -21,9 +23,11 @@
* Reconcile `where` behavior with recent 1.0 changes (porting #283 to v2).
* Fix #280 by adding `:escape` as special syntax for regular expression patterns.
* Fix #277 by adding `:join-by`/`join-by` so that you can have multiple `JOIN`'s in a specific order.
* 2.0.0-alpha2 (for early testing)
* Since Alpha 1, a lot more documentation has been written and docstrings have been added to most functions in `honey.sql.helpers`.
* Numerous small improvements have been made to clauses and helpers around insert/upsert.
* 2.0.0-alpha1 (for early testing)
* This is a complete rewrite/simplification of HoneySQL that provides just two namespaces:
* `honey.sql` -- this is the primary API via the `format` function as well as the various extension points.

2
doc/clause-reference.md
View file

@ -428,6 +428,8 @@ user=> (sql/format '{insert-into (((transport t) (id, name)) {select (*) from (c
["INSERT INTO transport AS t (id, name) SELECT * FROM cars"]
```
> Note: if you specify `:columns` for an `:insert-into` that also includes column names, you will get invalid SQL. Similarly, if you specify `:columns` when `:values` is based on hash maps, you will get invalid SQL. Since clauses are generated independently, there is no cross-checking performed if you provide an illegal combination of clauses.
## update
`:update` expects either a simple SQL entity (table name)

16
src/honey/sql.cljc
View file

@ -98,6 +98,10 @@
(def ^:private ^:dynamic *quoted* nil)
(def ^:private ^:dynamic *inline* nil)
(def ^:private ^:dynamic *params* nil)
;; there is no way, currently, to enable suspicious characters
;; in entities; if someone complains about this check, an option
;; can be added to format to turn this on:
(def ^:private ^:dynamic *allow-suspicious-entities* false)
;; clause helpers
@ -164,12 +168,18 @@
(if aliased
[nil (nn x)]
(let [[t c] (str/split (nn x) #"\.")]
(if c [t c] [nil t]))))]
(cond->> c
(if c [t c] [nil t]))))
entity (cond->> c
(not= "*" c)
(q)
t
(str (q t) "."))))
(str (q t) "."))
suspicious #";"]
(when-not *allow-suspicious-entities*
(when (re-find suspicious entity)
(throw (ex-info (str "suspicious character found in entity: " entity)
{:disallowed suspicious}))))
entity))
(comment
(for [v [:foo-bar 'foo-bar "foo-bar"

34
src/honey/sql/helpers.cljc
View file

@ -592,12 +592,26 @@
(defn where
"Accepts one or more SQL expressions (conditions) and
combines them with AND:
combines them with AND (by default):
(where [:= :status 0] [:<> :task \"backup\"])
or:
(where :and [:= :status 0] [:<> :task \"backup\"])
Produces: WHERE (status = ?) AND (task <> ?)
Parameters: 0 \"backup\""
Parameters: 0 \"backup\"
For a single expression, the brackets can be omitted:
(where := :status 0) ; same as (where [:= :status 0])
With multiple expressions, the conjunction may be
specified as a leading symbol:
(where :or [:= :status 0] [:= :task \"stop\"])
Produces: WHERE (status = 0) OR (task = ?)
Parameters: 0 \"stop\""
[& exprs]
(generic :where exprs))
@ -615,12 +629,24 @@
(defn having
"Like `where`, accepts one or more SQL expressions
(conditions) and combines them with AND:
(conditions) and combines them with AND (by default):
(having [:> :count 0] [:<> :name nil])
or:
(having :and [:> :count 0] [:<> :name nil])
Produces: HAVING (count > ?) AND (name IS NOT NULL)
Parameters: 0"
Parameters: 0
(having :> :count 0)
Produces: HAVING count > ?
Parameters: 0
(having :or [:> :count 0] [:= :name \"\"])
Produces: HAVING (count > ?) OR (name = ?)
Parameters: 0 \"\""
[& exprs]
(generic :having exprs))

61
test/honey/sql/helpers_test.cljc
View file

@ -20,6 +20,7 @@
with-data]]))
(deftest test-select
(testing "large helper expression"
(let [m1 (-> (with [:cte (-> (select :*)
(from :example)
(where [:= :example-column 0]))])
@ -79,6 +80,66 @@
{:params {:param1 "gabba" :param2 2}
;; to enable :lock
:dialect :mysql :quoted false}))))))
(testing "large helper expression with simplified where"
(let [m1 (-> (with [:cte (-> (select :*)
(from :example)
(where := :example-column 0))])
(select-distinct :f.* :b.baz :c.quux [:b.bla "bla-bla"]
:%now [[:raw "@x := 10"]])
(from [:foo :f] [:baz :b])
(join :draq [:= :f.b :draq.x])
(left-join [:clod :c] [:= :f.a :c.d])
(right-join :bock [:= :bock.z :c.e])
(full-join :beck [:= :beck.x :c.y])
(where :or
[:and [:= :f.a "bort"] [:not= :b.baz :?param1]]
[:and [:< 1 2] [:< 2 3]]
[:in :f.e [1 [:param :param2] 3]]
[:between :f.e 10 20])
(group-by :f.a)
(having :< 0 :f.e)
(order-by [:b.baz :desc] :c.quux [:f.a :nulls-first])
(limit 50)
(offset 10))
m2 {:with [[:cte {:select [:*]
:from [:example]
:where [:= :example-column 0]}]]
:select-distinct [:f.* :b.baz :c.quux [:b.bla "bla-bla"]
:%now [[:raw "@x := 10"]]]
:from [[:foo :f] [:baz :b]]
:join [:draq [:= :f.b :draq.x]]
:left-join [[:clod :c] [:= :f.a :c.d]]
:right-join [:bock [:= :bock.z :c.e]]
:full-join [:beck [:= :beck.x :c.y]]
:where [:or
[:and [:= :f.a "bort"] [:not= :b.baz :?param1]]
[:and [:< 1 2] [:< 2 3]]
[:in :f.e [1 [:param :param2] 3]]
[:between :f.e 10 20]]
:group-by [:f.a]
:having [:< 0 :f.e]
:order-by [[:b.baz :desc] :c.quux [:f.a :nulls-first]]
:limit 50
:offset 10}]
(testing "Various construction methods are consistent"
(is (= m1 m2)))
(testing "SQL data formats correctly"
(is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ?"
0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10]
(sql/format m1 {:params {:param1 "gabba" :param2 2}}))))
#?(:clj (testing "SQL data prints and reads correctly"
(is (= m1 (read-string (pr-str m1))))))
#_(testing "SQL data formats correctly with alternate param naming"
(is (= (sql/format m1 {:params {:param1 "gabba" :param2 2}})
["WITH cte AS (SELECT * FROM example WHERE example_column = $1) SELECT DISTINCT f.*, b.baz, c.quux, b.bla \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = $2) AND (b.baz <> $3)) OR (($4 < $5) AND ($6 < $7)) OR (f.e IN ($8, $9, $10)) OR f.e BETWEEN $11 AND $12 GROUP BY f.a HAVING $13 < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT $14 OFFSET $15"
0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10])))
(testing "Locking"
(is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS `bla-bla`, NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ? LOCK IN SHARE MODE"
0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10]
(sql/format (assoc m1 :lock [:in-share-mode])
{:params {:param1 "gabba" :param2 2}
;; to enable :lock
:dialect :mysql :quoted false})))))))
(deftest select-top-tests
(testing "Basic TOP syntax"

14
test/honey/sql_test.cljc
View file

@ -649,6 +649,20 @@ ORDER BY id = ? DESC
:values [{:name name
:enabled enabled}]})))))
(deftest issue-316-test
(testing "SQL injection via keyword is detected"
(let [sort-column "foo; select * from users"]
(try
(-> {:select [:foo :bar]
:from [:mytable]
:order-by [(keyword sort-column)]}
(format))
(is false "; not detected in entity!")
(catch #?(:clj Throwable :cljs :default) e
(is (:disallowed (ex-data e))))))
;; should not produce: ["SELECT foo, bar FROM mytable ORDER BY foo; select * from users"]
))
(deftest issue-319-test
(testing "that registering a clause is idempotent"
(is (= ["FOO"]