プロジェクト

全般

プロフィール

Vote #74660

完了

Validating a Setting with invalid name triggers an error

Admin Redmine さんが約4年前に追加. 約4年前に更新.

ステータス:
Closed
優先度:
通常
担当者:
-
カテゴリ:
Code cleanup/refactoring_30
対象バージョン:
開始日:
2022/05/09
期日:
進捗率:

0%

予定工数:
category_id:
30
version_id:
75
issue_org_id:
15551
author_id:
35734
assigned_to_id:
1
comments:
4
status_id:
5
tracker_id:
1
plus1:
0
affected_version:
closed_on:
affected_version_id:
70
ステータス-->[Closed]

説明

In case you want directly add a setting by insert through the model you get an error

insert the following test case into setting_test.rb

def test_insert_name_not_in_settings_yml
setting = Setting.new(:name => "does_not_exist", :value => "should_not_be_allowed" )
assert !setting.save
end

Then you get the errors below:

test_insert_name_not_in_settings_yml(SettingTest):
NoMethodError: undefined method `[]' for nil:NilClass
    /redmine/app/models/setting.rb:86:in `block in '
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:425:in `_run__2686202268941026958__validate__1184212570148052443__callbacks'
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:405:in `__run_callback'
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:385:in `_run_validate_callbacks'
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /usr/lib64/ruby/gems/1.9.1/gems/activemodel-3.2.15/lib/active_model/validations.rb:228:in `run_validations!'
    /usr/lib64/ruby/gems/1.9.1/gems/activemodel-3.2.15/lib/active_model/validations/callbacks.rb:53:in `block in run_validations!'
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:403:in `_run__2686202268941026958__validation__1184212570148052443__callbacks'
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:405:in `__run_callback'
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:385:in `_run_validation_callbacks'
    /usr/lib64/ruby/gems/1.9.1/gems/activesupport-3.2.15/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /usr/lib64/ruby/gems/1.9.1/gems/activemodel-3.2.15/lib/active_model/validations/callbacks.rb:53:in `run_validations!'
    /usr/lib64/ruby/gems/1.9.1/gems/activemodel-3.2.15/lib/active_model/validations.rb:195:in `valid?'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/validations.rb:69:in `valid?'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/validations.rb:77:in `perform_validations'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/validations.rb:50:in `save'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/attribute_methods/dirty.rb:22:in `save'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/transactions.rb:259:in `block (2 levels) in save'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/transactions.rb:313:in `block in with_transaction_returning_status'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/connection_adapters/abstract/database_statements.rb:192:in `transaction'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/transactions.rb:208:in `transaction'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/transactions.rb:311:in `with_transaction_returning_status'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/transactions.rb:259:in `block in save'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/transactions.rb:270:in `rollback_active_record_state!'
    /usr/lib64/ruby/gems/1.9.1/gems/activerecord-3.2.15/lib/active_record/transactions.rb:258:in `save'
    test/unit/setting_test.rb:93:in `test_insert_name_not_in_settings_yml'
    /usr/lib64/ruby/gems/1.9.1/gems/mocha-0.14.0/lib/mocha/integration/mini_test/version_230_to_2101.rb:36:in `run'

Same if you call the following on console

s = Setting.new(:name => "does_not_exist", :value => "should_not_be_allowed" )
s.valid?

The reason is in the app/model/setting.rb

validates_numericality_of :value, :only_integer => true, :if => Proc.new { |setting| @@available_settings[setting.name]['format'] == 'int' }

The Proc breaks if name (the key) is not in the hash.

Fix for it is to check if key is in hash inside the proc before using the non existing key so that check of numericality is skipped when key does not exist:

validates_numericality_of :value, :only_integer => true, :if => Proc.new { |setting| @@available_settings.has_key?(setting) ? @@available_settings[setting.name]['format'] == 'int' : false}

journals

Yes, the fix is trivial. Could you attach a patch with the fix and the test ?
--------------------------------------------------------------------------------
Sorry that I forgot that, Jean-Philippe:
Attached you can find the patch.

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
With this patch applied, integer settings are no longer validated:
@@@available_settings.has_key?(setting)@ always returns false, should be @@@available_settings.has_key?(setting.name)@ instead.

It's fixed with an additional test in r12348, thanks for pointing this out.
--------------------------------------------------------------------------------

Admin Redmine さんが約4年前に更新

  • カテゴリCode cleanup/refactoring_30 にセット
  • 対象バージョン2.5.0_75 にセット

他の形式にエクスポート: Atom PDF

いいね!0
いいね!0