Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

Properly quote all identifiers and literals in InfluxDBClient #361

Merged
merged 1 commit into from
Sep 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 21 additions & 19 deletions influxdb/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import requests.exceptions
from sys import version_info

from influxdb.line_protocol import make_lines
from influxdb.line_protocol import make_lines, quote_ident, quote_literal
from influxdb.resultset import ResultSet
from .exceptions import InfluxDBClientError
from .exceptions import InfluxDBServerError
Expand Down Expand Up @@ -532,8 +532,8 @@ def alter_retention_policy(self, name, database=None,
should be set. Otherwise the operation will fail.
"""
query_string = (
"ALTER RETENTION POLICY \"{0}\" ON \"{1}\""
).format(name, database or self._database)
"ALTER RETENTION POLICY {0} ON {1}"
).format(quote_ident(name), quote_ident(database or self._database))
if duration:
query_string += " DURATION {0}".format(duration)
if replication:
Expand All @@ -553,8 +553,8 @@ def drop_retention_policy(self, name, database=None):
:type database: str
"""
query_string = (
"DROP RETENTION POLICY \"{0}\" ON \"{1}\""
).format(name, database or self._database)
"DROP RETENTION POLICY {0} ON {1}"
).format(quote_ident(name), quote_ident(database or self._database))
self.query(query_string)

def get_list_retention_policies(self, database=None):
Expand Down Expand Up @@ -611,8 +611,8 @@ def create_user(self, username, password, admin=False):
privileges or not
:type admin: boolean
"""
text = "CREATE USER \"{0}\" WITH PASSWORD '{1}'".format(username,
password)
text = "CREATE USER {0} WITH PASSWORD {1}".format(
quote_ident(username), quote_literal(password))
if admin:
text += ' WITH ALL PRIVILEGES'
self.query(text)
Expand All @@ -623,7 +623,7 @@ def drop_user(self, username):
:param username: the username to drop
:type username: str
"""
text = "DROP USER {0}".format(username)
text = "DROP USER {0}".format(quote_ident(username))
self.query(text)

def set_user_password(self, username, password):
Expand All @@ -634,7 +634,8 @@ def set_user_password(self, username, password):
:param password: the new password for the user
:type password: str
"""
text = "SET PASSWORD FOR {0} = '{1}'".format(username, password)
text = "SET PASSWORD FOR {0} = {1}".format(
quote_ident(username), quote_literal(password))
self.query(text)

def delete_series(self, database=None, measurement=None, tags=None):
Expand All @@ -652,11 +653,12 @@ def delete_series(self, database=None, measurement=None, tags=None):
database = database or self._database
query_str = 'DROP SERIES'
if measurement:
query_str += ' FROM "{0}"'.format(measurement)
query_str += ' FROM {0}'.format(quote_ident(measurement))

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

def grant_admin_privileges(self, username):
Expand All @@ -668,7 +670,7 @@ def grant_admin_privileges(self, username):
.. note:: Only a cluster administrator can create/drop databases
and manage users.
"""
text = "GRANT ALL PRIVILEGES TO {0}".format(username)
text = "GRANT ALL PRIVILEGES TO {0}".format(quote_ident(username))
self.query(text)

def revoke_admin_privileges(self, username):
Expand All @@ -680,7 +682,7 @@ def revoke_admin_privileges(self, username):
.. note:: Only a cluster administrator can create/ drop databases
and manage users.
"""
text = "REVOKE ALL PRIVILEGES FROM {0}".format(username)
text = "REVOKE ALL PRIVILEGES FROM {0}".format(quote_ident(username))
self.query(text)

def grant_privilege(self, privilege, database, username):
Expand All @@ -695,8 +697,8 @@ def grant_privilege(self, privilege, database, username):
:type username: str
"""
text = "GRANT {0} ON {1} TO {2}".format(privilege,
database,
username)
quote_ident(database),
quote_ident(username))
self.query(text)

