Copilot commented on code in PR #62659:
URL: https://github.com/apache/airflow/pull/62659#discussion_r2868714255
##########
airflow/api_fastapi/common/parameters.py:
##########
@@ -98,6 +100,11 @@ def to_orm(self, select: Select) -> Select:
@classmethod
def depends(cls, limit: NonNegativeInt = 100) -> LimitFilter:
+ if limit > cls.MAX_LIMIT:
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ f"limit must be less than or equal to {cls.MAX_LIMIT}",
+ )
return cls().set_value(limit)
Review Comment:
The max-limit enforcement is only applied in `LimitFilter.depends()`.
Several call sites instantiate `LimitFilter` directly (e.g. batch endpoints use
`LimitFilter(body.page_limit)`), which bypasses this check and still allows
very large limits (defeating the memory-exhaustion protection described in the
PR). Consider enforcing the cap inside `LimitFilter` itself (e.g., override
`set_value()` / validate in `__init__()` / guard in `to_orm()`) so all
construction paths are covered, and then decide whether the error should be
HTTP 400 or a validation error consistently across query/body inputs.
##########
airflow/api_fastapi/common/parameters.py:
##########
@@ -98,6 +100,11 @@ def to_orm(self, select: Select) -> Select:
@classmethod
def depends(cls, limit: NonNegativeInt = 100) -> LimitFilter:
+ if limit > cls.MAX_LIMIT:
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ f"limit must be less than or equal to {cls.MAX_LIMIT}",
+ )
Review Comment:
This introduces new behavior (rejecting `limit > 500` with HTTP 400) but
there doesn’t appear to be coverage asserting the 400 response and error detail
for an over-limit request. Adding a regression test around a representative
public list endpoint (and any batch endpoint if `page_limit` is also capped)
would help prevent accidental removal or inconsistent behavior across routes.
--
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]
