Skip to content

Commit 898c4b6

Browse files
aviauaviau
authored
Merge pull request influxdata#361 from saaros/quote_ident (Thanks @saaros!)
Properly quote all identifiers and literals in InfluxDBClient
2 parents bdc8875 + 97952ba commit 898c4b6

File tree

5 files changed

+60
-45
lines changed

5 files changed

+60
-45
lines changed

‎influxdb/client.py

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import requests.exceptions
1414
from sys import version_info
1515

16-
from influxdb.line_protocol import make_lines
16+
from influxdb.line_protocol import make_lines, quote_ident, quote_literal
1717
from influxdb.resultset import ResultSet
1818
from .exceptions import InfluxDBClientError
1919
from .exceptions import InfluxDBServerError
@@ -552,8 +552,8 @@ def alter_retention_policy(self, name, database=None,
552552
should be set. Otherwise the operation will fail.
553553
"""
554554
query_string = (
555-
"ALTER RETENTION POLICY \"{0}\" ON \"{1}\""
556-
).format(name, database or self._database)
555+
"ALTER RETENTION POLICY {0} ON {1}"
556+
).format(quote_ident(name), quote_ident(database or self._database))
557557
if duration:
558558
query_string += " DURATION {0}".format(duration)
559559
if replication:
@@ -573,8 +573,8 @@ def drop_retention_policy(self, name, database=None):
573573
:type database: str
574574
"""
575575
query_string = (
576-
"DROP RETENTION POLICY \"{0}\" ON \"{1}\""
577-
).format(name, database or self._database)
576+
"DROP RETENTION POLICY {0} ON {1}"
577+
).format(quote_ident(name), quote_ident(database or self._database))
578578
self.query(query_string)
579579

580580
def get_list_retention_policies(self, database=None):
@@ -631,8 +631,8 @@ def create_user(self, username, password, admin=False):
631631
privileges or not
632632
:type admin: boolean
633633
"""
634-
text = "CREATE USER \"{0}\" WITH PASSWORD '{1}'".format(username,
635-
password)
634+
text = "CREATE USER {0} WITH PASSWORD {1}".format(
635+
quote_ident(username), quote_literal(password))
636636
if admin:
637637
text += ' WITH ALL PRIVILEGES'
638638
self.query(text)
@@ -643,7 +643,7 @@ def drop_user(self, username):
643643
:param username: the username to drop
644644
:type username: str
645645
"""
646-
text = "DROP USER {0}".format(username)
646+
text = "DROP USER {0}".format(quote_ident(username))
647647
self.query(text)
648648

649649
def set_user_password(self, username, password):
@@ -654,7 +654,8 @@ def set_user_password(self, username, password):
654654
:param password: the new password for the user
655655
:type password: str
656656
"""
657-
text = "SET PASSWORD FOR {0} = '{1}'".format(username, password)
657+
text = "SET PASSWORD FOR {0} = {1}".format(
658+
quote_ident(username), quote_literal(password))
658659
self.query(text)
659660

660661
def delete_series(self, database=None, measurement=None, tags=None):
@@ -672,11 +673,12 @@ def delete_series(self, database=None, measurement=None, tags=None):
672673
database = database or self._database
673674
query_str = 'DROP SERIES'
674675
if measurement:
675-
query_str += ' FROM "{0}"'.format(measurement)
676+
query_str += ' FROM {0}'.format(quote_ident(measurement))
676677

677678
if tags:
678-
query_str += ' WHERE ' + ' and '.join(["{0}='{1}'".format(k, v)
679-
for k, v in tags.items()])
679+
tag_eq_list = ["{0}={1}".format(quote_ident(k), quote_literal(v))
680+
for k, v in tags.items()]
681+
query_str += ' WHERE ' + ' AND '.join(tag_eq_list)
680682
self.query(query_str, database=database)
681683

682684
def grant_admin_privileges(self, username):
@@ -688,7 +690,7 @@ def grant_admin_privileges(self, username):
688690
.. note:: Only a cluster administrator can create/drop databases
689691
and manage users.
690692
"""
691-
text = "GRANT ALL PRIVILEGES TO {0}".format(username)
693+
text = "GRANT ALL PRIVILEGES TO {0}".format(quote_ident(username))
692694
self.query(text)
693695

694696
def revoke_admin_privileges(self, username):
@@ -700,7 +702,7 @@ def revoke_admin_privileges(self, username):
700702
.. note:: Only a cluster administrator can create/ drop databases
701703
and manage users.
702704
"""
703-
text = "REVOKE ALL PRIVILEGES FROM {0}".format(username)
705+
text = "REVOKE ALL PRIVILEGES FROM {0}".format(quote_ident(username))
704706
self.query(text)
705707

706708
def grant_privilege(self, privilege, database, username):
@@ -715,8 +717,8 @@ def grant_privilege(self, privilege, database, username):
715717
:type username: str
716718
"""
717719
text = "GRANT {0} ON {1} TO {2}".format(privilege,
718-
database,
719-
username)
720+
quote_ident(database),
721+
quote_ident(username))
720722
self.query(text)
721723

