Fix callMethodOnDumper treating false config values as no-argument calls#1961
Merged
freekmurze merged 2 commits intospatie:mainfrom Mar 8, 2026
Merged
Conversation
When a dump config option like skip_ssl is set to false, callMethodOnDumper
was calling the method with no arguments because of the falsy check:
if (! $methodValue) { $dbDumper->$methodName(); }
setSkipSsl() has a default parameter of true, so calling it with no
arguments sets skipSsl to true even when the config says false.
Change the check to $methodValue === null so only genuinely absent
values (flag-style options like use_single_transaction) invoke
the no-argument form. Falsy values like false and 0 are now passed
through correctly.
Fixes: spatie#1946
The 'custom dumpers' test registers a MongoDb factory for 'mysql' via the static $custom property, which persisted into subsequent tests causing the skip_ssl test to get a MongoDb instance instead of MySql. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
Thanks for the fix and the clear write-up, @isaackaara! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
When a dump config option is set to
false,callMethodOnDumperwas calling the method with no arguments because of the falsy check:The issue is
setSkipSsl()(and similar methods on DbDumper) havebool $param = trueas the default parameter. So calling them with no arguments sets the value totrueeven when the config explicitly saysfalse.Reproduction:
The dumper ends up with
skipSsl = truebecause:falseprocessExtraDumpParametersthen processes the dump config again, hittingskip_ssl => falsecallMethodOnDumpersees!false === trueand callssetSkipSsl()with no args → defaults totrueFix
Change the check from
! $methodValueto$methodValue === null. Flag-style options (likeuse_single_transaction) produce anull$methodValue fromprocessExtraDumpParameters, so those still call the no-argument form. Falsy values likefalseand0are now passed through correctly.Test added
Added a test that confirms
skip_ssl => falsein the dump config results inskipSslbeingfalseon the dumper.Relates to: #1946