Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape sql special like characters #1066

Merged
merged 1 commit into from
Nov 6, 2017
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
9 changes: 5 additions & 4 deletions libcodechecker/server/api/product_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import shared
from ProductManagement_v6 import ttypes

from libcodechecker import util
from libcodechecker.logger import LoggerFactory
from libcodechecker.profiler import timeit
from libcodechecker.server import permissions
Expand Down Expand Up @@ -119,12 +120,12 @@ def getProducts(self, product_endpoint_filter, product_name_filter):
prods = session.query(Product)

if product_endpoint_filter:
prods = prods.filter(Product.endpoint.ilike(
'%{0}%'.format(product_endpoint_filter)))
prods = prods.filter(Product.endpoint.ilike('%{0}%'.format(
util.escape_like(product_endpoint_filter)), escape='*'))

if product_name_filter:
prods = prods.filter(Product.display_name.ilike(
'%{0}%'.format(product_name_filter)))
prods = prods.filter(Product.display_name.ilike('%{0}%'.format(
util.escape_like(product_name_filter)), escape='*'))

prods = prods.all()

Expand Down
4 changes: 3 additions & 1 deletion libcodechecker/server/api/report_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

from libcodechecker import generic_package_context
from libcodechecker import suppress_handler
from libcodechecker import util
# TODO: Cross-subpackage import here.
from libcodechecker.analyze import plist_parser
from libcodechecker.analyze import store_handler
Expand Down Expand Up @@ -559,7 +560,8 @@ def getRunData(self, run_filter):
if run_filter.exactMatch:
q = q.filter(Run.name.in_(run_filter.names))
else:
OR = [Run.name.ilike('%' + name + '%') for
OR = [Run.name.ilike('%{0}%'.format(
util.escape_like(name)), escape='*') for
name in run_filter.names]
q = q.filter(or_(*OR))

Expand Down
7 changes: 7 additions & 0 deletions libcodechecker/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,3 +313,10 @@ def sizeof_fmt(num, suffix='B'):
return "%3.1f%s%s" % (num, unit, suffix)
num /= 1024.0
return "%.1f%s%s" % (num, 'Yi', suffix)


def escape_like(string, escape_char='*'):
"""Escape the string parameter used in SQL LIKE expressions."""
return string.replace(escape_char, escape_char * 2) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a * becomes **? How does this affect the query?

If I have csordas*marton as filter and I have csordas*marton and csordasFOOBARmarton as run names in the database I get only the first as result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have two run names (csordas*marton and csordasFOOBARmarton) you get only the first as result for csordas*marton filter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This escaping applies to the run and product names when the user starts typing an infix of these for filtering. I think it is not necessary to accept joker characters in this situation, so it wouldn't be a problem if asterisk is interpreted as an exact character.

.replace('%', escape_char + '%') \
.replace('_', escape_char + '_')
4 changes: 2 additions & 2 deletions tests/functional/report_viewer_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def setup_package():
# Extend the checker configuration with the server access.
codechecker_cfg.update(server_access)

test_project_name = project_info['name'] + '_' + uuid.uuid4().hex
test_project_name = project_info['name'] + '_' + uuid.uuid4().hex + '**'

ret = codechecker.check(codechecker_cfg,
test_project_name,
Expand All @@ -81,7 +81,7 @@ def setup_package():
sys.exit(1)
print("Analyzing the test project was successful.")

test_project_name_new = project_info['name'] + '_' + uuid.uuid4().hex
test_project_name_new = project_info['name'] + '*' + uuid.uuid4().hex + '%'

# Let's run the second analysis with different
# checkers to have some real difference.
Expand Down
29 changes: 25 additions & 4 deletions tests/functional/report_viewer_api/test_run_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,40 @@ def test_filter_run_names(self):

# Filter runs which name starts with `test_files_`.
test_runs = self.__get_runs('test_files_')
self.assertEqual(len(test_runs), 2,
"There should be two runs for this test.")
self.assertEqual(len(test_runs), 1,
"There should be one run for this test.")

# Run name filter is case insensitive.
test_runs = self.__get_runs('Test_Files_')
self.assertEqual(len(test_runs), 1,
"There should be one run for this test.")

# Filter runs which name contains `files_`.
test_runs = self.__get_runs('files_')
self.assertEqual(len(test_runs), 1,
"There should be one run for this test.")

# Filter runs which name contains `test_files*`.
test_runs = self.__get_runs('test_files*')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there could be more tests, e.g. for **, %.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've created a few more test cases.

self.assertEqual(len(test_runs), 1,
"There should be one run for this test.")

test_runs = self.__get_runs('_')
self.assertEqual(len(test_runs), 2,
"There should be two runs for this test.")

# Filter runs which name contains `files`.
test_runs = self.__get_runs('files_')
test_runs = self.__get_runs('*')
self.assertEqual(len(test_runs), 2,
"There should be two runs for this test.")

test_runs = self.__get_runs('**')
self.assertEqual(len(test_runs), 1,
"There should be one run for this test.")

test_runs = self.__get_runs('%')
self.assertEqual(len(test_runs), 1,
"There should be one run for this test.")

# Filter non existing run.
test_runs = self.__get_runs('non_existing_run_name')
self.assertEqual(len(test_runs), 0,
Expand Down