Repository navigation
Conversation
|
the windows CI failure is unrelated to this my changes. every failing job fails at the setup-php step before any tests run. whereas the Linux/macOS jobs pass. maybe a runner/action issue, re-running may resolve it. |
your PR fixes the non-string-scalar gap, but that only patches $v1 = Validator::make(['x' => ['0e123']], ['x' => 'contains:0']);
$v2 = Validator::make(['x' => ['0e123']], ['x' => 'doesnt_contain:0']);
$v1->passes(); // true, contains is still loose, "0" == "0e123" numerically
$v2->passes(); // true, value gets stringified here, but "0" !== "0e123" strictlyBoth pass again, same as the original bug report, just with 0e123 instead of true.. Also worth noting the value-side cast alone doesn't cover array-rule syntax ([['doesnt_contain', 1]]) since the param itself can arrive as a non-string there too and never gets normalized. and my PR patches both rules symmetrically so this can't reopen from the other side. |
|
Thanks for your pull request to Laravel! Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include. If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions! |
2 participants
Closes #61491:
containswas using loosein_array()whiledoesnt_containused strict. they're supposed to be logical opposites, but this inconsistency let both pass on the same input - e.g.flags => [true]withcontains:1anddoesnt_contain:1both pass, because"1" == trueloosely but"1" !== truestrictly (ValidatesAttributes.php: L568-L591). kindly check the issue #61491, I've added more detailed report there.This was actually fixed once (#61318 made
doesnt_containstrict, #61320 tried the same forcontains) but #61320 got reverted later. because going strict without normalizing types broke matches like[1](int) againstcontains:1(string param).Changes I made
in this fix, I've normalize the array values and the rule params to strings, and then compare strictly.. like
inrule already uses (#61146 by @crynobone). This closes the gap which #61320 hit, while keeping the strict comparison that #61318 needed (numeric-string collisions like'0e123'vs0stay correctly non-matching).Added tests covering: numeric-string edge cases (
'0e123'vs0) staying non-matching for both rules, non-string-scalar values matching via string-syntax, the same via array-rule syntax (['contains', 1]) - since that path hands parameters without string-casting them, unlike string-rule syntax, non-scalar params (e.g. an array) failing gracefully instead of throwing.Possible breaking change after this fix: normalizing values to strings means a few loose-comparison matches that worked before, now won't work:
[false]withcontains:0- used to pass (false == "0"), now fails ((string) falseis"", not"0")[1.0]withcontains:1.0- used to pass, now fails on format mismatch unless the param string matches PHP's exact float-to-string outputBoth are edge cases that only worked before because of the loose-comparison bug this PR fixes - anyone relying on them was actually relying on an inconsistency between two rules that are documented as opposites. this PR just fixes the bug. and the alternative (leaving
containsanddoesnt_containable to both pass on the same input) is a worse correctness bug than losing this narrow edge-case match.Still, if this looks problematic to merge into
13.x, I can submit this for themasteralso. please let me know which one is preferable.Note: this PR desp and all the changes I made is not AI-made and I've tested+reviewed my changes carefully. so if any further modification needed in this approach or any better approach/suggestion u have in mind for this issue, kindly let me know. thank you.