Merge pull request #38270 from sagarvora/correct-flags
This commit is contained in:
commit
f97388f5d9
2 changed files with 9 additions and 11 deletions
|
|
@ -136,11 +136,7 @@ WORDS_PATTERN = re.compile(r"\w+")
|
||||||
COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))")
|
COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))")
|
||||||
|
|
||||||
# Pattern for validating simple field names (alphanumeric + underscore)
|
# Pattern for validating simple field names (alphanumeric + underscore)
|
||||||
SIMPLE_FIELD_PATTERN = re.compile(r"^\w+$", flags=re.ASCII)
|
SIMPLE_FIELD_PATTERN = re.compile(r"^\w+$")
|
||||||
|
|
||||||
# Pattern for validating SQL identifiers (aliases, field names in functions)
|
|
||||||
# More restrictive: must start with letter or underscore
|
|
||||||
IDENTIFIER_PATTERN = re.compile(r"^[a-zA-Z_][a-zA-Z0-9_]*$", flags=re.ASCII)
|
|
||||||
|
|
||||||
# Pattern for detecting SQL function calls: identifier followed by opening parenthesis
|
# Pattern for detecting SQL function calls: identifier followed by opening parenthesis
|
||||||
FUNCTION_CALL_PATTERN = re.compile(r"^\s*[a-zA-Z_][a-zA-Z0-9_]*\s*\(", flags=re.ASCII)
|
FUNCTION_CALL_PATTERN = re.compile(r"^\s*[a-zA-Z_][a-zA-Z0-9_]*\s*\(", flags=re.ASCII)
|
||||||
|
|
@ -157,7 +153,7 @@ FUNCTION_CALL_PATTERN = re.compile(r"^\s*[a-zA-Z_][a-zA-Z0-9_]*\s*\(", flags=re.
|
||||||
# - ... as 'Child:field'
|
# - ... as 'Child:field'
|
||||||
ALLOWED_FIELD_PATTERN = re.compile(
|
ALLOWED_FIELD_PATTERN = re.compile(
|
||||||
r"^(?:(`[\w\s-]+`|\w+)\.)?(`\w+`|\w+)(?:\s+as\s+(?:`[\w\s-]+`|'[\w\s:-]+'|\w+))?$",
|
r"^(?:(`[\w\s-]+`|\w+)\.)?(`\w+`|\w+)(?:\s+as\s+(?:`[\w\s-]+`|'[\w\s:-]+'|\w+))?$",
|
||||||
flags=re.ASCII | re.IGNORECASE,
|
flags=re.IGNORECASE,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Regex to parse field names:
|
# Regex to parse field names:
|
||||||
|
|
@ -2424,14 +2420,15 @@ class SQLFunctionParser:
|
||||||
).format(arg),
|
).format(arg),
|
||||||
frappe.ValidationError,
|
frappe.ValidationError,
|
||||||
)
|
)
|
||||||
elif self._is_valid_field_name(arg):
|
|
||||||
self._check_function_field_permission(arg)
|
|
||||||
return self.engine.table[arg]
|
|
||||||
|
|
||||||
# Check if it's a numeric string like "1" (for COUNT(1), etc.)
|
# Check if it's a numeric string like "1" (for COUNT(1), etc.)
|
||||||
elif arg.isdigit():
|
elif arg.isdigit():
|
||||||
return int(arg)
|
return int(arg)
|
||||||
|
|
||||||
|
elif self._is_valid_field_name(arg):
|
||||||
|
self._check_function_field_permission(arg)
|
||||||
|
return self.engine.table[arg]
|
||||||
|
|
||||||
else:
|
else:
|
||||||
frappe.throw(
|
frappe.throw(
|
||||||
_(
|
_(
|
||||||
|
|
@ -2443,7 +2440,7 @@ class SQLFunctionParser:
|
||||||
def _is_valid_field_name(self, name: str) -> bool:
|
def _is_valid_field_name(self, name: str) -> bool:
|
||||||
"""Check if a string is a valid field name."""
|
"""Check if a string is a valid field name."""
|
||||||
# Field names should only contain alphanumeric characters and underscores
|
# Field names should only contain alphanumeric characters and underscores
|
||||||
return IDENTIFIER_PATTERN.match(name) is not None
|
return SIMPLE_FIELD_PATTERN.match(name) is not None
|
||||||
|
|
||||||
def _validate_alias(self, alias: str):
|
def _validate_alias(self, alias: str):
|
||||||
"""Validate alias name for SQL injection."""
|
"""Validate alias name for SQL injection."""
|
||||||
|
|
@ -2456,7 +2453,7 @@ class SQLFunctionParser:
|
||||||
|
|
||||||
# Alias should be a simple identifier
|
# Alias should be a simple identifier
|
||||||
# Note: pypika wraps aliases in backticks, so anything without backticks is safe
|
# Note: pypika wraps aliases in backticks, so anything without backticks is safe
|
||||||
if not IDENTIFIER_PATTERN.match(alias):
|
if not SIMPLE_FIELD_PATTERN.match(alias):
|
||||||
frappe.throw(
|
frappe.throw(
|
||||||
_("Invalid alias format: {0}. Alias must be a simple identifier.").format(alias),
|
_("Invalid alias format: {0}. Alias must be a simple identifier.").format(alias),
|
||||||
frappe.ValidationError,
|
frappe.ValidationError,
|
||||||
|
|
|
||||||
|
|
@ -167,6 +167,7 @@ class TestQuery(IntegrationTestCase):
|
||||||
"*",
|
"*",
|
||||||
"`tabHas Role`.`name`",
|
"`tabHas Role`.`name`",
|
||||||
"field as `alias with space`",
|
"field as `alias with space`",
|
||||||
|
"frappé", # unicode field names should be valid
|
||||||
]
|
]
|
||||||
|
|
||||||
invalid_fields = [
|
invalid_fields = [
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue