fix(refactor): lockdown frappe.desk.reportview #12713
Conversation
| is_report = data.get('view') == 'Report' | ||
| return data | ||
|
|
||
| def validate_fields(data): |
There was a problem hiding this comment.
Looks like all validators changing the data inline without returning the data.
I prefer to go with a class here as we are trying to keep the state of the data. May be something like
q = QueryBuilder(data)
q.is_valid_query() - says the query is valid or not
q.data - returns me the constructed data that I can use to query
There was a problem hiding this comment.
validate_fields is also called independently from frappe.client.get_list
| data.pop('aggregate_function') | ||
|
|
||
| def raise_invalid_field(fieldname): | ||
| frappe.throw(_('Field not permitted in query') + ': {0}'.format(fieldname), frappe.DataError) |
There was a problem hiding this comment.
| frappe.throw(_('Field not permitted in query') + ': {0}'.format(fieldname), frappe.DataError) | |
| frappe.throw(_('Field {0} is not permitted in query').format(frapp.bold(fieldname)), frappe.DataError) |
There was a problem hiding this comment.
Bold will highlight the field and fix readability right?
Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com>
made that choice for readability
| if (fieldname.startswith('count(') | ||
| or fieldname.startswith('sum(') | ||
| or fieldname.startswith('avg(')): | ||
| if not fieldname.strip().endswith(')'): |
There was a problem hiding this comment.
May be a list of agregator_functions?
allowed_aggregators = ['count', 'sum', 'avg']
| for sep in (' as ', ' AS '): | ||
| if sep in fieldname: | ||
| fieldname = fieldname.split(sep)[0] |
There was a problem hiding this comment.
We can use start_index = field.lower().find(' as ').
With this we can handle separators with mixed case aswell.
|
@Mergifyio backport version-13-pre-release |
|
Command
|
|
@Mergifyio backport version-12-hotfix |
…se/pr-12713 fix(refactor): lockdown frappe.desk.reportview (bp #12713)
|
Command
|
This method locks down publicly facing
frappe.desk.reportview.getandfrappe.client.get_listto allow only valid fieldnames as parameters.