ChiliProject is not maintained anymore. Please be advised that there will be no more updates.

We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.

Entering large numbers for 'Estimated Time' fails with 'Invalid big Decimal Value' (Bug #345)

Added by Gregor Schmidt at 2011-04-20 04:25 pm. Updated at 2011-05-27 09:40 pm.

Status:Closed Start date:2011-04-20
Priority:Normal Due date:
Assignee:Eric Davis % Done:

0%
Category:Issue tracking
Target version:2.0.0
Remote issue URL: Affected version:master

Description

While trying to set estimated time of an issue to 5000000 hours, I am getting a Server Error. The back trace looks like the following:

RuntimeError (Invalid big Decimal Value):
  config/initializers/bigdecimal-segfault-fix.rb:31:in `BigDecimal'
  /Users/schmidt/.rvm/rubies/ree-1.8.7-2011.03/lib/ruby/1.8/bigdecimal/util.rb:20:in `to_d'
  app/models/issue.rb:741:in `recalculate_attributes_for_without_remaining_hours'
  app/models/issue.rb:720:in `update_parent_attributes'
  app/models/issue.rb:556:in `save_issue_with_child_records'
  app/models/issue.rb:538:in `save_issue_with_child_records'
  app/controllers/issues_controller.rb:170:in `update'

I am using Ruby 1.8.7-p334, ChiliProject master, mysql2 with MySQL 5.1. It also fails in ree-1.8.7-2011.03.

Analysis:

Back in Summer 09, Eric Davis added a workaround for a segfault bug in Ruby to the ChiliProject sources (source:config/initializers/bigdecimal-segfault-fix.rb)

That bug was present in

  • 1.8.6-p368 and all prior versions
  • 1.8.7-p160 and all prior versions

and is fixed in all ruby version released after June 2009. More information may be found at https://github.com/NZKoz/bigdecimal-segfault-fix.

Anyway, this work around has some undesired side effects, i.e. you cannot enter really large numbers in some form fields.

I am proposing to remove that workaround, since we should be able to assume, that nowadays, nearly 2 years later, everybody was able to update their ruby interpreter. I think, there have been multiple other bugs and security issues, that where reported since then and ChiliProject does not provide work-arounds for them either. Ergo, this work around does not increase ChiliProject's security in a significant way.

If removing the workaround seems to be unfeasible, we could at least guard the patch with checks, so that only those versions are patched, that are affected by the bug.

Associated revisions

Revision 9e1b6c14
Added by Eric Davis at 2011-05-27 11:37 pm

[#345] Remove BigDecimal patch since Rails 2.3.11 includes a mitigation

History

#2
Updated by Eric Davis at 2011-04-20 06:51 pm

Since most people won't be logging 5,000,000 hours in one time entry (570 years of work), I think we should just remove the patch from 2.0.0 since we would be dropping/phasing out 1.8.6 then.

#3
Updated by Gregor Schmidt at 2011-04-20 08:03 pm

Thanks for having a look at this issue.

Although I agree with your opinion, I do not follow your conclusion:

The security issue, that is fixed by this work around, is present in older versions of 1.8.6 and 1.8.7. Furthermore it is fixed in current versions of 1.8.6 and 1.8.7. Therefore, phasing out 1.8.6 is not helping in this context. This would also mean, that removing the patch does not need a major release but could be done in 1.3.0.

#4
Updated by Eric Davis at 2011-04-20 11:59 pm

Some clarification as to why I said to just remove it in 2.0.0

  • From what I remember, this bug was worked around in a newer rails version which is in unstable already, so 2.0 could have it removed. (I'll need to check the security report).
  • Removing the patch from 1.x could re-expose the security hole for users on older versions of Ruby. (You can't assume everyone is on the latest Ruby. I had a client on 1.8.5 until only a few months ago.)
  • If the scoping patch works for all users on older Ruby versions, then we might be able to add it for 1.x but this feels like such an edge case, I'm not sure there is enough time to include it (i.e. more important bugs are still pending). If someone has the time to review it with older versions of Ruby before mid-next week, we might be able to include it in 1.3.0. Otherwise it will need to wait until 1.4.0, which might not be released if 2.0 is ready before then :)

#5
Updated by Gregor Schmidt at 2011-04-21 05:20 am

Thanks for your detailed explanations.

I don't think it is worth it to first add the guards, just to completely remove the patch in the next release.

Shall I open a pull request targeting the unstable branch which just removes the file?

  • Target version changed from 1.3.0 to 2.0.0

#6
Updated by Gregor Schmidt at 2011-04-21 05:24 am

Eric Davis wrote:

  • From what I remember, this bug was worked around in a newer rails version which is in unstable already, so 2.0 could have it removed. (I'll need to check the security report).

According to this post on the Rails Security mailing list these changes where introduced in Rails 2.3.3, i.e. they are already in master.

#7
Updated by Eric Davis at 2011-05-27 09:40 pm

I've removed the patch. Looking at the history, it was added while on Rails 2.2.2. Since we are on 2.3.11 now Rails should handle it for us.

Thanks for reviewing and researching this. We could probably do a sweep through the code and remove old patches and compatibility hacks now (e.g. the cruft at the bottom of config/routes.rb...)

  • Assignee set to Eric Davis
  • (deleted custom field) set to master
  • Status changed from Ready for review to Closed

Also available in: Atom PDF