launchpad-reviewers team mailing list archive

[Adam Collard <adam.collard@xxxxxxxxxxxxx>]

Looks OK - just the SQL injection issue jumps out (see inline)

Diff comments:

> 
> === modified file 'src/lazr/postgresql/upgrade.py'
> --- src/lazr/postgresql/upgrade.py	2017-04-23 23:44:51 +0000
> +++ src/lazr/postgresql/upgrade.py	2017-07-27 15:36:33 +0000
> @@ -135,16 +135,16 @@
>      """Cannot apply some patch."""
>  
>  
> -def _table_exists(con, tablename):
> +def _table_exists(con, tablename, schema):
>      """Return True if tablename exists."""
>      cur = con.cursor()
>      cur.execute("""
>          SELECT EXISTS (
>              SELECT TRUE FROM information_schema.tables
>              WHERE
> -                table_schema='public'
> +                table_schema='%s'
>                  AND table_name='%s')
> -            """ % tablename)
> +            """ % (schema, tablename))

hmm, Little Bobby Tables says hi?

(this should be parameterised to avoid SQL injection attack, I know we don't expect this to come from user-data but from a defence-in-depth angle..)

>      return cur.fetchone()[0]
>  
>  


-- 
https://code.launchpad.net/~bloodearnest/lazr-postgresql/schema/+merge/328173
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bloodearnest/lazr-postgresql/schema into lp:lazr-postgresql.