def revoke_privilege(self, privilege, database, username):
Expand All @@ -711,8 +713,8 @@ def revoke_privilege(self, privilege, database, username):
:type username: str
"""
text = "REVOKE {0} ON {1} FROM {2}".format(privilege,
database,
username)
quote_ident(database),
quote_ident(username))
self.query(text)

def get_list_privileges(self, username):
Expand All @@ -734,7 +736,7 @@ def get_list_privileges(self, username):
{u'privilege': u'ALL PRIVILEGES', u'database': u'db2'},
{u'privilege': u'NO PRIVILEGES', u'database': u'db3'}]
"""
text = "SHOW GRANTS FOR {0}".format(username)
text = "SHOW GRANTS FOR {0}".format(quote_ident(username))
return list(self.query(text).get_points())

def send_packet(self, packet):
Expand Down
30 changes: 23 additions & 7 deletions influxdb/line_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,32 @@ def _escape_tag(tag):
)


def quote_ident(value):
return "\"{0}\"".format(
value.replace(
"\\", "\\\\"
).replace(
"\"", "\\\""
).replace(
"\n", "\\n"
)
)


def quote_literal(value):
return "'{0}'".format(
value.replace(
"\\", "\\\\"
).replace(
"'", "\\'"
)
)


def _escape_value(value):
value = _get_unicode(value)
if isinstance(value, text_type) and value != '':
return "\"{0}\"".format(
value.replace(
"\"", "\\\""
).replace(
"\n", "\\n"
)
)
return quote_ident(value)
elif isinstance(value, integer_types) and not isinstance(value, bool):
return str(value) + 'i'
else:
Expand Down
8 changes: 4 additions & 4 deletions influxdb/tests/client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ def test_grant_admin_privileges(self):

self.assertEqual(
m.last_request.qs['q'][0],
'grant all privileges to test'
'grant all privileges to "test"'
)

@raises(Exception)
Expand All @@ -710,7 +710,7 @@ def test_revoke_admin_privileges(self):

self.assertEqual(
m.last_request.qs['q'][0],
'revoke all privileges from test'
'revoke all privileges from "test"'
)

@raises(Exception)
Expand All @@ -732,7 +732,7 @@ def test_grant_privilege(self):

self.assertEqual(
m.last_request.qs['q'][0],
'grant read on testdb to test'
'grant read on "testdb" to "test"'
)

@raises(Exception)
Expand All @@ -754,7 +754,7 @@ def test_revoke_privilege(self):

self.assertEqual(
m.last_request.qs['q'][0],
'revoke read on testdb from test'
'revoke read on "testdb" from "test"'
)

@raises(Exception)
Expand Down
15 changes: 0 additions & 15 deletions influxdb/tests/server_tests/client_test_with_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,6 @@ def test_drop_user_nonexisting(self):
self.assertIn('user not found',
ctx.exception.content)

def test_drop_user_invalid(self):
with self.assertRaises(InfluxDBClientError) as ctx:
self.cli.drop_user('very invalid')
self.assertEqual(400, ctx.exception.code)
self.assertIn('{"error":"error parsing query: '
'found invalid, expected',
ctx.exception.content)

@unittest.skip("Broken as of 0.9.0")
def test_revoke_admin_privileges(self):
self.cli.create_user('test', 'test', admin=True)
Expand All @@ -217,13 +209,6 @@ def test_revoke_admin_privileges(self):
self.assertEqual([{'user': 'test', 'admin': False}],
self.cli.get_list_users())

def test_revoke_admin_privileges_invalid(self):
with self.assertRaises(InfluxDBClientError) as ctx:
self.cli.revoke_admin_privileges('')
self.assertEqual(400, ctx.exception.code)
self.assertIn('{"error":"error parsing query: ',
ctx.exception.content)

def test_grant_privilege(self):
self.cli.create_user('test', 'test')
self.cli.create_database('testdb')
Expand Down
12 changes: 12 additions & 0 deletions influxdb/tests/test_line_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,15 @@ def test_make_lines_unicode(self):
line_protocol.make_lines(data),
'test,unicode_tag=\'Привет!\' unicode_val="Привет!"\n'
)

def test_quote_ident(self):
self.assertEqual(
line_protocol.quote_ident(r"""\foo ' bar " Örf"""),
r'''"\\foo ' bar \" Örf"'''
)

def test_quote_literal(self):
self.assertEqual(
line_protocol.quote_literal(r"""\foo ' bar " Örf"""),
r"""'\\foo \' bar " Örf'"""
)