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

Ticket #10272 (closed defect: fixed)

Opened 1 year ago

Last modified 11 months ago

[PATCH] redirect_to nil crashes the Ruby runtime

Reported by: jacobat Assigned to: core
Priority: normal Milestone:
Component: ActionPack Version: edge
Severity: normal Keywords: patch tiny verified
Cc: farleyknight

Description

Doing a redirect_to nil makes my Ruby 1.8.6 end up with a "illegal instruction" and dump core. It seems the problem is some sort of infinite recursion in the redirect_to code. The problem can be easily reproduced with a "redirect_to nil" in a controller.

I experienced the problem when trying to redirect_to a URL given from a form. If the client somehow failed to supply the URL Ruby crashed.

I'm not sure what the correct course of action is in this case. Maybe the redirect_to code could simply throw an exception in case the argument is nil. Anything would be better than having Ruby crash.

Attachments

redirect_to_nil_raised_its_own_error.diff (2.0 kB) - added by farleyknight on 11/30/07 23:10:34.
Simple patch which raises RedirectToNil instead of SystemStackError
redirect_to_nil_raises_its_own_exception.diff (1.6 kB) - added by farleyknight on 12/24/07 14:19:45.
Simple patch which raises RedirectToNil instead of SystemStackError (update for Edge Rails)
redirect_to_nil_raises_action_controller_error.diff (1.3 kB) - added by farleyknight on 01/12/08 01:02:43.
Changed error to the generic ActionControllerError

Change History

11/30/07 23:10:34 changed by farleyknight

  • attachment redirect_to_nil_raised_its_own_error.diff added.

Simple patch which raises RedirectToNil instead of SystemStackError

12/01/07 00:13:41 changed by farleyknight

  • keywords set to patch tiny.
  • version changed from edge to 1.2.3.
  • milestone deleted.

12/06/07 05:54:07 changed by farleyknight

  • owner changed from core to farleyknight.

12/24/07 14:10:50 changed by fcheung

Seems reasonable enough to me, but the patch wouldn't apply cleanly for me on the latest edge

12/24/07 14:19:45 changed by farleyknight

  • attachment redirect_to_nil_raises_its_own_exception.diff added.

Simple patch which raises RedirectToNil instead of SystemStackError (update for Edge Rails)

12/24/07 14:25:57 changed by fcheung

+1 seems reasonable enough to me.

12/24/07 14:41:08 changed by dbussink

+1 Crashing is never good

12/25/07 17:33:52 changed by dubek

+1 patch redirect_to_nil_raises_its_own_exception.diff applies cleanly to edge.

12/25/07 17:34:20 changed by dubek

  • keywords changed from patch tiny to patch tiny verified.

12/28/07 07:47:20 changed by farleyknight

  • cc set to farleyknight.
  • owner changed from farleyknight to core.

12/28/07 08:36:00 changed by chuyeow

  • summary changed from redirect_to nil crashes the Ruby runtime to [PATCH] redirect_to nil crashes the Ruby runtime.

I think your patch applies to edge Rails only now (instead of 1.2.3). If so, please change the Version. Thanks!

12/28/07 08:37:39 changed by farleyknight

  • version changed from 1.2.3 to edge.

01/03/08 15:52:21 changed by david

  • keywords changed from patch tiny verified to patch tiny.
  • status changed from new to closed.
  • resolution set to incomplete.

It's good to catch this, but we don't need a whole exception class dedicated to it. Just use the ActionControllerError class and set the message. It's not like you're going to want to explicitly catch this exception anywhere (if you were, you'd know you were making a mistake). Re-verify and open when fixed. Thanks for working on this.

01/12/08 01:02:43 changed by farleyknight

  • attachment redirect_to_nil_raises_action_controller_error.diff added.

Changed error to the generic ActionControllerError

01/12/08 01:04:08 changed by farleyknight

  • keywords changed from patch tiny to patch tiny verified.
  • status changed from closed to reopened.
  • resolution deleted.

Busy lately, finally fixed this. Let me know if there are any issues..

01/12/08 03:09:42 changed by nzkoz

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

(In [8633]) don't misbehave when redirecting to nil. Closes #10272 [farleyknight]

01/12/08 03:10:43 changed by nzkoz

(In [8634]) Merge the fix for redirecting to nil. References #10272 [farleyknight]