bito-code-review[bot] commented on code in PR #41527:
URL: https://github.com/apache/superset/pull/41527#discussion_r3504401294
##########
superset/utils/date_parser.py:
##########
@@ -426,10 +426,27 @@ def get_since_until( # pylint:
disable=too-many-arguments,too-many-locals,too-m
return None, None
if time_range and time_range.startswith("Last") and separator not in
time_range:
- time_range = time_range + separator + _relative_end
+ # For granular time units (second/minute/hour), "today" (midnight)
would be
+ # earlier than the computed since time (e.g. now-1h), causing a
+ # "From date cannot be larger than to date" error. Always use "now" for
+ # sub-day units so that the until bound is consistent with the since
bound.
+ _granular_last_pattern: str = (
+ r"^Last\s+(?:[0-9]+\s+)?(second|minute|hour)s?$"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-20: Regex quantifier precedence bug</b></div>
<div id="fix">
The regex alternation `(second|minute|hour)s?` incorrectly allows invalid
plural forms without a number (e.g., "Last hours", "Next hours") to match. The
`s?` should apply only to matched units when preceded by the optional number
group. For "Last hour" the pattern works, but for "Last hours" it incorrectly
captures "hour" as the group and accepts the trailing "s" because `s?` follows
the group, not because the number is present. (See also:
[CWE-20](https://cwe.mitre.org/data/definitions/20.html))
</div>
</div>
<small><i>Code Review Run #4b128a</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:
##########
@@ -426,10 +426,27 @@ def get_since_until( # pylint:
disable=too-many-arguments,too-many-locals,too-m
return None, None
if time_range and time_range.startswith("Last") and separator not in
time_range:
- time_range = time_range + separator + _relative_end
+ # For granular time units (second/minute/hour), "today" (midnight)
would be
+ # earlier than the computed since time (e.g. now-1h), causing a
+ # "From date cannot be larger than to date" error. Always use "now" for
+ # sub-day units so that the until bound is consistent with the since
bound.
+ _granular_last_pattern: str = (
+ r"^Last\s+(?:[0-9]+\s+)?(second|minute|hour)s?$"
+ )
+ if re.search(_granular_last_pattern, time_range, re.IGNORECASE):
+ time_range = time_range + separator + "now"
+ else:
+ time_range = time_range + separator + _relative_end
if time_range and time_range.startswith("Next") and separator not in
time_range:
- time_range = _relative_start + separator + time_range
+ # Mirror the same logic for Next: granular units should start from
"now".
+ _granular_next_pattern: str = (
+ r"^Next\s+(?:[0-9]+\s+)?(second|minute|hour)s?$"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-20: Identical regex bug in Next pattern</b></div>
<div id="fix">
The "Next" pattern has the identical regex precedence bug as the "Last"
pattern on line 434. Both patterns use `(second|minute|hour)s?` and both should
be corrected consistently to prevent invalid plural forms from matching. (See
also: [CWE-20](https://cwe.mitre.org/data/definitions/20.html))
</div>
</div>
<small><i>Code Review Run #4b128a</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]
