From cd408e581bcd40ae1eb0cc30d6abea00551fb5cf Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 20 Jan 2023 22:08:18 +0100 Subject: [PATCH] Catch various validation errors --- app/Api/V1/Controllers/Controller.php | 26 ++++++++++++++++--- app/Api/V2/Controllers/Controller.php | 18 +++++++++++-- app/Http/Controllers/SearchController.php | 6 ++++- app/Http/Requests/ReportFormRequest.php | 7 ++++- .../Authentication/RemoteUserGuard.php | 2 +- app/Support/Request/ConvertsDataTypes.php | 22 +++++++++++----- app/Support/Search/OperatorQuerySearch.php | 2 +- 7 files changed, 68 insertions(+), 15 deletions(-) diff --git a/app/Api/V1/Controllers/Controller.php b/app/Api/V1/Controllers/Controller.php index 75a5f01407..d4a17f1157 100644 --- a/app/Api/V1/Controllers/Controller.php +++ b/app/Api/V1/Controllers/Controller.php @@ -31,10 +31,12 @@ use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Foundation\Validation\ValidatesRequests; use Illuminate\Routing\Controller as BaseController; +use Illuminate\Support\Facades\Log; use League\Fractal\Manager; use League\Fractal\Serializer\JsonApiSerializer; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; +use Symfony\Component\HttpFoundation\Exception\BadRequestException; use Symfony\Component\HttpFoundation\ParameterBag; /** @@ -95,7 +97,13 @@ abstract class Controller extends BaseController // some date fields: $dates = ['start', 'end', 'date']; foreach ($dates as $field) { - $date = request()->query->get($field); + try { + $date = request()->query->get($field); + } catch(BadRequestException $e) { + Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $field)); + Log::error($e->getMessage()); + $value = null; + } $obj = null; if (null !== $date) { try { @@ -111,7 +119,13 @@ abstract class Controller extends BaseController // integer fields: $integers = ['limit']; foreach ($integers as $integer) { - $value = request()->query->get($integer); + try { + $value = request()->query->get($integer); + } catch(BadRequestException $e) { + Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $integer)); + Log::error($e->getMessage()); + $value = null; + } if (null !== $value) { $bag->set($integer, (int)$value); } @@ -129,7 +143,13 @@ abstract class Controller extends BaseController private function getSortParameters(ParameterBag $bag): ParameterBag { $sortParameters = []; - $param = (string)request()->query->get('sort'); + try { + $param = (string)request()->query->get('sort'); + } catch(BadRequestException $e) { + Log::error('Request field "sort" contains a non-scalar value. Value set to NULL.'); + Log::error($e->getMessage()); + $param = ''; + } if ('' === $param) { return $bag; } diff --git a/app/Api/V2/Controllers/Controller.php b/app/Api/V2/Controllers/Controller.php index 4295397932..91a4865019 100644 --- a/app/Api/V2/Controllers/Controller.php +++ b/app/Api/V2/Controllers/Controller.php @@ -32,6 +32,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Routing\Controller as BaseController; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Log; use League\Fractal\Manager; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; @@ -39,6 +40,7 @@ use League\Fractal\Resource\Item; use League\Fractal\Serializer\JsonApiSerializer; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; +use Symfony\Component\HttpFoundation\Exception\BadRequestException; use Symfony\Component\HttpFoundation\ParameterBag; /** @@ -90,7 +92,13 @@ class Controller extends BaseController // some date fields: foreach ($dates as $field) { - $date = request()->query->get($field); + try { + $date = request()->query->get($field); + } catch(BadRequestException $e) { + Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $field)); + Log::error($e->getMessage()); + $value = null; + } $obj = null; if (null !== $date) { try { @@ -105,7 +113,13 @@ class Controller extends BaseController // integer fields: foreach ($integers as $integer) { - $value = request()->query->get($integer); + try { + $value = request()->query->get($integer); + } catch(BadRequestException $e) { + Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $integer)); + Log::error($e->getMessage()); + $value = null; + } if (null !== $value) { $bag->set($integer, (int)$value); } diff --git a/app/Http/Controllers/SearchController.php b/app/Http/Controllers/SearchController.php index 03030e264f..f8ce3ae7ed 100644 --- a/app/Http/Controllers/SearchController.php +++ b/app/Http/Controllers/SearchController.php @@ -106,7 +106,11 @@ class SearchController extends Controller */ public function search(Request $request, SearchInterface $searcher): JsonResponse { - $fullQuery = (string)$request->get('query'); + $entry = $request->get('query'); + if (!is_scalar($entry)) { + $entry = ''; + } + $fullQuery = (string)$entry; $page = 0 === (int)$request->get('page') ? 1 : (int)$request->get('page'); $searcher->parseQuery($fullQuery); diff --git a/app/Http/Requests/ReportFormRequest.php b/app/Http/Requests/ReportFormRequest.php index 16f21d0da0..499de9f21d 100644 --- a/app/Http/Requests/ReportFormRequest.php +++ b/app/Http/Requests/ReportFormRequest.php @@ -215,7 +215,12 @@ class ReportFormRequest extends FormRequest $repository = app(TagRepositoryInterface::class); $set = $this->get('tag'); $collection = new Collection(); - Log::debug('Set is:', $set ?? []); + if (is_array($set)) { + Log::debug('Set is:', $set); + } + if (!is_array($set)) { + Log::error(sprintf('Set is not an array! "%s"', $set)); + } if (is_array($set)) { foreach ($set as $tagTag) { Log::debug(sprintf('Now searching for "%s"', $tagTag)); diff --git a/app/Support/Authentication/RemoteUserGuard.php b/app/Support/Authentication/RemoteUserGuard.php index efe2f9d1de..5f9bb6b1b7 100644 --- a/app/Support/Authentication/RemoteUserGuard.php +++ b/app/Support/Authentication/RemoteUserGuard.php @@ -142,7 +142,7 @@ class RemoteUserGuard implements Guard { Log::debug(sprintf('Now at %s', __METHOD__)); $user = $this->user; - if(null === $user) { + if (null === $user) { Log::debug('User is NULL'); return null; } diff --git a/app/Support/Request/ConvertsDataTypes.php b/app/Support/Request/ConvertsDataTypes.php index 17d217d10f..0999cb2a6e 100644 --- a/app/Support/Request/ConvertsDataTypes.php +++ b/app/Support/Request/ConvertsDataTypes.php @@ -46,10 +46,11 @@ trait ConvertsDataTypes * Abstract method that always exists in the Request classes that use this * trait, OR a stub needs to be added by any other class that uses this train. * - * @param mixed $key + * @param mixed $key * @return mixed */ abstract public function has($key); + /** * Return integer value. * @@ -71,7 +72,11 @@ trait ConvertsDataTypes */ public function convertString(string $field): string { - return $this->clearString((string)($this->get($field) ?? ''), false); + $entry = $this->get($field); + if (!is_scalar($entry)) { + return ''; + } + return $this->clearString((string)$entry, false); } /** @@ -85,7 +90,9 @@ trait ConvertsDataTypes if (null === $string) { return null; } - $search = [ + var_dump($string); + + $search = [ "\0", // NUL "\f", // form feed "\v", // vertical tab @@ -135,14 +142,17 @@ trait ConvertsDataTypes "\u{3000}", // ideographic space "\u{FEFF}", // zero width no -break space ]; - $replace = "\x20"; // plain old normal space - $string = str_replace($search, $replace, $string); + $replace = "\x20"; // plain old normal space + $string = str_replace($search, $replace, $string); + $secondSearch = $keepNewlines ? ["\r"] : ["\r", "\n", "\t", "\036", "\025"]; $string = str_replace($secondSearch, '', $string); // clear zalgo text (TODO also in API v2) $string = preg_replace('/\pM/u', '', $string); - + if (null === $string) { + return null; + } return trim($string); } diff --git a/app/Support/Search/OperatorQuerySearch.php b/app/Support/Search/OperatorQuerySearch.php index 58db390d41..01bbbd9a9e 100644 --- a/app/Support/Search/OperatorQuerySearch.php +++ b/app/Support/Search/OperatorQuerySearch.php @@ -162,7 +162,7 @@ class OperatorQuerySearch implements SearchInterface } catch (TypeError|LogicException $e) { Log::error($e->getMessage()); Log::error(sprintf('Could not parse search: "%s".', $query)); - throw new FireflyException('Invalid search value. See the logs.', 0, $e); + throw new FireflyException(sprintf('Invalid search value "%s". See the logs.', e($query)), 0, $e); } Log::debug(sprintf('Found %d node(s)', count($query1->getNodes())));