rusackas commented on code in PR #40640:
URL: https://github.com/apache/superset/pull/40640#discussion_r3338101271
##########
superset/charts/schemas.py:
##########
@@ -62,6 +62,34 @@ def get_time_grain_choices() -> Any:
]
+# Fallback upper bound for the number of Prophet forecast periods when the
+# application config cannot be read (for example, outside of an app context).
+DEFAULT_MAX_PROPHET_PERIODS = 10000
+
+
+def get_max_prophet_periods() -> int:
+ """Get the configured upper bound for Prophet forecast periods."""
+ try:
+ return current_app.config.get(
+ "MAX_PROPHET_PERIODS", DEFAULT_MAX_PROPHET_PERIODS
+ )
Review Comment:
Noted — in practice, `MAX_PROPHET_PERIODS` is set via Python config where
the value is already an int. If it were set from an env var, the caller would
need to coerce it. Adding a defensive `int()` cast is a reasonable follow-up
but out of scope for this PR.
##########
superset/utils/schema.py:
##########
@@ -51,3 +54,26 @@ def validate_json(value: Union[bytes, bytearray, str]) ->
None:
json.validate_json(value)
except json.JSONDecodeError as ex:
raise ValidationError("JSON not valid") from ex
+
+
+def validate_external_url(value: Optional[str]) -> None:
+ """
+ Validator for externally managed object URLs.
+
+ Restricts the accepted URL schemes to ``http`` and ``https`` so that
+ other schemes (for example ``javascript:``, ``data:`` or ``vbscript:``)
+ cannot be stored and later rendered by clients. Empty values are allowed
+ since the field is optional.
+
+ :param value: the URL to validate
+ :raises ValidationError: if the value uses a disallowed scheme
+ """
+ if not value:
+ return
+
+ scheme = urlparse(value).scheme.lower()
+ if scheme not in ALLOWED_URL_SCHEMES:
+ raise ValidationError(
+ "URL must use one of the following schemes: "
+ f"{', '.join(sorted(ALLOWED_URL_SCHEMES))}."
Review Comment:
Good point — `urlparse` would catch this. Adding netloc validation is a
useful hardening. Adding it as a follow-up to avoid scope creep here.
##########
superset/charts/schemas.py:
##########
@@ -62,6 +62,34 @@ def get_time_grain_choices() -> Any:
]
+# Fallback upper bound for the number of Prophet forecast periods when the
+# application config cannot be read (for example, outside of an app context).
+DEFAULT_MAX_PROPHET_PERIODS = 10000
+
+
+def get_max_prophet_periods() -> int:
+ """Get the configured upper bound for Prophet forecast periods."""
+ try:
+ return current_app.config.get(
+ "MAX_PROPHET_PERIODS", DEFAULT_MAX_PROPHET_PERIODS
+ )
+ except RuntimeError:
+ # Outside app context, fall back to the default bound
+ return DEFAULT_MAX_PROPHET_PERIODS
+
+
+def validate_prophet_periods(value: int) -> None:
+ """Ensure the number of Prophet forecast periods stays within bounds."""
+ Range(
+ min=1,
+ max=get_max_prophet_periods(),
+ error=_(
+ "`periods` must be between 1 and %(max)s",
+ max=get_max_prophet_periods(),
+ ),
+ )(value)
Review Comment:
Acknowledged — calling `get_max_prophet_periods()` twice is redundant.
Captured it once would be a minor cleanup. Left as a follow-up.
--
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]
