Action dispatch http params #12370

Closed
wants to merge 2 commits into
from

Projects

None yet

6 participants

@idoo
idoo commented Sep 26, 2013

Add a rescue when convert nested Hash to HashWithIndifferentAccess

@senny senny commented on the diff Sep 26, 2013
actionpack/lib/action_dispatch/http/parameters.rb
def normalize_encode_params(params)
case params
- when String
- params.force_encoding(Encoding::UTF_8).encode!
- when Hash
+ when String
+ encode_string(params)
+ when Hash
@senny
senny Sep 26, 2013 Member

please don't change indent on the when statements.

@senny
Member
senny commented Sep 26, 2013

can you add a test-case to illustrate the use-case for the behavior?

@robin850 robin850 commented on the diff Sep 26, 2013
actionpack/CHANGELOG.md
@@ -139,4 +139,8 @@
*Piotr Sarnacki*
+* Add a rescue when convert nested Hash to HashWithIndifferentAccess
@robin850
robin850 Sep 26, 2013 Member

Actually, entries should go at the very top of the changelog file. Could you update it please ?

@egilburg egilburg commented on the diff Sep 26, 2013
actionpack/lib/action_dispatch/http/parameters.rb
if params.has_key?(:tempfile)
UploadedFile.new(params)
else
params.each_with_object({}) do |(key, val), new_hash|
- new_key = key.is_a?(String) ? key.dup.force_encoding(Encoding::UTF_8).encode! : key
+ if key.is_a?(String)
@egilburg
egilburg Sep 26, 2013 Contributor

Don't really think this needs to be a multiline if statement, especially given that you made it shorter by extracting logic into the encode_string method.

@idoo
idoo commented Sep 27, 2013

I have a problem with the test. For rescue i need some characters like this "𐀀 ø€ ø€€€¯ ñårƒ"
But i can't store them in testfile. Any ideas?
As sample i can get this string from File.open, but don't think, that this good idea.
As other sample i can generate this string like this
10.chr.force_encoding("ibm855")
but i need symbols, that don't have a number (like IBM855 'unmappable'
http://www.fileformat.info/info/charset/IBM855/encode.htm%C3%B8%E2%82%AC%E2%82%AC%E2%82%AC
Do you have a better idea of ​​how to do this test?

@senny
Member
senny commented Sep 27, 2013

why can't you store them in the test-file?

@idoo
idoo commented Sep 27, 2013

i get a SyntaxError

string = "ñårƒ"
            ^
@senny
Member
senny commented Sep 27, 2013

@idoo did you set the file encoding?

Here is an example of a test-case with special characters: https://github.com/rails/rails/blob/master/activerecord/test/cases/base_test.rb#L602

and the declaration:
https://github.com/rails/rails/blob/master/activerecord/test/cases/base_test.rb#L1

@jamo
Contributor
jamo commented Feb 11, 2014

I checked this, and #13999 will most likely fix this. @tenderlove should have fixed the cause in Rack.

@tenderlove tenderlove closed this in #13999 Jul 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment