[Tutor] Proper SQLite cursor handling?

[Cameron Simpson]

On 05Jul2021 21:27, boB Stepp <robertvstepp at gmail.com> wrote:
>> Also, very importantly, this is the wrong way to insert values into 
>> an
>> SQL query.  The format field {self.game_name} will just get the bare
>> game name inserted raw into the SQL, which will likely be a syntax error
>> or be misread as a column name. See this document:
>>
>>     https://xkcd.com/327/
>
>I am aware of this, but game_name is supposed to be fully vetted by
>this point and would map str in Python to TEXT in that column.  Can
>there still be issues with this getting inserted into the table?

Well, you're not even quoting it. So you'd be making a piece of SQL 
like:

    SELECT * FROM games WHERE game_name = my_game_name

That tries to compare two columns, "game_name" and "my_game_name", and 
there is no my_game_name column.  You presumably want:

    SELECT * FROM games WHERE game_name = 'my_game_name'

But that takes you down the rabbit hole of correct SQL quoting and 
always remembering to do it. Got a quote inside you string? Etc.

Never trust you string to be "vetted". This is a syntax issue. You 
_should_ be able to support a game name like "; drop table ..." without 
issue or risk. (You might forbid it, but it shouldn't represent an SQL 
injection risk if you don't.)

The placeholder approach in fact often separates the SQL from the 
parameters in the network protocol, so it isn't even trying to insert 
quoted values into the SQL - it will be sending some 
SQL-with-placeholders and sepearate protocol fields for the values to go 
in the placeholders. What happens in SQLite itself I'm not sure.

But the placeholder approach means this is not your problem! The db 
driver / python module handles that. _Always_ use placeholders!

>> >        game_data = cur.fetchall()[0]
>>
>> You ccould just fetchone().
>
>In retrospect I don't know why I didn't use fetchone() as there will
>always only be one row returned as game_name must be unique in that
>table.
>
>> >        if not game_data:
>>
>> This test is too late. game_data won't be empty or false, you'll get an
>> exception during the fetch. You could go:
>
>I beg to differ here.  This test is meant to cover the "new game" case
>where there is not an entry yet for that game.  I tested this in the
>interpreter and as I originally wrote it returns an empty list.

I suggest that this line:

    game_data = cur.fetchall()[0]

will raise an index error when fetchall() returns an empty list. That is 
why the test is too late in the code you had.

Cheers,
Cameron Simpson <cs at cskk.id.au>