Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #8190 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

[PATCH] Check to ensure that xml_value is a Hash before accessing an attribute

Reported by: al3x Assigned to: core
Priority: high Milestone: 1.2.4
Component: ActiveSupport Version: edge
Severity: normal Keywords: xml_value
Cc:

Description

Fixes up XML conversion. See attached patch.

Attachments

check_for_hash_when_returning_xml_value_file.diff (0.8 kB) - added by al3x on 04/25/07 22:07:22.
patch against 6579
check_for_hash_when_returning_xml_value_file_with_tests.diff (1.3 kB) - added by al3x on 04/26/07 00:41:10.
as above, with tests

Change History

04/25/07 22:07:22 changed by al3x

  • attachment check_for_hash_when_returning_xml_value_file.diff added.

patch against 6579

04/25/07 22:09:54 changed by al3x

  • priority changed from normal to high.

This is blocking our deploy. So, high priority.

04/25/07 23:25:44 changed by bitsweat

  • status changed from new to closed.
  • resolution set to untested.
  • milestone changed from 1.x to 1.2.4.

Please add a unit test demonstrating the fix.

04/26/07 00:13:51 changed by al3x

Will include a test. For the record, here's my broken test output (coming from our integration tests): 

NoMethodError: You have a nil object when you didn't expect it!
You might have expected an instance of Array.
The error occurred while evaluating nil.[]
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:176:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/inflector.rb:250:in `inject'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:183:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:183:in `map!'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:183:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/inflector.rb:250:in `inject'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/inflector.rb:250:in `inject'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    /Users/al3x/src/twitter/vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/hash/conversions.rb:143:in `from_xml'
    ./test/integration/api_test.rb:262:in `deserialise_content'
 ...

04/26/07 00:40:18 changed by al3x

  • status changed from closed to reopened.
  • resolution deleted.

The follow *existing tests* break without this fix: 

  1) Error:
test_multiple_records_from_xml(HashToXmlTest):
NoMethodError: undefined method `[]' for nil:NilClass
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:176:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/inflector.rb:250:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:183:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:183:in `map!'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:183:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/inflector.rb:250:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/inflector.rb:250:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:143:in `from_xml'
    ./activesupport/test/core_ext/hash_ext_test.rb:494:in `test_multiple_records_from_xml'

  2) Error:
test_single_record_from_xml(HashToXmlTest):
NoMethodError: undefined method `[]' for nil:NilClass
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:176:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/inflector.rb:250:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/inflector.rb:250:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:143:in `from_xml'
    ./activesupport/test/core_ext/hash_ext_test.rb:419:in `test_single_record_from_xml'

  3) Error:
test_single_record_from_xml_with_nil_values(HashToXmlTest):
NoMethodError: undefined method `[]' for nil:NilClass
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:176:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/inflector.rb:250:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:170:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/inflector.rb:250:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `each'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `inject'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:169:in `typecast_xml_value'
    ./activesupport/test/core_ext/../../lib/active_support/core_ext/hash/conversions.rb:143:in `from_xml'
    ./activesupport/test/core_ext/hash_ext_test.rb:445:in `test_single_record_from_xml_with_nil_values'

04/26/07 00:41:10 changed by al3x

  • attachment check_for_hash_when_returning_xml_value_file_with_tests.diff added.

as above, with tests

04/26/07 01:17:13 changed by al3x

  • status changed from reopened to closed.
  • resolution set to fixed.

Worked around in [6582].

04/26/07 01:20:25 changed by bitsweat

Tightened up in [6583]. Thanks Alex.