search

datamapper / dm-types

DataMapper plugin providing extra data types
http://datamapper.org/
MIT License
55 stars 80 forks source link

DataMapper::Property::IPAddress attempts to typecase LIKE-queries #53

Open postmodern opened 12 years ago

postmodern commented 12 years ago

I discovered that the IPAddress property will attempt to typecast the values used in .like queries, which results in an ArgumentError being raised by IPAddr.new.

User.all(:sign_in_ip.like => '%192.168.%')
/home/hal/.rvm/rubies/ruby-1.9.3-p0/lib/ruby/1.9.1/ipaddr.rb:496:in `rescue in initialize': invalid address (ArgumentError)
from /home/hal/.rvm/rubies/ruby-1.9.3-p0/lib/ruby/1.9.1/ipaddr.rb:493:in `initialize'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-types-08966f30a9e1/lib/dm-types/ip_address.rb:16:in `new'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-types-08966f30a9e1/lib/dm-types/ip_address.rb:16:in `load'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-types-08966f30a9e1/lib/dm-types/ip_address.rb:30:in `typecast'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query/conditions/comparison.rb:320:in `typecast_property'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query/conditions/comparison.rb:315:in `typecast'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query/conditions/comparison.rb:291:in `initialize'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query/conditions/comparison.rb:60:in `new'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query/conditions/comparison.rb:60:in `new'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1182:in `append_property_condition'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1148:in `append_condition'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1213:in `append_string_condition'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1150:in `append_condition'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1193:in `append_symbol_condition'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1149:in `append_condition'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1219:in `append_operator_conditions'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1151:in `append_condition'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1020:in `block (2 levels) in merge_conditions'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1020:in `each'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1020:in `block in merge_conditions'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1014:in `each'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:1014:in `merge_conditions'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:370:in `update'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/query.rb:386:in `merge'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/model.rb:767:in `scoped_query'
from /home/hal/.rvm/gems/ruby-1.9.3-p0/bundler/gems/dm-core-4bee5b395a41/lib/dm-core/model.rb:342:in `all'

Reproduction: https://github.com/postmodern/dm-bug-report/tree/ip_address_typecast

Reproduced on 1.1.0, 1.2.0 and Edge.

dkubb commented 12 years ago

I'm thinking that maybe we need to add a LikeComparison#typecast that just passes-through the value provided, similar to how RegexpComparison#typecast does.

skord commented 12 years ago

@dkubb Just being a noob here, where does the magic with that comparison happen?

2 cents: Ruby's IPAddr is pretty weak, and doesn't easily allow for important comparisons, like seeing if an IP is in a subnet, getting the subnet mask back from the object after it's set, or other interesting things that ipaddr_extensions and ruby-ip provide. Also, serialization to integer seems to make some more sense than a string.

I would suppose the point of an IP type is getting an IPAddr object back. Is it a bug that you can't compare unlike classes?

postmodern commented 12 years ago

@skord I've written a crude NumericIPAddr Property before. A formal one should probably be added to dm-types, that also packs the netmask as an extra byte. I'm curious if we could mimic the underlying format that MySQL / PostgreSQL uses for native IPs.

postmodern commented 12 years ago

@skord Although switching to a numeric format would make supporting .like impossible.

skord commented 12 years ago

Packing the extra byte? Not thinking about that so much as the efficiency gained from numerical storage and indexing. It's especially apparent when you need to query for all the IP's in a supernet, which may outweigh a text like query when crossing obvious classful subnets. The crux of the issue is deep.