diff --git a/frappe/database/database.py b/frappe/database/database.py index aff9395846..3d2997a5a3 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -826,6 +826,7 @@ class Database: fields=fields, distinct=distinct, limit=limit, + validate_filters=True, ) if isinstance(fields, str) and fields == "*": as_dict = True @@ -854,6 +855,7 @@ class Database: order_by=order_by, distinct=distinct, limit=limit, + validate_filters=True, ).run(debug=debug, run=run, as_dict=as_dict, pluck=pluck) return {} @@ -903,7 +905,12 @@ class Database: field, val, modified=modified, modified_by=modified_by, update_modified=update_modified ) - query = frappe.qb.get_query(table=dt, filters=dn, update=True) + query = frappe.qb.get_query( + table=dt, + filters=dn, + update=True, + validate_filters=True, + ) if isinstance(dn, str): frappe.clear_document_cache(dt, dn) @@ -1071,9 +1078,13 @@ class Database: cache_count = frappe.cache().get_value(f"doctype:count:{dt}") if cache_count is not None: return cache_count - count = frappe.qb.get_query(table=dt, filters=filters, fields=Count("*"), distinct=distinct).run( - debug=debug - )[0][0] + count = frappe.qb.get_query( + table=dt, + filters=filters, + fields=Count("*"), + distinct=distinct, + validate_filters=True, + ).run(debug=debug)[0][0] if not filters and cache: frappe.cache().set_value(f"doctype:count:{dt}", count, expires_in_sec=86400) return count @@ -1193,7 +1204,12 @@ class Database: Doctype name can be passed directly, it will be pre-pended with `tab`. """ filters = filters or kwargs.get("conditions") - query = frappe.qb.get_query(table=doctype, filters=filters, delete=True) + query = frappe.qb.get_query( + table=doctype, + filters=filters, + delete=True, + validate_filters=True, + ) if "debug" not in kwargs: kwargs["debug"] = debug return query.run(**kwargs) diff --git a/frappe/database/query.py b/frappe/database/query.py index 02beff9afc..06295d33a6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -9,6 +9,7 @@ from pypika.queries import QueryBuilder, Table import frappe from frappe import _ from frappe.database.operator_map import OPERATOR_MAP +from frappe.database.schema import SPECIAL_CHAR_PATTERN from frappe.database.utils import DefaultOrderBy, get_doctype_name from frappe.query_builder import Criterion, Field, Order, functions from frappe.query_builder.functions import Function, SqlFunctions @@ -44,9 +45,12 @@ class Engine: update: bool = False, into: bool = False, delete: bool = False, + *, + validate_filters: bool = False, ) -> QueryBuilder: self.is_mariadb = frappe.db.db_type == "mariadb" self.is_postgres = frappe.db.db_type == "postgres" + self.validate_filters = validate_filters if isinstance(table, Table): self.table = table @@ -157,14 +161,16 @@ class Engine: _value = value _operator = operator - if isinstance(_field, Field): + if not isinstance(_field, str): pass - elif dynamic_field := DynamicTableField.parse(field, self.doctype): + elif not self.validate_filters and ( + dynamic_field := DynamicTableField.parse(field, self.doctype) + ): # apply implicit join if link field's field is referenced self.query = dynamic_field.apply_join(self.query) _field = dynamic_field.field - elif has_function(field): - _field = self.get_function_object(field) + elif self.validate_filters and SPECIAL_CHAR_PATTERN.search(_field): + frappe.throw(_("Invalid filter: {0}").format(_field)) elif not doctype or doctype == self.doctype: _field = self.table[field] elif doctype: diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index 451dc699fe..a8e4841953 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -202,7 +202,11 @@ def get_cards_for_user(doctype, txt, searchfield, start, page_len, filters): if txt: search_conditions = [numberCard[field].like(f"%{txt}%") for field in searchfields] - condition_query = frappe.qb.get_query(doctype, filters=filters) + condition_query = frappe.qb.get_query( + doctype, + filters=filters, + validate_filters=True, + ) return ( condition_query.select(numberCard.name, numberCard.label, numberCard.document_type) diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index 05d45ad9ac..a1db82810e 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -36,7 +36,12 @@ def get_group_by_count(doctype: str, current_filters: str, field: str) -> list[d ToDo = DocType("ToDo") User = DocType("User") count = Count("*").as_("count") - filtered_records = frappe.qb.get_query(doctype, filters=current_filters, fields=["name"]) + filtered_records = frappe.qb.get_query( + doctype, + filters=current_filters, + fields=["name"], + validate_filters=True, + ) return ( frappe.qb.from_(ToDo) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index dfebf5e890..9242630104 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -218,13 +218,6 @@ class TestQuery(FrappeTestCase): @run_only_if(db_type_is.MARIADB) def test_filters(self): - self.assertEqual( - frappe.qb.get_query( - "User", filters={"IfNull(name, " ")": ("<", Now())}, fields=["Max(name)"] - ).run(), - frappe.qb.from_("User").select(Max(Field("name"))).where(Ifnull("name", "") < Now()).run(), - ) - self.assertEqual( frappe.qb.get_query( "DocType", @@ -258,6 +251,17 @@ class TestQuery(FrappeTestCase): ), ) + self.assertRaisesRegex( + frappe.ValidationError, + "Invalid filter", + lambda: frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"permissions.role": "System Manager"}, + validate_filters=True, + ), + ) + self.assertEqual( frappe.qb.get_query( "DocType", diff --git a/frappe/utils/goal.py b/frappe/utils/goal.py index 709fdc1644..01cd9d835e 100644 --- a/frappe/utils/goal.py +++ b/frappe/utils/goal.py @@ -31,6 +31,7 @@ def get_monthly_results( Function(aggregation, goal_field), ], filters=filters, + validate_filters=True, ) .groupby("month_year") .run()