Reputation: 21
I'm creating functions for use with a game server. This server uses plugins. I have these functions which use an SQLite database, along with apsw to retrieve items stored by another function. I have 3 questions on this.
Question One: I keep getting the error "SQLError: near "?": syntax error" Since my statement features multiple ?
, it's proving hard to track down what is exactly wrong. So what is wrong?
Question Two: I know about SQL-Injection, but these functions only take input from the runner of the script, and the only stuff he would be damaging is his own. Even so, is there an easy way to make this SQL-injection proof?
Question Three: Is there any way to make this function more efficient?
Here's what it looks like now:
def readdb(self,entry,column,returncolumn = "id,matbefore,matafter,name,date"):
self.memwrite
if isinstance(entry, int) or isinstance(entry, str):
statement = 'SELECT {0} FROM main WHERE {1} IN {2}'.format(returncolumn,column,entry)
self.memcursor.execute(statement)
blockinfo = self.memcursor.fetchall()
return(blockinfo)
if isinstance(entry, tuple) or isinstance(entry, list):
statement = '''SELECT {0} FROM main WHERE {1} IN (%s)'''.format(returncolumn,column)
self.memcursor.execute(statement % ("?," * len(entry))[:-1], entry)
blockinfo = self.memcursor.fetchall()
return(blockinfo
Upvotes: 2
Views: 339
Reputation: 3348
Use names to get more informative error messages. For example I deliberately left out a comma with this:
cur.execute("select ? ?,?", (1,2,3))
SQLError: near "?": syntax error
Now with names:
cur.execute("select :1 :2,:3", (1,2,3))
SQLError: near ":2": syntax error
If you have a lot of bindings I'd recommend you switch to the named bindings style and pass a dictionary for the bindings themselves.
cur.execute("select :artist, :painting, :date",
{"artist": "Monster", "painting": "The Duck", "date": "10/10/2010" })
You can only use bindings for values, not for column or table names. There are several possible approaches. Although SQLite supports arbitrary column/table names, you can require that they are only ASCII alphanumeric text. If you want to be less restrictive then you need to quote the names. Use square brackets around a name that has double quotes in it and double quotes around a name that has square brackets in it. A name can't have both.
The alternative to all that is using the authorizer mechanism. See Connection.setauthorizer for the API and a pointer to an example. In brief your callback is called with the actions that will be taken so for example you can reject anything that would write to the database.
In terms of efficiency, you can improve things depending on how the caller uses the results. Cursors are very cheap. There is no need to try to reuse the same one over and over again and doing so can lead to subtle errors. SQLite only gets the next row of a result when you ask for it. By using fetchall you insist of building a list of all the results. If the list could be large or if you may stop part way through then just return db.cursor().execute("... query ...")
. The caller should then use your function to iterate:
for id,matbefore,matafter,name,date in readdb(...):
... do something ...
In your place I would just junk this readdb function completely as it doesn't add any value and write queries directly:
for id,foo,bar in db.cursor().execute("select id,foo,bar from .... where ...."):
... do something ...
Your coding style indicates that you are fairly new to Python. I strongly recommend looking up iterators and generators. It is a far simpler coding style producing and consuming results as they are needed.
BTW this SQL creates a table with a zero length name, and columns named double quote and semicolon. SQLite functions just fine, but don't do this :-) It is however useful for testing.
create table "" (["], ";");
Disclosure: I am the APSW author
Upvotes: 0
Reputation: 76876
This is funny (read on to learn why).
The first statement you have actually uses the value binding mechanism of the sqlite3
-module (I assume that is what you use). Hence, the *
(which is the default column) gets escaped, making the statement invalid. This is SQL-injection proof, and your own code tries to inject SQL (see the funny now?).
The second time you use Pythons string replacement in order to build the query string, which is not SQL-injection proof.
Upvotes: 3