Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing encoding of non-string arrays #11159

Closed
wants to merge 1 commit into from
Closed

Fixing encoding of non-string arrays #11159

wants to merge 1 commit into from

Conversation

alno
Copy link
Contributor

@alno alno commented Jun 28, 2013

When array of non-string values assigned from array of strings (controller params, for example) it blows up with NoMethodError: undefined method `gsub' in quote_and_escape

It's because of quote_and_escape is called for values, which are strings before cast, while after casting it may become something other than string.

This patch fixes it, by calling quote_and_escape for values, which are strings after cast, not before.

Correct casting string values (from parameters, for example) to target types inside arrays
@carlosantoniodasilva
Copy link
Member

/cc @rafaelfranca @tenderlove

@kirs
Copy link
Member

kirs commented Jul 16, 2013

@steveklabnik ping :)

@steveklabnik
Copy link
Member

I don't have enough experience with AR to know what's up in this code, sorry. It seems fine, but I'm not comfortable merging.

@anayden
Copy link
Contributor

anayden commented Jul 17, 2013

👍

@arthurnn
Copy link
Member

arthurnn commented Apr 9, 2014

Tried to apply only the tests on latest master, and seems that the issue still exist..
cc @senny for review in here too.

@senny
Copy link
Member

senny commented Apr 10, 2014

@arthurnn I'll take a look.

@senny senny added this to the 4.2.0 milestone Apr 10, 2014
@senny
Copy link
Member

senny commented May 26, 2014

@arthurnn can you still reproduce the error? I just applied the tests on master and they passed.

@arthurnn
Copy link
Member

@senny seems like this has been fixed in master, I assume it was fixed as part of the cast refactoring that it is happening right now.
Will close this for now, @alno please fell free to ping us if this is still a issue on current master.
thanks for the PR

@arthurnn arthurnn closed this May 29, 2014
@senny
Copy link
Member

senny commented May 29, 2014

@arthurnn just to clarify, the current cast refactoring should not have had any impacts on this issue. We kept the existing type casting mechanics and rearranged them. However I did a lot of work on the PG specific types lately. It might be related to that.

Anyway, thanks for closing 💛

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

Successfully merging this pull request may close these issues.

None yet

7 participants