CurtTilmes / raku-dbsqlite

SQLite access for Raku
10 stars 4 forks source link

prepare statements don't protect apostrophes #4

Closed tbrowder closed 5 years ago

tbrowder commented 5 years ago

I thought by using a prepared statement with ? placeholders I wouldn't have to be concerned about taking care of embedded apostrophes. Something like this doesn't work as expected:

my $last = "O'Connor";
my $sth = $s.db.prepare('insert into person_tbl (last) values (?)');
$sth.execute($last);
BOOM
CurtTilmes commented 5 years ago

I'm going to need more of a working example of the failure -- here's what I tried:

use DB::SQLite;                                                                 
my $s = DB::SQLite.new(filename => 'this.db');                                  
$s.execute('create table person_tbl ( last text )');                            
my $last = "O'Connor";                                                          
my $sth = $s.db.prepare('insert into person_tbl (last) values (?)');            
$sth.execute($last);                                                            
say $s.query('select * from person_tbl').hashes;                                

It prints out this:

({last => O'Connor})
tbrowder commented 5 years ago

Curt that looks good and I probably made another mistake. I'll try again.

I really like this module--much easier to use than others I've tried.

I won't be able to reply for a while.

UPDATE

It was my error in an insert statement early in my program development. Your module works great! Sorry for the bad call.

tbrowder commented 5 years ago

Strange, but the error I get is:

DEBUG: key = <|oconnor|michael|michaeloconnor2@gmail.com|>
NOTE: found person key '|oconnor|michael|michaeloconnor2@gmail.com|' already entered.
DEBUG: key = <|o'connor|patrick|pocon1932@aol.com|>
near "connor": syntax error
  in method prepare-nocache at /usr/local/rakudo.d/share/perl6/site/sources/E0C8745F742CD981123D6FBD6B9BE68BA67C0A70 (DB::SQLite::Connection) line 17
  in method prepare at /usr/local/rakudo.d/share/perl6/site/sources/31DAC65B428E3BE7CAFC630257CA1C4190EE6AC3 (DB::Connection) line 45
  in method query at /usr/local/rakudo.d/share/perl6/site/sources/31DAC65B428E3BE7CAFC630257CA1C4190EE6AC3 (DB::Connection) line 50
  in method query at /usr/local/rakudo.d/share/perl6/site/sources/A79D993708E8048217C956D0437DC98D02AF9B83 (DB) line 23
  in block <unit> at ./load-db-sql.p6 line 151

I'm trying to golf it down a bit, using two names.

When I dump the person table I get:

sqlite> .dump person
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
CREATE TABLE person (
    key   VARCHAR(255) PRIMARY KEY UNIQUE NOT NULL,
    last  VARCHAR(255) NOT NULL,
    first VARCHAR(255) NOT NULL,
    notes VARCHAR(255)
);
INSERT INTO person VALUES('|oconnor|michael|michaeloconnor2@gmail.com|','OConnor','Michael',NULL);
COMMIT;

When I dump the email table I get:

sqlite> .dump email 
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
CREATE TABLE email (
    email      VARCHAR(255) NOT NULL,
    notes      VARCHAR(255),
    status     INTEGER,
    person_id  VARCHAR(255) NOT NULL,
    CONSTRAINT key PRIMARY KEY (email, person_id)
);
INSERT INTO email VALUES('michaeloconnor2@gmail.com',NULL,NULL,'|oconnor|michael|michaeloconnor2@gmail.com|');
COMMIT;

If I remove the apostrophe from the keys all works well. and the apostrophe in the last name shows properly and is taken care of as expected.

CurtTilmes commented 5 years ago

Print the exact arguments to both prepare() and execute just before you call them.

tbrowder commented 5 years ago

Okay, it will be a while, though. Thanks for looking at this.

CurtTilmes commented 5 years ago

I would immediately suspect a mismatch in the placeholders and arguments.

DBIish uses its own placeholders and parses the statements itself checking all of that. This takes time, but allows it to provide better error messages. It also prevents it from using all the fancy placeholder options that sqlite offers.

DB::SQLite just passes things through to sqlite exactly as you specify them without parsing or validation. This is the fastest and most efficient, and allows you the complete power of sqlite, at the cost of obscure error messages if you get it wrong.

We made different tradeoffs.

tbrowder commented 5 years ago

I have carefully checked the code for the prepare and execute statements:

my $sthP = $db.prepare('insert into person (key, last, first) values (?, ?, ?)');

Execution:

$sthP.execute($key, $last, $first);

where, from a debugging "note" just before the failed execute, I get:

$key   = <|o'connor|patrick|pocon1932@aol.com|>
$last  = <O'Connor>
$first = <Patrick>

Note if I remove the apostrophe from the offending key, the program runs fine and the tables have the proper entries including the apostrophe in Patrick O'Connor's last name.

tbrowder commented 5 years ago

I've golfed all down to a single, short script and it shows no problem--looks like MY problem, but I'll keep this open until I isolate the bug.

tbrowder commented 5 years ago

Getting closer to the problem, but I still don't know exactly what triggered the failure. I had a non-prepared select query in a loop between prepared insert queries to determine the existence of a person before attempting an insert. The apparent fix was to properly determine person row existence before any attempted insert.

I'm still debugging though so maybe I can suggest a doc change to help db novices.

CurtTilmes commented 5 years ago

That's ok, but use a different handle for the different queries. Either use $s.query() to get a new handle each time, or call $s.db for each purpose.

tbrowder commented 5 years ago

Curt, I am getting a "handle" on the problem, but could you give an example of the canonical way to check for the presence of a unique key in a table with DB::SQLlite? Reason I ask is when I do that with a prepared query and execute it I get a good response (either Nil or 1), but when I try a dynamic query at the same place with $s.db or $s it gives that weird error shown above.

CurtTilmes commented 5 years ago

I dunno -- maybe:

$s.query('select 1 from foo where key = ?', $value).value
tbrowder commented 5 years ago

From my experiments I see I have been using formats for non-prepared queries that are double-quoted strings with embedded interpreted args which resulted in my problems.

Using the in-place prepared form (as your example above) with the ? place holders and adding the args separately in the same Perl 6 statement works reliably as DB::SQLite handles apostrophes in strings properly behind the scenes.

Bottom line: Let DB::SQLite be your friend and use it correctly!