722724
def revoke_privilege(self, privilege, database, username):
@@ -731,8 +733,8 @@ def revoke_privilege(self, privilege, database, username):
731733
:type username: str
732734
"""
733735
text = "REVOKE {0} ON {1} FROM {2}".format(privilege,
734-
database,
735-
username)
736+
quote_ident(database),
737+
quote_ident(username))
736738
self.query(text)
737739

738740
def get_list_privileges(self, username):
@@ -754,7 +756,7 @@ def get_list_privileges(self, username):
754756
{u'privilege': u'ALL PRIVILEGES', u'database': u'db2'},
755757
{u'privilege': u'NO PRIVILEGES', u'database': u'db3'}]
756758
"""
757-
text = "SHOW GRANTS FOR {0}".format(username)
759+
text = "SHOW GRANTS FOR {0}".format(quote_ident(username))
758760
return list(self.query(text).get_points())
759761

760762
def send_packet(self, packet, protocol='json'):

‎influxdb/line_protocol.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,32 @@ def _escape_tag(tag):
5353
)
5454

5555

56+
def quote_ident(value):
57+
return "\"{0}\"".format(
58+
value.replace(
59+
"\\", "\\\\"
60+
).replace(
61+
"\"", "\\\""
62+
).replace(
63+
"\n", "\\n"
64+
)
65+
)
66+
67+
68+
def quote_literal(value):
69+
return "'{0}'".format(
70+
value.replace(
71+
"\\", "\\\\"
72+
).replace(
73+
"'", "\\'"
74+
)
75+
)
76+
77+
5678
def _escape_value(value):
5779
value = _get_unicode(value)
5880
if isinstance(value, text_type) and value != '':
59-
return "\"{0}\"".format(
60-
value.replace(
61-
"\"", "\\\""
62-
).replace(
63-
"\n", "\\n"
64-
)
65-
)
81+
return quote_ident(value)
6682
elif isinstance(value, integer_types) and not isinstance(value, bool):
6783
return str(value) + 'i'
6884
else:

‎influxdb/tests/client_test.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ def test_grant_admin_privileges(self):
688688

689689
self.assertEqual(
690690
m.last_request.qs['q'][0],
691-
'grant all privileges to test'
691+
'grant all privileges to "test"'
692692
)
693693

694694
@raises(Exception)
@@ -710,7 +710,7 @@ def test_revoke_admin_privileges(self):
710710

711711
self.assertEqual(
712712
m.last_request.qs['q'][0],
713-
'revoke all privileges from test'
713+
'revoke all privileges from "test"'
714714
)
715715

716716
@raises(Exception)
@@ -732,7 +732,7 @@ def test_grant_privilege(self):
732732

733733
self.assertEqual(
734734
m.last_request.qs['q'][0],
735-
'grant read on testdb to test'
735+
'grant read on "testdb" to "test"'
736736
)
737737

738738
@raises(Exception)
@@ -754,7 +754,7 @@ def test_revoke_privilege(self):
754754

755755
self.assertEqual(
756756
m.last_request.qs['q'][0],
757-
'revoke read on testdb from test'
757+
'revoke read on "testdb" from "test"'
758758
)
759759

760760
@raises(Exception)

‎influxdb/tests/server_tests/client_test_with_server.py

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,6 @@ def test_drop_user_nonexisting(self):
200200
self.assertIn('user not found',
201201
ctx.exception.content)
202202

203-
def test_drop_user_invalid(self):
204-
with self.assertRaises(InfluxDBClientError) as ctx:
205-
self.cli.drop_user('very invalid')
206-
self.assertEqual(400, ctx.exception.code)
207-
self.assertIn('{"error":"error parsing query: '
208-
'found invalid, expected',
209-
ctx.exception.content)
210-
211203
@unittest.skip("Broken as of 0.9.0")
212204
def test_revoke_admin_privileges(self):
213205
self.cli.create_user('test', 'test', admin=True)
@@ -217,13 +209,6 @@ def test_revoke_admin_privileges(self):
217209
self.assertEqual([{'user': 'test', 'admin': False}],
218210
self.cli.get_list_users())
219211

220-
def test_revoke_admin_privileges_invalid(self):
221-
with self.assertRaises(InfluxDBClientError) as ctx:
222-
self.cli.revoke_admin_privileges('')
223-
self.assertEqual(400, ctx.exception.code)
224-
self.assertIn('{"error":"error parsing query: ',
225-
ctx.exception.content)
226-
227212
def test_grant_privilege(self):
228213
self.cli.create_user('test', 'test')
229214
self.cli.create_database('testdb')

‎influxdb/tests/test_line_protocol.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,15 @@ def test_make_lines_unicode(self):
7676
line_protocol.make_lines(data),
7777
'test,unicode_tag=\'Привет!\' unicode_val="Привет!"\n'
7878
)
79+
80+
def test_quote_ident(self):
81+
self.assertEqual(
82+
line_protocol.quote_ident(r"""\foo ' bar " Örf"""),
83+
r'''"\\foo ' bar \" Örf"'''
84+
)
85+
86+
def test_quote_literal(self):
87+
self.assertEqual(
88+
line_protocol.quote_literal(r"""\foo ' bar " Örf"""),
89+
r"""'\\foo \' bar " Örf'"""
90+
)

0 commit comments

Comments
 (0)