rusackas commented on code in PR #40195:
URL: https://github.com/apache/superset/pull/40195#discussion_r3564765099
##########
superset/utils/csv.py:
##########
@@ -27,18 +26,37 @@
logger = logging.getLogger(__name__)
-negative_number_re = re.compile(r"^-[0-9.]+$")
+PROBLEMATIC_CSV_PREFIXES = "-@+|=%"
Review Comment:
Fair, annotated it as str.
##########
superset/utils/csv.py:
##########
@@ -27,18 +26,37 @@
logger = logging.getLogger(__name__)
-negative_number_re = re.compile(r"^-[0-9.]+$")
+PROBLEMATIC_CSV_PREFIXES = "-@+|=%"
-# This regex will match if the string starts with:
-#
-# 1. one of -, @, +, |, =, %, a tab, or a carriage return
-# 2. two double quotes immediately followed by one of -, @, +, |, =, %
-# 3. one or more spaces immediately followed by one of -, @, +, |, =, %
-#
-# A leading tab or carriage return is treated as dangerous on its own because
-# some spreadsheet software trims that leading whitespace and then evaluates
-# the remaining cell content as a formula.
-problematic_chars_re =
re.compile(r'^(?:"{2}|\s{1,})(?=[\-@+|=%])|^[\-@+|=%\t\r]')
+
+def _starts_with_formula_prefix(value: str) -> bool:
+ first = value[0]
+ if first in PROBLEMATIC_CSV_PREFIXES:
+ return True
+ if first == '"' and len(value) > 2:
+ return value[1] == '"' and value[2] in PROBLEMATIC_CSV_PREFIXES
+ return False
+
+
+def _starts_like_spreadsheet_formula(value: str) -> bool:
+ # A leading tab or carriage return is treated as dangerous on its own
+ # because some spreadsheet software trims that leading whitespace and
+ # then evaluates the remaining cell content as a formula.
+ first = value[0]
+ if first in ("\t", "\r"):
+ return True
+ if first.isspace():
+ stripped = value.lstrip()
+ return bool(stripped) and _starts_with_formula_prefix(stripped)
+ return _starts_with_formula_prefix(value)
+
+
+def _is_negative_number(value: str) -> bool:
+ return (
+ len(value) > 1
+ and value[0] == "-"
+ and all("0" <= character <= "9" or character == "." for character in
value[1:])
+ )
Review Comment:
`-.`, `-..`, `-1.2.3` all matched the old `^-[0-9.]+$` regex too, so this
isn't a new gap. This PR is a perf refactor that preserves the prior escaping
semantics exactly. Tightening the numeric validation is a separate behavior
change, not something to slip into this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
