Skip to content

Commit fc96d6f

Browse files
rawwarLefteris Gilmaz
rawwar
authored and
Lefteris Gilmaz
committed
AIP-84 Allow only positive limit and offset (apache#44109)
* limit and offset can't be negative * add tests * refactor * use NonNegativeInt
1 parent 1f9e4f0 commit fc96d6f

File tree

3 files changed

+92
-5
lines changed

3 files changed

+92
-5
lines changed

‎airflow/api_fastapi/common/parameters.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
from fastapi import Depends, HTTPException, Query
2525
from pendulum.parsing.exceptions import ParserError
26-
from pydantic import AfterValidator, BaseModel
26+
from pydantic import AfterValidator, BaseModel, NonNegativeInt
2727
from sqlalchemy import Column, case, or_
2828
from sqlalchemy.inspection import inspect
2929

@@ -66,7 +66,7 @@ def depends(self, *args: Any, **kwargs: Any) -> Self:
6666
pass
6767

6868

69-
class LimitFilter(BaseParam[int]):
69+
class LimitFilter(BaseParam[NonNegativeInt]):
7070
"""Filter on the limit."""
7171

7272
def to_orm(self, select: Select) -> Select:
@@ -75,19 +75,19 @@ def to_orm(self, select: Select) -> Select:
7575

7676
return select.limit(self.value)
7777

78-
def depends(self, limit: int = 100) -> LimitFilter:
78+
def depends(self, limit: NonNegativeInt = 100) -> LimitFilter:
7979
return self.set_value(limit)
8080

8181

82-
class OffsetFilter(BaseParam[int]):
82+
class OffsetFilter(BaseParam[NonNegativeInt]):
8383
"""Filter on offset."""
8484

8585
def to_orm(self, select: Select) -> Select:
8686
if self.value is None and self.skip_none:
8787
return select
8888
return select.offset(self.value)
8989

90-
def depends(self, offset: int = 0) -> OffsetFilter:
90+
def depends(self, offset: NonNegativeInt = 0) -> OffsetFilter:
9191
return self.set_value(offset)
9292

9393

‎airflow/api_fastapi/core_api/openapi/v1-generated.yaml‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,15 @@ paths:
9393
required: false
9494
schema:
9595
type: integer
96+
minimum: 0
9697
default: 100
9798
title: Limit
9899
- name: offset
99100
in: query
100101
required: false
101102
schema:
102103
type: integer
104+
minimum: 0
103105
default: 0
104106
title: Offset
105107
- name: tags
@@ -183,13 +185,15 @@ paths:
183185
required: false
184186
schema:
185187
type: integer
188+
minimum: 0
186189
default: 100
187190
title: Limit
188191
- name: offset
189192
in: query
190193
required: false
191194
schema:
192195
type: integer
196+
minimum: 0
193197
default: 0
194198
title: Offset
195199
- name: uri_pattern
@@ -259,13 +263,15 @@ paths:
259263
required: false
260264
schema:
261265
type: integer
266+
minimum: 0
262267
default: 100
263268
title: Limit
264269
- name: offset
265270
in: query
266271
required: false
267272
schema:
268273
type: integer
274+
minimum: 0
269275
default: 0
270276
title: Offset
271277
- name: order_by
@@ -558,13 +564,15 @@ paths:
558564
required: false
559565
schema:
560566
type: integer
567+
minimum: 0
561568
default: 100
562569
title: Limit
563570
- name: offset
564571
in: query
565572
required: false
566573
schema:
567574
type: integer
575+
minimum: 0
568576
default: 0
569577
title: Offset
570578
- name: order_by
@@ -1002,13 +1010,15 @@ paths:
10021010
required: false
10031011
schema:
10041012
type: integer
1013+
minimum: 0
10051014
default: 100
10061015
title: Limit
10071016
- name: offset
10081017
in: query
10091018
required: false
10101019
schema:
10111020
type: integer
1021+
minimum: 0
10121022
default: 0
10131023
title: Offset
10141024
- name: order_by
@@ -1574,13 +1584,15 @@ paths:
15741584
required: false
15751585
schema:
15761586
type: integer
1587+
minimum: 0
15771588
default: 100
15781589
title: Limit
15791590
- name: offset
15801591
in: query
15811592
required: false
15821593
schema:
15831594
type: integer
1595+
minimum: 0
15841596
default: 0
15851597
title: Offset
15861598
- name: order_by
@@ -1628,13 +1640,15 @@ paths:
16281640
required: false
16291641
schema:
16301642
type: integer
1643+
minimum: 0
16311644
default: 100
16321645
title: Limit
16331646
- name: offset
16341647
in: query
16351648
required: false
16361649
schema:
16371650
type: integer
1651+
minimum: 0
16381652
default: 0
16391653
title: Offset
16401654
- name: tags
@@ -1746,13 +1760,15 @@ paths:
17461760
required: false
17471761
schema:
17481762
type: integer
1763+
minimum: 0
17491764
default: 100
17501765
title: Limit
17511766
- name: offset
17521767
in: query
17531768
required: false
17541769
schema:
17551770
type: integer
1771+
minimum: 0
17561772
default: 0
17571773
title: Offset
17581774
- name: tags
@@ -1858,13 +1874,15 @@ paths:
18581874
required: false
18591875
schema:
18601876
type: integer
1877+
minimum: 0
18611878
default: 100
18621879
title: Limit
18631880
- name: offset
18641881
in: query
18651882
required: false
18661883
schema:
18671884
type: integer
1885+
minimum: 0
18681886
default: 0
18691887
title: Offset
18701888
- name: order_by
@@ -2275,13 +2293,15 @@ paths:
22752293
required: false
22762294
schema:
22772295
type: integer
2296+
minimum: 0
22782297
default: 100
22792298
title: Limit
22802299
- name: offset
22812300
in: query
22822301
required: false
22832302
schema:
22842303
type: integer
2304+
minimum: 0
22852305
default: 0
22862306
title: Offset
22872307
- name: order_by
@@ -2374,13 +2394,15 @@ paths:
23742394
required: false
23752395
schema:
23762396
type: integer
2397+
minimum: 0
23772398
default: 100
23782399
title: Limit
23792400
- name: offset
23802401
in: query
23812402
required: false
23822403
schema:
23832404
type: integer
2405+
minimum: 0
23842406
default: 0
23852407
title: Offset
23862408
- name: order_by
@@ -2427,13 +2449,15 @@ paths:
24272449
required: false
24282450
schema:
24292451
type: integer
2452+
minimum: 0
24302453
default: 100
24312454
title: Limit
24322455
- name: offset
24332456
in: query
24342457
required: false
24352458
schema:
24362459
type: integer
2460+
minimum: 0
24372461
default: 0
24382462
title: Offset
24392463
responses:
@@ -2631,13 +2655,15 @@ paths:
26312655
required: false
26322656
schema:
26332657
type: integer
2658+
minimum: 0
26342659
default: 100
26352660
title: Limit
26362661
- name: offset
26372662
in: query
26382663
required: false
26392664
schema:
26402665
type: integer
2666+
minimum: 0
26412667
default: 0
26422668
title: Offset
26432669
- name: order_by
@@ -2728,13 +2754,15 @@ paths:
27282754
required: false
27292755
schema:
27302756
type: integer
2757+
minimum: 0
27312758
default: 100
27322759
title: Limit
27332760
- name: offset
27342761
in: query
27352762
required: false
27362763
schema:
27372764
type: integer
2765+
minimum: 0
27382766
default: 0
27392767
title: Offset
27402768
responses:
@@ -2970,13 +2998,15 @@ paths:
29702998
required: false
29712999
schema:
29723000
type: integer
3001+
minimum: 0
29733002
default: 100
29743003
title: Limit
29753004
- name: offset
29763005
in: query
29773006
required: false
29783007
schema:
29793008
type: integer
3009+
minimum: 0
29803010
default: 0
29813011
title: Offset
29823012
- name: order_by
@@ -3358,13 +3388,15 @@ paths:
33583388
required: false
33593389
schema:
33603390
type: integer
3391+
minimum: 0
33613392
default: 100
33623393
title: Limit
33633394
- name: offset
33643395
in: query
33653396
required: false
33663397
schema:
33673398
type: integer
3399+
minimum: 0
33683400
default: 0
33693401
title: Offset
33703402
- name: order_by
@@ -3727,13 +3759,15 @@ paths:
37273759
required: false
37283760
schema:
37293761
type: integer
3762+
minimum: 0
37303763
default: 100
37313764
title: Limit
37323765
- name: offset
37333766
in: query
37343767
required: false
37353768
schema:
37363769
type: integer
3770+
minimum: 0
37373771
default: 0
37383772
title: Offset
37393773
- name: order_by

‎tests/api_fastapi/core_api/routes/public/test_assets.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,59 @@ def test_should_respect_page_size_limit_default(self, test_client):
367367
assert response.status_code == 200
368368
assert len(response.json()["assets"]) == 100
369369

370+
@pytest.mark.parametrize(
371+
"query_params, expected_detail",
372+
[
373+
(
374+
{"limit": 1, "offset": -1},
375+
[
376+
{
377+
"type": "greater_than_equal",
378+
"loc": ["query", "offset"],
379+
"msg": "Input should be greater than or equal to 0",
380+
"input": "-1",
381+
"ctx": {"ge": 0},
382+
}
383+
],
384+
),
385+
(
386+
{"limit": -1, "offset": 1},
387+
[
388+
{
389+
"type": "greater_than_equal",
390+
"loc": ["query", "limit"],
391+
"msg": "Input should be greater than or equal to 0",
392+
"input": "-1",
393+
"ctx": {"ge": 0},
394+
}
395+
],
396+
),
397+
(
398+
{"limit": -1, "offset": -1},
399+
[
400+
{
401+
"type": "greater_than_equal",
402+
"loc": ["query", "limit"],
403+
"msg": "Input should be greater than or equal to 0",
404+
"input": "-1",
405+
"ctx": {"ge": 0},
406+
},
407+
{
408+
"type": "greater_than_equal",
409+
"loc": ["query", "offset"],
410+
"msg": "Input should be greater than or equal to 0",
411+
"input": "-1",
412+
"ctx": {"ge": 0},
413+
},
414+
],
415+
),
416+
],
417+
)
418+
def test_bad_limit_and_offset(self, test_client, query_params, expected_detail):
419+
response = test_client.get("/public/assets", params=query_params)
420+
assert response.status_code == 422
421+
assert response.json()["detail"] == expected_detail
422+
370423

371424
class TestGetAssetEvents(TestAssets):
372425
def test_should_respond_200(self, test_client, session):

0 commit comments

Comments
 (0)