3 Questions involving Python and SQLite

Question

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

Some corrections to your code for you: use `if isinstance(entry, int) or isinstance(entry, str)`. Or at least `if vartype in [ int, str ]`. `if vartype == int or str` won't do what you expect. Also remember to call functions with `()` when you don't want to pass any parameters, eg. `return self.memcursor.fetchall())`. Otherwise you're returning a reference to the function rather than calling it. — Robie Basak, Nov 06 '10 at 19:49
Also in Python 2 it is better to use basestring instead of str as basestring is the parent class of both str and unicode. Similarly you may be passed a long or int so look for both. — Roger Binns, Nov 20 '10 at 04:53

score 3 · Answer 1 · answered Nov 06 '10 at 19:21

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.

score 0 · Answer 2 · answered Nov 20 '10 at 05:24

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

3 Questions involving Python and SQLite

2 Answers2