Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• 

  Rohit Arondekar May 17th, 2010 @ 05:12 AM

  I've attached a patch.

  P.S rubydiamond from #railsbridge has confirmed that the tests pass in 1.8.7, so it might be something that needs to be looked into more carefully.

  • 4622_patch.diff 1.3 KB

• You flagged this item as spam.
  

  Anil Wadghule May 17th, 2010 @ 09:59 AM

  +1

  Verified patch on ruby-1.8.7-p249 and ruby-1.9.2-head.

  Still a question I have is, why 1.9.2 considers 0xdeadbeef as valid number but 1.8.7 does not.

  When inspected in irb for both rubies, it shows

  ruby-1.8.7-p249 >   0xdeadbeef.class
   => Bignum 
  ruby-1.8.7-p249 > 0xdeadbeef
   => 3735928559
  

  ruby-1.9.2-head > 0xdeadbeef.class
   => Bignum 
  ruby-1.9.2-head > 0xdeadbeef
   => 3735928559
  

  Need to know reason for putting this hex number 0xdeadbeef as not valid numerical test. Thoughts?

• You flagged this item as spam.
  

  Anil Wadghule May 17th, 2010 @ 10:10 AM

  Looks like 1.9.2 is converting hex number to float but 1.8.7 don't

  ruby-1.8.7-p249 > Kernel.Float("0xdeadbeef")
  ArgumentError: invalid value for Float(): "0xdeadbeef"
      from (irb):1:in `Float'
      from (irb):1
  

  ruby-1.9.2-head > Kernel.Float("0xdeadbeef")
   => 3735928559.0
  

  +1 for the patch

• 

  Rohit Arondekar May 17th, 2010 @ 11:28 AM

  I think I've got it! Take a look at the following code from activemodel/lib/active_model/validations/numericality.rb

  
  

    def parse_raw_value_as_a_number(raw_value)
      begin
        Kernel.Float(raw_value)
      rescue ArgumentError, TypeError
        nil
      end
    end
  
    def parse_raw_value_as_an_integer(raw_value)
      raw_value.to_i if raw_value.to_s =~ /\A[+-]?\d+\Z/
    end
  

  Note how a raw integer value is parsed. Using a regex, most probably because Kernel.Integer honors radix indicators like 0x.

  Whereas a raw float is parsed using Kernel.Float which until 1.9.2 (or maybe 1.9.1) didn't honor radix indicators. But now that it does on 1.9.2, bam! it accepts a number which was not meant to be accepted. That's my understanding so far about this issue.

• 

  Rohit Arondekar May 17th, 2010 @ 11:52 AM

  I can confirm that ruby 1.9.1p378 (2010-01-10 revision 26273) [x86_64-linux] behaves like 1.8.7 but 1.9.2 doesn't. I believe I've found the changeset that did this => http://redmine.ruby-lang.org/repositories/revision/ruby-19?rev=26965 and it's associated feature => http://redmine.ruby-lang.org/issues/show/2969 but the discussions are in Japanese :(

  I can't make any more progress on this issue as I think I've reached a dead-end, I don't plan on digging in the Ruby source code :P. Hopefully a Core member or somebody who knows how the validator should behave can shed some light.

• 

  Rizwan Reza May 17th, 2010 @ 12:08 PM

  • Milestone cleared.
  • Tag changed from “rails 3.0 activemodel activerecord tests, bugmash” to “bugmash-review”
  • State changed from “new” to “verified”
• 

  José Valim May 17th, 2010 @ 12:33 PM

  • Assigned user changed from “Pratik” to “José Valim”

  Great work debugging guys!

  The fix is not changing the test to use "0xdeadbeet". I doubt our applications should allow hexadecimals entries. IMHO, we should simply change the code to check if the number does not start with "0x". If it does, it should be marked as not a number.

• You flagged this item as spam.
  

  Anil Wadghule May 17th, 2010 @ 02:05 PM

  I've attached a patch which fixes the broken numericality tests for ruby-1.9.2-head. Tests pass on ruby-1.8.7-p249 too.

  • fix_numericality_check_code_4622.diff 916 Bytes
• 

  Repository May 17th, 2010 @ 03:59 PM

  • State changed from “verified” to “resolved”

  (from [5371242384171dc0255716e31e9257ddeec17d10]) Valid hex strings aren't valid float column values, to match the integer restriction. [#4622 state:resolved] http://github.com/rails/rails/commit/5371242384171dc0255716e31e9257...

• 

  Rizwan Reza May 17th, 2010 @ 04:17 PM

  • Tag cleared.
• 

  Rohit Arondekar May 18th, 2010 @ 01:42 AM

  • Tag set to “bugmash”

  I've attached a patch. First only a failing test to show that the commit doesn't block hex numbers of the form 0X22 and another patch to fix the code (with the test).

  • 4622_am_failing_test.diff 1.3 KB
  • 4622_fix_code.diff 2.2 KB
• 

  Ryan Bigg May 18th, 2010 @ 01:54 AM

  • Tag changed from “bugmash” to “bugmash, bugmash-review”
  • State changed from “resolved” to “open”
• 

  Repository May 18th, 2010 @ 02:19 AM

  (from [05e3fb45eeacd20f2b8b5691f17d6fdf2fb4582b]) Add a valid hex that shouldn't be valid to ActiveModel numericality tests [#4622 state:commited]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/05e3fb45eeacd20f2b8b5691f17d6f...

• 

  Repository May 18th, 2010 @ 02:19 AM

  (from [8e3c3b06dc8ff8842f6390efc58eaf4bb1a23060]) Fixed numericality validator in ActiveModel to reject hex numbers for floats completely [#4622 state:commited]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/8e3c3b06dc8ff8842f6390efc58eaf...

• 

  Ryan Bigg May 19th, 2010 @ 10:21 AM

  • State changed from “open” to “committed”
• 

  Rizwan Reza June 6th, 2010 @ 09:29 AM

  • Tag changed from “bugmash, bugmash-review” to “bugmash”
• 

  Rizwan Reza June 6th, 2010 @ 09:30 AM

  • Tag cleared.
• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”
  • Importance changed from “” to “Low”

  [bulk edit]

• 

  Jeff Kreeftmeijer November 8th, 2010 @ 08:48 AM

  Automatic cleanup of spam.

• You flagged this item as spam.
  

  teiddy November 30th, 2010 @ 05:54 AM

  Instructions: download and untar to /sites/all/modules

  also - Download and install 'Rules' module to /sites/all/modules

  Run /update.php

  Goto Admin>Build>Modules, enable "OA Single Group Login Redirect" module and allow Rules module to be activated when prompted.

  Log-out as admin and log-in as user with 1 group - page redirects as expected.

  Greyside Thank-you!
  

  Thank you for this information,I like it very much,Would you lik a pair of
  Pachuco Suits
  pack linen clothes
  pack suit into suit
  pant length
  pant length for men

  paul smith mens suits
  Welcome to our store,We have the best service team!