mutations
mutations copied to clipboard
dup default values to prevent leaks across calls
Modifies InputFilter to return a dup of the given default value rather than the value itself. This prevents leaks across multiple calls of a command.
Previously, if the validation or execution logic of a command mutated the default value object, that change was persisted to subsequent calls of the command and could lead to unexpected behavior.
By duping the default value in InputFilter, any command logic that mutates the value is prevented from leaking.
Bump. Any possibility of getting this merged? The build failure, fwiw, is due to an error in the build, not something caused by this PR.
I've been bitten by this problem before, so I'm into this idea. A few notes:
Calling dup
on true, false, or nil raises a TypeError on Ruby < 2.4:
$ RBENV_VERSION=2.3.8 ruby -e "puts true.dup"
-e:1:in `dup': can't dup TrueClass (TypeError)
from -e:1:in `<main>'
$ RBENV_VERSION=2.4.9 ruby -e "puts true.dup"
true
This project still supports Ruby 1.9 (!), so we'll have to avoid calling dup on those values.
There will be a (small) performance penalty from copying the default value on every invocation. If we skip the dup
call for frozen?
values, applications will have a way to avoid the overhead if necessary, and using the frozen string literal magic comment will give the speed boost for free. This behaviour also seems more correct to me, since calling dup
on a frozen object returns an unfrozen one, which is no longer the default that the user provided. It would handle the caveat I mentioned above too, since true, false, and nil are all frozen.
Rebasing against master to pick up https://github.com/cypriss/mutations/commit/36018dec5f0230546a97321c2fda5b584aae97eb should fix CI.
@eugeneius thanks for the feedback! I've rebased and updated the PR with your suggestions :)