Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Before/After validation rules ignore the validation state of the dependent field #49863

Closed
naquad opened this issue Jan 26, 2024 · 3 comments · Fixed by #49871
Closed

Before/After validation rules ignore the validation state of the dependent field #49863

naquad opened this issue Jan 26, 2024 · 3 comments · Fixed by #49871
Labels

Comments

@naquad
Copy link
Contributor

naquad commented Jan 26, 2024

Laravel Version

10.34.2

PHP Version

8.3.2

Database Driver & Version

No response

Description

Validator's before and after rules are trying to construct the second operand from non-validated potentially invalid data (ValidatesAttributes::compareDates() is retrieving the data using Validator::getValue() which returns raw data). This causes unexpected exceptions.

Steps To Reproduce

Sample script

<?php

require_once(__DIR__ . '/vendor/autoload.php');
$app = require __DIR__.'/bootstrap/app.php';
$app->make(Illuminate\Contracts\Console\Kernel::class)->bootstrap();

use Illuminate\Support\Facades\Validator;

$data = [
    'dt1' => ['this' => ['is' => ['a' => ['mess']]]],
    'dt2' => 'some invalid date',
];

$val = Validator::make($data, [
    'dt1' => 'required|date',
    'dt2' => 'required|date|before:dt1',
]);

$val->validate();

Output


   TypeError 

  DateTime::__construct(): Argument #1 ($datetime) must be of type string, array given

  at vendor/nesbot/carbon/src/Carbon/Traits/Creator.php:89
     85▕             setlocale(LC_NUMERIC, 'C'); // @codeCoverageIgnore
     86▕         }
     87▕ 
     88▕         try {
  ➜  89▕             parent::__construct($time ?: 'now', static::safeCreateDateTimeZone($tz) ?: null);
     90▕         } catch (Exception $exception) {
     91▕             throw new InvalidFormatException($exception->getMessage(), 0, $exception);
     92▕         }
     93▕ 

      +13 vendor frames 

  14  poc.php:19
      Illuminate\Validation\Validator::validate()

Expected result

Validation exception stating that both values are invalid.

Actual result

TypeError fatal error.

@naquad
Copy link
Contributor Author

naquad commented Feb 23, 2024

Please reopen the issue as this has not been actually fixed. The PR #49871 has been reverted.

@driesvints
Copy link
Member

@naquad since it seems not possible right now to solve this we're not going to make any changes here. If you want, you can attempt a PR at the docs to explain this gotcha.

@naquad
Copy link
Contributor Author

naquad commented Feb 23, 2024

I'm trying to wrap my head around all related cases and the issue that has caused the reversal.

The issues are:
#49955
#49988
#49994

In summary, they are about the null behavior which is something like this (but I'm not sure about it): null in the comparison always yields true (no idea why, it's just the expectation).

There's also some strange syntax like after:field +1 day which I haven't seen before and the docs are not mentioning it.

Am I getting it right that the fix has been blocked by some new feature?

This is not a "gotcha". That's a bug. The Validator must be bulletproof, otherwise malicious input may cause some epic failures.

A case: mail log channel for errors is configured and someone is sending dozens (not even thousands) of requests with malicious payload causing you to lose the mail channel effectively as your emails will be throttled/marked as spam very quickly.

I honestly doubt someone has a special test that tries to stuff random invalid payloads into all validation rules of every endpoint they have with the corresponding extra handling for the corner cases.

Everyone relies on the fact that the Validator can't broken by the input and will return the invariant of the data that can be handled by the user code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants