From c204533195e25b4aac4dc520a278fb7da4d7ccbe Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 26 Jan 2025 06:30:38 +0100 Subject: [PATCH] Fix various API 500 errors. --- app/Exceptions/Handler.php | 5 ++- app/Rules/UniqueAccountNumber.php | 5 +++ app/Rules/UniqueIban.php | 4 ++ .../Http/Controllers/ChartGeneration.php | 2 +- app/Validation/FireflyValidator.php | 42 +++++++++++-------- resources/lang/en_US/validation.php | 1 + 6 files changed, 39 insertions(+), 20 deletions(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index af70410cbc..9c6399230c 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -24,6 +24,7 @@ declare(strict_types=1); namespace FireflyIII\Exceptions; +use Brick\Math\Exception\NumberFormatException; use FireflyIII\Jobs\MailError; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Auth\AuthenticationException; @@ -133,10 +134,10 @@ class Handler extends ExceptionHandler return response()->json(['message' => $e->getMessage(), 'exception' => 'BadHttpHeaderException'], $e->statusCode); } - if($e instanceof ValidationException && $expectsJson) { + if(($e instanceof ValidationException || $e instanceof NumberFormatException) && $expectsJson) { $errorCode = 422; return response()->json( - ['message' => sprintf('Validation exception: %s', $e->getMessage()), 'errors' => ['date' => 'Date is invalid']], + ['message' => sprintf('Validation exception: %s', $e->getMessage()), 'errors' => ['field' => 'Field is invalid']], $errorCode ); } diff --git a/app/Rules/UniqueAccountNumber.php b/app/Rules/UniqueAccountNumber.php index 422ed44604..54673e6dbe 100644 --- a/app/Rules/UniqueAccountNumber.php +++ b/app/Rules/UniqueAccountNumber.php @@ -79,6 +79,11 @@ class UniqueAccountNumber implements ValidationRule if (null === $this->expectedType) { return; } + if(is_array($value)) { + $fail('validation.generic_invalid')->translate(); + return; + } + $value = (string) $value; $maxCounts = $this->getMaxOccurrences(); foreach ($maxCounts as $type => $max) { diff --git a/app/Rules/UniqueIban.php b/app/Rules/UniqueIban.php index 5c7282648f..0a9328e1a1 100644 --- a/app/Rules/UniqueIban.php +++ b/app/Rules/UniqueIban.php @@ -94,6 +94,10 @@ class UniqueIban implements ValidationRule if (0 === count($this->expectedTypes)) { return true; } + if(is_array($value)) { + return false; + } + $value = (string) $value; $maxCounts = $this->getMaxOccurrences(); foreach ($maxCounts as $type => $max) { diff --git a/app/Support/Http/Controllers/ChartGeneration.php b/app/Support/Http/Controllers/ChartGeneration.php index 2dcc43fe18..93c2a238eb 100644 --- a/app/Support/Http/Controllers/ChartGeneration.php +++ b/app/Support/Http/Controllers/ChartGeneration.php @@ -90,7 +90,7 @@ trait ChartGeneration $balance = $range[$format] ?? $previous; $previous = $balance; $currentStart->addDay(); - $currentSet['entries'][$label] = $balance[$field]; + $currentSet['entries'][$label] = $balance[$field] ?? '0'; } $chartData[] = $currentSet; } diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index cb2769a439..bee4aab639 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -38,6 +38,7 @@ use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use FireflyIII\Services\Password\Verifier; use FireflyIII\Support\ParseDateString; use FireflyIII\User; +use Illuminate\Support\Facades\Log; use Illuminate\Validation\Validator; use PragmaRX\Google2FA\Exceptions\IncompatibleWithGoogleAuthenticatorException; use PragmaRX\Google2FA\Exceptions\InvalidCharactersException; @@ -66,7 +67,7 @@ class FireflyValidator extends Validator } $user = auth()->user(); if (null === $user) { - app('log')->error('No user during validate2faCode'); + Log::error('No user during validate2faCode'); return false; } @@ -104,6 +105,9 @@ class FireflyValidator extends Validator */ public function validateBic(mixed $attribute, mixed $value): bool { + if(!is_string($value) || strlen($value) < 8) { + return false; + } $regex = '/^[a-z]{6}[0-9a-z]{2}([0-9a-z]{3})?\z/i'; $result = preg_match($regex, $value); if (false === $result || 0 === $result) { @@ -120,7 +124,7 @@ class FireflyValidator extends Validator } $user = auth()->user(); if (null === $user) { - app('log')->error('No user during validate2faCode'); + Log::error('No user during validate2faCode'); return false; } @@ -212,8 +216,8 @@ class FireflyValidator extends Validator $checksum = bcmod($iban, '97'); } catch (\ValueError $e) { // @phpstan-ignore-line $message = sprintf('Could not validate IBAN check value "%s" (IBAN "%s")', $iban, $value); - app('log')->error($message); - app('log')->error($e->getTraceAsString()); + Log::error($message); + Log::error($e->getTraceAsString()); return false; } @@ -427,7 +431,7 @@ class FireflyValidator extends Validator try { $parser->parseDate($value); } catch (FireflyException $e) { - app('log')->error($e->getMessage()); + Log::error($e->getMessage()); return false; } @@ -467,41 +471,45 @@ class FireflyValidator extends Validator */ public function validateUniqueAccountForUser($attribute, $value, $parameters): bool { + if(is_array($value)) { + Log::debug('$value is an array, always return false', $value); + return false; + } // because a user does not have to be logged in (tests and what-not). if (!auth()->check()) { - app('log')->debug('validateUniqueAccountForUser::anon'); + Log::debug('validateUniqueAccountForUser::anon'); return $this->validateAccountAnonymously(); } if (array_key_exists('objectType', $this->data)) { - app('log')->debug('validateUniqueAccountForUser::typeString'); + Log::debug('validateUniqueAccountForUser::typeString'); return $this->validateByAccountTypeString($value, $parameters, $this->data['objectType']); } - if (array_key_exists('type', $this->data)) { - app('log')->debug('validateUniqueAccountForUser::typeString'); + if (array_key_exists('type', $this->data) && !is_array($this->data['type'])) { + Log::debug('validateUniqueAccountForUser::typeString'); return $this->validateByAccountTypeString($value, $parameters, (string) $this->data['type']); } if (array_key_exists('account_type_id', $this->data)) { - app('log')->debug('validateUniqueAccountForUser::typeId'); + Log::debug('validateUniqueAccountForUser::typeId'); return $this->validateByAccountTypeId($value, $parameters); } $parameterId = $parameters[0] ?? null; if (null !== $parameterId) { - app('log')->debug('validateUniqueAccountForUser::paramId'); + Log::debug('validateUniqueAccountForUser::paramId'); return $this->validateByParameterId((int) $parameterId, $value); } if (array_key_exists('id', $this->data)) { - app('log')->debug('validateUniqueAccountForUser::accountId'); + Log::debug('validateUniqueAccountForUser::accountId'); return $this->validateByAccountId($value); } // without type, just try to validate the name. - app('log')->debug('validateUniqueAccountForUser::accountName'); + Log::debug('validateUniqueAccountForUser::accountName'); return $this->validateByAccountName($value); } @@ -642,11 +650,11 @@ class FireflyValidator extends Validator } $type = $this->data['objectType'] ?? 'unknown'; if ('expense' !== $type && 'revenue' !== $type) { - app('log')->warning(sprintf('Account number "%s" is not unique and account type "%s" cannot share its account number.', $value, $type)); + Log::warning(sprintf('Account number "%s" is not unique and account type "%s" cannot share its account number.', $value, $type)); return false; } - app('log')->debug(sprintf('Account number "%s" is not unique but account type "%s" may share its account number.', $value, $type)); + Log::debug(sprintf('Account number "%s" is not unique but account type "%s" may share its account number.', $value, $type)); // one other account with this account number. /** @var AccountMeta $entry */ @@ -654,11 +662,11 @@ class FireflyValidator extends Validator $otherAccount = $entry->account; $otherType = (string) config(sprintf('firefly.shortNamesByFullName.%s', $otherAccount->accountType->type)); if (('expense' === $otherType || 'revenue' === $otherType) && $otherType !== $type) { - app('log')->debug(sprintf('The other account with this account number is a "%s" so return true.', $otherType)); + Log::debug(sprintf('The other account with this account number is a "%s" so return true.', $otherType)); return true; } - app('log')->debug(sprintf('The other account with this account number is a "%s" so return false.', $otherType)); + Log::debug(sprintf('The other account with this account number is a "%s" so return false.', $otherType)); } return false; diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php index 44640c380c..4ae0fa6033 100644 --- a/resources/lang/en_US/validation.php +++ b/resources/lang/en_US/validation.php @@ -144,6 +144,7 @@ return [ 'numeric_native' => 'The native amount must be a number.', 'numeric_destination' => 'The destination amount must be a number.', 'numeric_source' => 'The source amount must be a number.', + 'generic_invalid' => 'This value is invalid.', 'regex' => 'The :attribute format is invalid.', 'required' => 'The :attribute field is required.', 'required_if' => 'The :attribute field is required when :other is :value.',