Don't nilify non-string values in case_insensitive_keys / strip_whitespace_keys#5841
Open
MahmoudBakr23 wants to merge 1 commit intoheartcombo:mainfrom
Open
Conversation
…keys Previously, apply_to_attribute_or_variable used .try(method) to apply :downcase or :strip to each configured key. When the value did not respond to the method (e.g. an Integer column accidentally listed in config.case_insensitive_keys), .try returned nil and that nil was written back, silently overwriting the column. Gate the assignment on respond_to? so a non-responding value is left untouched. This matches what Devise::ParameterFilter already does for the same keys on the params side. Closes heartcombo#5780
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
When a non-string value (e.g. an integer column) is listed in
config.case_insensitive_keysorconfig.strip_whitespace_keys, the value is silently overwritten withnilduringbefore_validation.Why
Authenticatable#apply_to_attribute_or_variableapplies:downcase/:stripvia.try(method).Object#tryreturnsnilwhen the receiver doesn't respond to the method, and the result is then assigned back to the attribute. So an integer column listed in one of these keys gets nil-ified on every save attempt.This was reported in #5780, where a typo in the config (
%i[email, site_id], which parses to[:"email,", :site_id]) caused:site_id— an integer column — to end up in the list and silently get reset tonil. The misconfiguration is on the user side, but the failure mode is silent data loss rather than a no-op or a clear error, which is what makes it hard to debug (the reporter spent an hour tracing it).Fix
Gate the assignment on
respond_to?so a value that can't handle:downcaseor:stripis left untouched:Devise::ParameterFilter#filtered_hash_by_method_for_given_keysalready takes the same approach on the params side, so this aligns the two paths.Tests
Two tests in
test/models/database_authenticatable_test.rbcover the case where an integer attribute (sign_in_count) is included in the keys list — they assert the value is preserved aftervalid?runs thebefore_validation :downcase_keys/:strip_whitespacecallbacks. Both tests fail onmain(the integer is replaced withnil) and pass with the fix.Closes #5780.