import strutils, sequtils
type
SQL = distinct string
proc properQuote(s: string): SQL =
return SQL(s)
proc `%` (frmt: SQL, values: openarray[string]): SQL =
let v = values.mapIt(properQuote(it))
type StrSeq = seq[string]
result = SQL(string(frmt) % StrSeq(v))
import odbc
#[ ConnectionEngine ]#
type ConnectionEngine = ref object of RootObj
database: string
connection: ODBCConnection
method start(self: ConnectionEngine) {.base.} =
self.connection = newODBCConnection()
let c = self.connection
c.driver = "ODBC Driver 17 for SQL Server"
c.host = "magic"
c.port = 1410
c.database = "DataWarehouse"
c.integratedSecurity = false
c.userName = "sa"
c.password = "Admin3970#xx"
c.authenticationType = "Plain"
c.connectionType = "Direct"
if not self.connection.connect:
echo "Could not connect to database."
#[ ResultEngine ]#
type ResultEngine = ref object of RootObj
query: SQLQuery
method getResult(self: ResultEngine, connectionEngine: ConnectionEngine, script: string): SQLResults {.base.} =
self.query = newQuery(connectionEngine.connection)
self.query.statement = script
self.query.open
echo script
return self.query.fetch
Hi @iwcoetzer,
Araq is correct, your properQuote doesn't do anything. To get a big portion of SQL injection safety, always use parameterised queries. Never combine strings with SQL. As soon as you start combining text for queries, you're in trouble.
For example, this is a bad idea:
proc `%` (frmt: SQL, values: openarray[string]): SQL =
let v = values.mapIt(properQuote(it))
type StrSeq = seq[string]
result = SQL(string(frmt) % StrSeq(v))
Ideally the query should be able to be run without being changed at all, and only parameters changed. As well as better sanitation, this also means your queries don't need to be prepared each time they're executed and a whole host of other performance bonuses. For example, each preparation requires tokenising and sending the query to the server, processing the queries (without running them), then waiting for a response back! Additionally, parameterised queries will tell you if there is some type issue relating to parameters and the table way earlier than if you're combining text.
The obdc library has support for handling parameterised queries natively and therefore avoid injection. What I would do personally is update the query with paramters directly:
#[ prepare a SQL script ]#
query.statement = "SELECT TOP 100 * FROM $1 (NOLOCK) WHERE EffectivePeriod >= ?effectivePeriod" % ["dbo.DimContract"]
query.params["effectivePeriod"] = "201901" # Or maybe this wants to be an actual date rather than a string?
Note that there is still a string concatination for the table name. In this case you have two options:
2) Use dynamic SQL To select from a table, and pass the table name as a parameter as above to the dynamic SQL. This is, as I understand it, the 'preferred' approach, though it does seem a tad unweildy, it is very powerful. A quick tutorial on how to do this is here: https://www.sqlservertutorial.net/sql-server-stored-procedures/sql-server-dynamic-sql/ So, in their example the line
SET @table = N'production.products';
SET @table = ?myTableName;You still need to be careful to only use parameterised data in dynamic SQL though, so don't pass in strings to be combined into dynamic SQL unless there is no way there's user input.
To make things a bit easier, I've recently added fire-and-forget queries to odbc so you can use type checked parameters for instant queries if you don't want to create a query object but want type safe parameter checks.
let results = connection.dbq("SELECT ?value1, ?value2", [("value1", 17), ("value2", 24)]
@Araq: I wrote the odbc library for a few reasons:
Yes some effort is made to escape strings, but this is to me wholey worse than the guaranteed parameter isolation that native odbc parameters give you. There is absolutely no way to escape parameterised queries when used properly.
Finally - and most importantly as I've detailed above, db_odbc doesn't use parameterisation and instead just uses string concatination, so injections are still possible: https://github.com/nim-lang/Nim/blob/b6924383df63c91f0ad6baf63d0b1aa84f9329b7/lib/impure/db_odbc.nim#L194
How so?
And mapping values to (named) parameters is the easy stuff, often you need to generate the query based on some filter setting and then it's still all the same bad string based SQL construction. Many APIs do not offer to construct the SQL parse tree...
method getresultTwo(self: ResultEngine, connectionEngine: ConnectionEngine): SQLResults {.base.} =
self.query = newQuery(connectionEngine.connection)
self.query.statement = ("SELECT * FROM dbo.Contract WHERE EffectivePeriod >= ?value1", [("value1", 201906)])
self.query.open
return self.query.fetch
I was trying to do this, but an error "first type mismatch at position: 2" ?
This also did not work ...
method getresultThree(self: ResultEngine, connectionEngine: ConnectionEngine): SQLResults {.base.} =
let results = connectionEngine.connection.dbq("SELECT * FROM dbo.DimContract WHERE EffectivePeriod >= ?value1", [("value1", 201906)])
return results
@iwcoetzer Whoops that will teach me to not add a test case for new functionality (my internal checks passed because my connection is called con!)
I added a fix for dbq and a test for the latest version.
Your first version can't work because statement is a string so can't take the extra parameters. The second version should work now though :)