[5.5] Validation bypass for 'before' and 'after' rules when paired with 'date_format' rule. #24191
Conversation
When the rules before and/or after are used together with the date_format rules, the validator should give preference to validate a given field relative to the fixed date specified in the rule, instead of comparing to a field with the same name as the specified date.
|
We don't merge broken tests. Please re-submit with fix. |
|
@tillkruss - to be fair - the contribution documents explicity state he's done the right thing:
|
|
Maybe the docs just need to be reworded though because Taylor consistently says the same about not merging failing tests. |
|
@devcircus I didn't expect this to be merged. It's just bug report with tests attached. Tests are a better way of reporting a bug than simple creating a new issue. |
|
Sure but the common response to these PRs can be confusing. "We can't do anything with this" (though it may be true), doesn't quite reflect the suggestion that this is a legitimate way to report bugs. So maybe the docs could be clarified as to how these cases should be handled. |
|
Could we create an issue for this PR and create label for the issue like "development needed"? |
|
@taylorotwell: Thoughts? |
… 'date_format'. Now when when there is both a 'date_format' rule and a 'before' and/or 'after' rule, the first parameter will be converted to a timestamp, and only if that fails, it will be considered as a name for another field. This exactly the same behavior when there isn't a date_format rule. fixes #24191
|
I think I've fixed it. Added another commit. |
When using the validation rules 'before' and/or 'after' together with the 'date_format' rule, it is possible to have what should be considered invalid data be accepted by the validator as valid.
My understanding from reading the code is that the 'before' and 'after' rules, when used without de 'date_format' rule, will compare two fields relative to each other. e.g. given the rules:
The 'start' field must have a date which is less than the 'finish' field, and the 'finish' field must have a date which is greater than the 'start' field.
But, when the 'date_format' rule is also used, you can compare a field to another field or a specific date. And therein lies the problem. If you know what date the field is being compared to (You can easily guess that through the error message) and all fields from the request is being passed directly to the validator as data to be validated (This is quite common, Laravel itself does that in the 'ValidatesRequests' trait[1]). You can then send an extra field in the request with the same name as that date and a value of your choosing. The validator will then give preference to compare the field being validate relative to the bogus field, which can have an arbitrary date. (the tests added illustrate this more clearly)
I honestly have no idea how to fix this without breaking backwards compatibility, so this pull request only adds tests that should pass but are now failing.
[1]
framework/src/Illuminate/Foundation/Validation/ValidatesRequests.php
Line 24 in e90a6bf