seancorfield / next-jdbc

A modern low-level Clojure wrapper for JDBC-based access to databases.
https://cljdoc.org/d/com.github.seancorfield/next.jdbc/
Eclipse Public License 1.0
750 stars 89 forks source link

"ansi" (and by extenstion "posgresql" and "oracle") quoting is incomplete #246

Closed iarenaza closed 1 year ago

iarenaza commented 1 year ago

Describe the bug For a a number of reasons, our application needs to use end-user supplied values for column names in the database. In order to make sure the supplied names can be used as column names without problems, and that they don't collide with SQL key words, we need to use them as "delimited identifiers" (using ANSI SQL lingo).

We are using Postgresql, so the natural way of making sure everything works as expected in this case, is to use next.jdbc.quoted/postgres (directly, or indirectly through next.jdbc.quoted/schema) as the value for :column-fn option, to quote the column names.

But it turns out that ansi quoting (which is what both postgres and oracle quoting functions use underneath) doesn't correctly deal with names that include double quotes inside them (yeah, we know, as I said, before, they users need to be able to choose those names, that is something that was imposed on us by $EXTERNAL_POWERS).

According to the ANSI SQL 99 standard (the only one we could get our hands on without paying a lot of €€):

A is a character string, up to 128 characters long, surrounded by a pair of double quote marks. (The delimiting double quotes aren’t part of the , so they’re not included in the calculation of its size.) Two consecutive double quotes within the character string (i.e., “”) represent one double quote mark; together, they count as one character when calculating the size of the .

If our interpretation of the above is right, that means every single double quote mark in the original value (what the end-user provided) should produce a corresponding double quote mark in the quoted value.

But the ansi/posgres/oracle quoting functions just add the "surrounding" pair of quote marks, and don't deal with any quote marks that may be part of the value to quote.

To Reproduce

The following piece of code, when evaluated in the REPL, should return true. But it returns false.

user> (require '[next.jdbc.quoted :as quoted])
nil
user> (let [col-name "col name with question mark \" in it"]
        (= "\"col name with question mark \"\" in it\""
           (quoted/ansi col-name)))
false
user>

Expected behavior

As stated before, the above piece of code should return true.

project.clj/deps.edn

project.clj, trimmed to the relevant parts.

(defproject biotz-webapp "0.1.0-SNAPSHOT"
  :min-lein-version "2.9.8"
  :dependencies [[org.clojure/clojure "1.11.0"]
                 .....
                 [dev.gethop/database.sql.hikaricp "0.4.1"]
                 [org.postgresql/postgresql "42.5.3"]
                 [com.github.seancorfield/next.jdbc "1.3.862"]
                 [com.github.seancorfield/honeysql "2.4.972"]
                 [camel-snake-kebab "0.4.3"]
                 .....]
....)

Environment (please complete the following information):

seancorfield commented 1 year ago

Ah, good catch! This is something I added in HoneySQ to deal with quoted entities that include the closing quote (which can be other characters in other dialects) but it hasn't been ported to next.jdbc yet:

https://github.com/seancorfield/honeysql/blob/develop/src/honey/sql.cljc#L92-L95

seancorfield commented 1 year ago

And, to be clear, this can affect MySQL and SQL Server quoting too.