0

I have the following query in my python code:

query = """SELECT id
           FROM dings_archive
           WHERE doorbot_id IN {doorbot_ids};
        """

query = query.format(doorbot_ids=tuple(doorbot_ids))

doorbot_ids has type List[int] and I have a problem when my array contains just 1 element, because after query.format I have the following query:

SELECT id
FROM dings_archive
WHERE doorbot_id IN (123,);

As you see I have a bug because of comma , at the end. How to fix this?

  • Maybe you should do `query.format(doorbot_ids=", ".join(doorbot_ids))` (add the parenthesis manually in `query`) – RompePC Jun 14 '17 at 09:00
  • its because door_bots is a tuple – Eliethesaiyan Jun 14 '17 at 09:00
  • 1
    @RompePC that's a bad idea, as people could potentially inject SQL code and wreak havok. – Nils Werner Jun 14 '17 at 09:01
  • take repr from the list or tuple and do some regex? – suvy Jun 14 '17 at 09:05
  • @Nils Werner The final SQL would be the same in both cases (except for the final comma), `(123)`, I don't see why it could wreak havok. If it fails in one, it fails in the other. Another case is that he/she is escaping values when doing it. – RompePC Jun 14 '17 at 09:06

3 Answers3

3

To prevent SQL injection you should never use automatic casting of tuple to strings (what you are currently doing), or manually join a list of values before passing it to the driver.

You are currently making use of the fact that a tuple cast to string yields reasonable results:

str((1, 2))
# '(1, 2)'

but as you have noticed, single element tuples yield syntax errors:

str((1, ))
# '(1,)'

Some have suggested to concatenate the list manually:

','.join((1, 2))
# '1,2'

which would also work for single element tuples:

','.join((1,))
# '1'

however this opens the door to SQL injections, as the comma is in fact a control statement for MySQL, not just a value. So if somebody was able to sneak control statements in your list, they could inject code:

','.join((1,'1) OR 1=1 --'))
# '1,1) OR 1=1 --'

To be safe against such attacks you should prepare a query with exactly as many placeholders as elements in your list of id's:

doorbot_ids = [1, 2]

query = "SELECT id FROM dings_archive WHERE doorbot_id IN ({});".format(','.join(['%s'] * len(doorbot_ids)))

This returns a new query with exactly as many placeholders as required:

'SELECT id FROM dings_archive WHERE doorbot_id IN (%s,%s);'

and each placeholder is then filled with a value:

query = cursor.execute(query, doorbot_ids)
Nils Werner
  • 34,832
  • 7
  • 76
  • 98
  • what about this `query = query.format(doorbot_ids ='(%s)' % ', '.join(map(repr, doorbot_ids)))` ? Is it any difference between time complexity with your algo? –  Jun 14 '17 at 09:23
  • also this `query = query.format(doorbot_ids)` didn't filled placeholder with a value. After the execution I got `SELECT id FROM dings_archive WHERE doorbot_id IN (%s);` –  Jun 14 '17 at 09:37
  • Ah sorry, I mistyped. You should obviously use `cursor.execute()`. – Nils Werner Jun 14 '17 at 10:00
  • The first question is answered in my answer. Don't do this. – Nils Werner Jun 14 '17 at 10:39
  • This will fail for one element list as it will put a comma at the end. – ar-siddiqui Sep 13 '21 at 23:00
0

Okay, thinking outside the box here: If you have to use a tuple in a WHERE IN clause you could always test for the length of the tuple. if the tuple length == 1, convert it into a list using list(tuple_object), append the list with the value at index 0 then convert it back to tuple! Steps: 1. the_single_value_tuple=('Salim',) 2. a_list= list(the_single_value_tuple) 3. return tuple(a_list.append(a_list[0])

Denis Xavier
  • 1
  • 2
-1

Do not convert list to tuple, use it directly:

query = """SELECT id
           FROM dings_archive
           WHERE doorbot_id = any(%s);
        """
...
cursor.mogrify(query, (doorbot_ids,))
Abelisto
  • 14,826
  • 2
  • 33
  • 41