Fix side effect on strings in url_for #6003

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@nbeloglazov
Contributor

nbeloglazov commented Apr 26, 2012

url_for method changes controller that passed to the method. Example:

controller = '/projects'
url_for {:controller => controller, :action => 'status'}
puts 'Side effect!' if controller == 'projects'

I suppose url_for shouldn't change any arguments it takes.
Controller is changed here

@ghost ghost assigned pixeltrix May 8, 2012

@pixeltrix

This comment has been minimized.

Show comment Hide comment
@pixeltrix

pixeltrix May 9, 2012

Owner

Merged manually in e154823.

Owner

pixeltrix commented May 9, 2012

Merged manually in e154823.

@pixeltrix pixeltrix closed this May 9, 2012

@dvandersluis

This comment has been minimized.

Show comment Hide comment
@dvandersluis

dvandersluis Nov 2, 2012

Was this fixed released anywhere? I can't find it in the source code and am experiencing the same problem.

Was this fixed released anywhere? I can't find it in the source code and am experiencing the same problem.

@carlosantoniodasilva

This comment has been minimized.

Show comment Hide comment
@carlosantoniodasilva

carlosantoniodasilva Nov 2, 2012

Owner

It seems to be in master only, so not in any stable release. Not sure it was backported to 3-2, or if it will be, @pixeltrix might be able to confirm that.

It seems to be in master only, so not in any stable release. Not sure it was backported to 3-2, or if it will be, @pixeltrix might be able to confirm that.

@pixeltrix

This comment has been minimized.

Show comment Hide comment
@pixeltrix

pixeltrix Nov 2, 2012

Owner

I refactored the original commit in beea9f5 which I never backported. I don't see any reason it can't be be, however it's too late for 3.2.9 now.

Owner

pixeltrix commented Nov 2, 2012

I refactored the original commit in beea9f5 which I never backported. I don't see any reason it can't be be, however it's too late for 3.2.9 now.

@dvandersluis

This comment has been minimized.

Show comment Hide comment
@dvandersluis

dvandersluis Nov 2, 2012

@pixeltrix I'd appreciate a backport, for whatever release it makes 😃

@pixeltrix I'd appreciate a backport, for whatever release it makes 😃

@carlosantoniodasilva

This comment has been minimized.

Show comment Hide comment
@carlosantoniodasilva

carlosantoniodasilva Nov 2, 2012

Owner

Ok, bookmarked to backport after 3.2.9 is released.

Ok, bookmarked to backport after 3.2.9 is released.

@carlosantoniodasilva

This comment has been minimized.

Show comment Hide comment
@carlosantoniodasilva

carlosantoniodasilva Nov 16, 2012

Owner

Backported 88a296d...ff5d606, thanks.

Backported 88a296d...ff5d606, thanks.

@dvandersluis

This comment has been minimized.

Show comment Hide comment
@dvandersluis

dvandersluis Nov 17, 2012

Thanks! 👍

Thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment