SQL errors when used with Postgres

[anonymous]

The field name "end" seems to be a reserved word in Postgres, so it needs to be quoted, otherwise I get SQL errors when adding events. Putting double quotes around all its instances in model.py fixes it.

[Ryan J Ollos]

Yeah, it is a ​reserved word. For cross-db compatibility, quote it using db.quote() from the Trac database API.

[lucid]

Thanks for the hint to db.quote(). Could anyone review or test the attached patch with Postgres?

Quoting the column name makes the code quite a bit uglier. I guess at some point the column could be renamed in a DB upgrade.

[Jun Omae]

The patch seems to be good, but the plugin has SQL injection in the plans argument of select_by_plans_and_time(). We should use Parameter passing in ​trac:wiki:TracDev/DatabaseApi#Parameterpassing.

Change

        with env.db_query as db:
            plan_sql = ','.join(["'%s'" % plan for plan in plans])
            rows = db("""
                    SELECT id, plan, title, start, %s
                    FROM weekplan
                    WHERE plan in (%s) AND start < %%s AND %s > %%s
                    """ % (db.quote('end'), plan_sql, db.quote('end')), (to_utimestamp(end), to_utimestamp(start)))

to

        if not plans:
            return []
        with env.db_query as db:
            plan_holder = ','.join(['%s'] * len(plans))
            rows = db("""
                    SELECT id, plan, title, start, %s
                    FROM weekplan
                    WHERE plan IN (%s) AND start < %%s AND %s > %%s
                    """ % (db.quote('end'), plan_holder, db.quote('end')),
                    list(plans) + [to_utimestamp(end), to_utimestamp(start)])

[lucid]

In 14780:

WeekPlanPlugin: PostgreSQL compatibility: quote "end" table name
Avoid SQL injection
Bump version to 1.2
(fix #12423)

[lucid]

Thanks for the review!

[lucid]

In 14781:

TimeTrackingPlugin: Avoid SQL injection
Bump version to 1.1
(see #12423)

[Peter Suter]

In 14782:

CardsPlugin: Avoid SQL injection
Bump version to 1.1
(see #12423)