in reply to Re: SQLite: INSERT into a unique column and retrieve rowid
in thread SQLite: INSERT into a unique column and retrieve rowid

It does seem to me a lot of code

It could be streamlined for sure, particularly if you make the database enforce the uniqueness constraint (as in the OP) and just trap the exceptions. But really the worrying part is that $table and $column are SQL injections just waiting to happen.

Also the fact that you've used a state hash to hold the prepared statement handles means that these variables only have relevance on the first invocation, which is an unusual situation. eg. call it once on table foo and then again on table bar and the second one will still use table foo. That's confusing at best. (edit: ignore this part)


🦛

Replies are listed 'Best First'.
Re^3: SQLite: INSERT into a unique column and retrieve rowid
by ibm1620 (Hermit) on May 05, 2024 at 20:42 UTC
    Regarding SQL injection -- had to look it up. Is my use of variables $table and $column in those prepared SELECTs necessarily dangerous? I can see how they could be if an enduser were prompted for them, but that's not how I intend to populate them. It's for a specific application with specific table and column names that are hardcoded into the program. As I say, I'm very much a novice at SQL apps. Maybe there's more to this than I'm aware of ... ? Thanks.
      I can see how they could be if an enduser were prompted for them, but that's not how I intend to populate them.

      Proverbially the road to Hell is paved with good intentions. The trouble is that it becomes harder and harder to keep track of all the possible routes through your code as it grows and the chances, however slight, of allowing external input to the variables can exist.

      At the very least you should sanitise these variables as close to their point of use as possible, ie: within the subroutine. So, something like this:

      sub insert_unique( $table, $column, $value ) { state %sths; if ( !exists $sths{$table} ) { die "Bad table '$table'" unless $table =~ /^[a-z0-9_]+$/; die "Bad column '$column'" unless $column =~ /^[a-z0-9_]+$/;

      Better, use Carp::confess instead of die. Better still, use a hash of acceptable tables to further nail things down:

      sub insert_unique( $table, $column, $value ) { use Carp; state %sths; state %good_table = map { $_ => 1 } qw/foo bar baz/; if ( !exists $sths{$table} ) { confess "Bad table '$table'" unless $good_table{$table};

      By doing this, and the same for $column, you are limiting the ability of an external agent (or your own fat fingers) to operate on parts of the database to which there should not be access.

      Oblig xkcd


      🦛

Re^3: SQLite: INSERT into a unique column and retrieve rowid
by ibm1620 (Hermit) on May 05, 2024 at 01:47 UTC
    I don't understand your comment about the state hash. I copied this subroutine into my actual program, which calls insert_unique() repeatedly for six different tables, and it worked correctly.

      You are quite right. I had somehow missed that you were keying the hash on the table and testing for this one level down. Mea culpa.


      🦛