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

[8.x] Fix bug discarding input fields with empty validation rules #38563

Merged
merged 2 commits into from
Aug 26, 2021
Merged

[8.x] Fix bug discarding input fields with empty validation rules #38563

merged 2 commits into from
Aug 26, 2021

Conversation

bakerkretzmar
Copy link
Contributor

This PR fixes a breaking change that was introduced with conditional validation rules in v8.55.0.

Before the change in v8.55, if input fields were being validated with an empty list of validation rules, no validation would be performed but those fields would be included in the data returned from $request->validated().

Now, fields with an empty list of validation rules ([] or '') are removed from the list of rules entirely, so matching input fields in the request are not included in $request->validated().

v8.54 and earlier:

validator([
    'name' => 'Jacob', // Input
], [
    'name' => '', // Rules
])->validated();

// ['name' => 'Jacob']

v8.55 and later:

validator([
    'name' => 'Jacob', // Input
], [
    'name' => '', // Rules
])->validated();

// []

Background

The conditional validation rules feature added in v8.55.0 runs filter() on the collection of validation rules so that conditional rules are not applied if the conditional is false.

This unintentionally introduced the side-effect that input data is excluded from the output of validated() for non-conditional rules with empty ([] or '') rules, and for conditional rules where the conditional is false. In the second case, if the only rule on a field is a conditional rule and the condition evaluates to be false, instead of just not running the conditional rules the field will be removed from validated() completely.

I'm not suggesting in this PR that the previous behaviour is or isn't correct (I asked some colleagues about this and most actually agree that the new behaviour is better), but it seems like a breaking change, so I think it would be ideal to avoid making it in a minor release. At Tighten we've already seen multiple applications break as a result of this change.

The code change in this PR is very small and completely backwards-compatible—it just stops filtering out validation rules that are [] or ''. If filtering these fields out is the preferred behaviour, it could easily be reintroduced in v9.

Fixes #38505.

Comment on lines +295 to +296
? array_filter($attributeRules->rules())
: array_filter($attributeRules->defaultRules()), ];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not strictly necessary, but it prevents empty rules inside a conditional from sometimes resulting in a list of rules of just an empty string in an empty array: [''].

@GrahamCampbell GrahamCampbell changed the title Fix bug discarding input fields with empty validation rules [8.x] Fix bug discarding input fields with empty validation rules Aug 26, 2021
@taylorotwell taylorotwell merged commit 1b2107e into laravel:8.x Aug 26, 2021
@bakerkretzmar bakerkretzmar deleted the preserve-input-with-empty-rules branch August 26, 2021 21:18
chu121su12 pushed a commit to chu121su12/framework that referenced this pull request Aug 29, 2021
…ravel#38563)

* Add failing tests

* Fix `ValidationRuleParser` to ignore and preserve empty rules
@jazz7381
Copy link

this make me to change all my empty rule to "sometimes" but its ok

victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
…ravel#38563)

* Add failing tests

* Fix `ValidationRuleParser` to ignore and preserve empty rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation rule parser removes unvalidated data from Form Requests
3 participants