bito-code-review[bot] commented on code in PR #36983:
URL: https://github.com/apache/superset/pull/36983#discussion_r2672566569
##########
superset/utils/date_parser.py:
##########
@@ -418,9 +418,9 @@ def get_since_until( # pylint:
disable=too-many-arguments,too-many-locals,too-m
if time_range and separator in time_range:
time_range_lookup = [
(
- r"^(start of|beginning of|end of)\s+"
- r"(this|last|next|prior)\s+"
- r"([0-9]+)?\s*"
+ r"^(start of|beginning of|end of)\s{1,5}"
+ r"(this|last|next|prior)\s{1,5}"
+ r"([0-9]+)?\s{0,5}"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Regex space limit changes parsing behavior</b></div>
<div id="fix">
This regex change limits spaces to 5, altering parsing behavior for inputs
like 'start of next month' (7 spaces), which would now fail to match and
fall back to invalid DATETIME() expressions. If security is the goal, consider
a higher limit or space normalization.
</div>
</div>
<small><i>Code Review Run #b98152</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
tests/unit_tests/utils/date_parser_tests.py:
##########
@@ -611,3 +611,48 @@ def test_date_range_migration() -> None:
field = "10 years ago"
assert not re.search(DateRangeMigration.x_dateunit, field)
+
+
+# Tests for bounded whitespace regex patterns in time_range_lookup
[email protected](
+ "time_range,should_parse",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect parametrize argument type</b></div>
<div id="fix">
Line 618: `pytest.mark.parametrize` expects a tuple for the first argument,
but a string was provided. Change `"time_range,should_parse"` to
`("time_range", "should_parse")` to fix the issue.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
@pytest.mark.parametrize(
("time_range", "should_parse"),
````
</div>
</details>
</div>
<small><i>Code Review Run #b98152</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/utils/date_parser.py:
##########
@@ -431,8 +431,8 @@ def get_since_until( # pylint:
disable=too-many-arguments,too-many-locals,too-m
),
),
(
- r"^(this|last|next|prior)\s+"
- r"([0-9]+)?\s*"
+ r"^(this|last|next|prior)\s{1,5}"
+ r"([0-9]+)?\s{0,5}"
r"(second|minute|day|week|month|quarter|year)s?$", # Matches
"next 5 days" or "last 2 weeks" # noqa: E501
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Regex space limit changes parsing behavior</b></div>
<div id="fix">
Similar to above, this regex change limits spaces to 5, potentially breaking
parsing for 'last 5 days' with extra spaces, leading to fallback to
invalid DATETIME() expressions. If intended for security, a higher limit may
preserve compatibility.
</div>
</div>
<small><i>Code Review Run #b98152</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
