From 2d59d97459e8010c388bb70687e9674321d7c1df Mon Sep 17 00:00:00 2001 From: Nabeel Shahzad <99736+nabeelio@users.noreply.github.com> Date: Sat, 25 Apr 2026 21:21:43 -0500 Subject: [PATCH] fix(airport): validate limits and preload options --- .../Controllers/Api/AirportController.php | 7 +++--- .../Controllers/Frontend/PirepController.php | 12 +++++++++- app/Http/Requests/SearchAirportsRequest.php | 3 +++ tests/Feature/AirportTest.php | 22 +++++++++++++++++++ tests/Feature/SimBriefTest.php | 20 +++++++++++++++++ .../Requests/SearchAirportsRequestTest.php | 15 +++++++++++++ 6 files changed, 75 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Api/AirportController.php b/app/Http/Controllers/Api/AirportController.php index 4c1c1eea..b7c0b5ab 100644 --- a/app/Http/Controllers/Api/AirportController.php +++ b/app/Http/Controllers/Api/AirportController.php @@ -34,7 +34,7 @@ class AirportController extends Controller return AirportResource::collection($airports); } - public function index_hubs(Request $request): AnonymousResourceCollection + public function index_hubs(SearchAirportsRequest $request): AnonymousResourceCollection { $airports = Airport::byHub() ->orderByIcao() @@ -97,8 +97,9 @@ class AirportController extends Controller */ private function perPage(Request $request): int { - $max = config('repository.pagination.limit', 50); + $max = (int) config('repository.pagination.limit', 50); + $limit = (int) $request->query('limit', $max); - return (int) ($request->query('limit') ?? $max); + return min(max($limit, 1), $max); } } diff --git a/app/Http/Controllers/Frontend/PirepController.php b/app/Http/Controllers/Frontend/PirepController.php index 901c5131..c9c3412c 100644 --- a/app/Http/Controllers/Frontend/PirepController.php +++ b/app/Http/Controllers/Frontend/PirepController.php @@ -280,6 +280,16 @@ class PirepController extends Controller } $pirep_source = filled(optional($pirep)->source) ? $pirep->source : PirepSource::MANUAL; + $airports = ['' => '']; + + if ($pirep instanceof Pirep) { + $airports[$pirep->arr_airport->id] = $pirep->arr_airport->full_name; + $airports[$pirep->dpt_airport->id] = $pirep->dpt_airport->full_name; + + if ($pirep->alt_airport_id) { + $airports[$pirep->alt_airport->id] = $pirep->alt_airport->full_name; + } + } return view('pireps.create', [ 'aircraft' => $aircraft, @@ -287,7 +297,7 @@ class PirepController extends Controller 'read_only' => false, 'airline_list' => $this->airlineRepo->selectBoxList(true), 'aircraft_list' => $aircraft_list, - 'airport_list' => [], + 'airport_list' => $airports, 'pirep_fields' => PirepField::whereIn('pirep_source', [$pirep_source, PirepFieldSource::BOTH])->get(), 'field_values' => [], 'fare_values' => $fare_values, diff --git a/app/Http/Requests/SearchAirportsRequest.php b/app/Http/Requests/SearchAirportsRequest.php index f8260afd..7155a8a7 100644 --- a/app/Http/Requests/SearchAirportsRequest.php +++ b/app/Http/Requests/SearchAirportsRequest.php @@ -45,8 +45,11 @@ class SearchAirportsRequest extends FormRequest public function rules(): array { + $maxLimit = (int) config('repository.pagination.limit', 50); + return [ 'search' => ['sometimes', 'string', 'max:255'], + 'limit' => ['sometimes', 'integer', 'min:1', 'max:'.$maxLimit], 'searchFields' => [ 'sometimes', 'string', diff --git a/tests/Feature/AirportTest.php b/tests/Feature/AirportTest.php index dbabcd20..2f3004f1 100644 --- a/tests/Feature/AirportTest.php +++ b/tests/Feature/AirportTest.php @@ -246,6 +246,28 @@ test('airport list honors ?limit query param', function () { ->and($response->json('meta.per_page'))->toBe(3); }); +test('airport list rejects invalid non-positive limits', function () { + $user = User::factory()->create(); + apiAs($user); + + Airport::factory()->count(3)->create(); + + $response = $this->get('/api/airports?limit=0'); + + $response->assertStatus(400); +}); + +test('airport hubs list rejects invalid non-positive limits', function () { + $user = User::factory()->create(); + apiAs($user); + + Airport::factory()->count(3)->create(); + + $response = $this->get('/api/airports/hubs?limit=0'); + + $response->assertStatus(400); +}); + test('GET /api/airports/{airport} resolves lowercase ICAO via route binding', function () { $user = User::factory()->create(); apiAs($user); diff --git a/tests/Feature/SimBriefTest.php b/tests/Feature/SimBriefTest.php index 0a7ccbf4..90894c1d 100644 --- a/tests/Feature/SimBriefTest.php +++ b/tests/Feature/SimBriefTest.php @@ -2,6 +2,7 @@ use App\Models\Acars; use App\Models\Aircraft; +use App\Models\Airport; use App\Models\Enums\AcarsType; use App\Models\Enums\FareType; use App\Models\Enums\UserState; @@ -281,6 +282,25 @@ test('attach to pirep', function () { ->and($briefing->pirep_id)->toEqual($pirep->id); }); +test('simbrief create form preloads selected airports', function () { + Airport::factory()->create(['id' => 'OMAA', 'icao' => 'OMAA', 'name' => 'Abu Dhabi International']); + Airport::factory()->create(['id' => 'OMDB', 'icao' => 'OMDB', 'name' => 'Dubai International']); + + $userinfo = createUserData(); + $user = $userinfo['user']; + $briefing = loadSimBrief($user, $userinfo['aircraft']->first(), []); + + $response = $this->actingAs($user)->get('/pireps/create?sb_id='.$briefing->id); + + $response->assertOk() + ->assertViewHas('airport_list', function (array $airportList): bool { + return ($airportList[''] ?? null) === '' + && ($airportList['OMAA'] ?? null) === 'OMAA - Abu Dhabi International' + && ($airportList['OMDB'] ?? null) === 'OMDB - Dubai International' + && count($airportList) === 3; + }); +}); + test('clear expired briefs', function () { $userinfo = createUserData(); $user = $userinfo['user']; diff --git a/tests/Unit/Http/Requests/SearchAirportsRequestTest.php b/tests/Unit/Http/Requests/SearchAirportsRequestTest.php index d8196e44..fec96a27 100644 --- a/tests/Unit/Http/Requests/SearchAirportsRequestTest.php +++ b/tests/Unit/Http/Requests/SearchAirportsRequestTest.php @@ -48,6 +48,10 @@ test('SearchAirportsRequest passes with legacy multi-column orderBy', function ( ])->passes())->toBeTrue(); }); +test('SearchAirportsRequest passes with limit inside configured page size', function () { + expect(validateSearchAirports(['limit' => (string) config('repository.pagination.limit')])->passes())->toBeTrue(); +}); + test('SearchAirportsRequest passes with additional legacy sortable columns', function () { expect(validateSearchAirports(['orderBy' => 'notes'])->passes())->toBeTrue(); }); @@ -71,3 +75,14 @@ test('SearchAirportsRequest rejects sortedBy outside asc/desc', function () { test('SearchAirportsRequest rejects search longer than 255 chars', function () { expect(validateSearchAirports(['search' => str_repeat('x', 256)])->fails())->toBeTrue(); }); + +test('SearchAirportsRequest rejects non-positive limit values', function () { + expect(validateSearchAirports(['limit' => '0'])->fails())->toBeTrue() + ->and(validateSearchAirports(['limit' => '-1'])->fails())->toBeTrue(); +}); + +test('SearchAirportsRequest rejects limit above configured page size', function () { + $tooLarge = (string) (config('repository.pagination.limit') + 1); + + expect(validateSearchAirports(['limit' => $tooLarge])->fails())->toBeTrue(); +});