Encode the uploaded file's name in utf8 - Closes #869 #1689

Merged
merged 2 commits into from Jun 14, 2011

Conversation

Projects
None yet
3 participants
Contributor

dmathieu commented Jun 14, 2011

See related issue : #869

Member

josevalim commented Jun 14, 2011

Thanks for the pull request, but there are a few changes still required here:

  1. AS provides a method on the string called encoding_aware? that returns true if the encoding system is available. please use it instead of respond_to.

  2. We cannot hardcode it to utf8. We should probably check default_external or whatever the current code in AD is using for encoding the rest of the request. force_encoding in general is not an option.

/cc @wycats

Contributor

dmathieu commented Jun 14, 2011

Oh ok, I saw the encoding_aware method but as it didn't respond in irb in 1.8, I thought it was only 1.9.
I didn't think it would be implemented by AS. I'll use it.

I'll also look into not hardcoding it.

Member

josevalim commented Jun 14, 2011

Tks <3 <3

Contributor

dmathieu commented Jun 14, 2011

I have pushed again in my fork with your suggestions.

Member

josevalim commented Jun 14, 2011

Much cleaner by moving it to its own method. :)

Let's wait for @wycats feedback. Although Rails is using default_external for loading templates, it seems we are hardcoding the request to be UTF-8 (which kinda makes sense). So actually your first implementation would be more correct according to this commit from Yehuda:

25215d7

Sorry about that. :(

@josevalim josevalim added a commit that referenced this pull request Jun 14, 2011

@josevalim josevalim Merge pull request #1689 from dmathieu/utf8-filename
Encode the uploaded file's name in utf8 - Closes #869
deff597

@josevalim josevalim merged commit deff597 into rails:master Jun 14, 2011

Shoud not there be assert_equal instead of plain assert?

Contributor

dmathieu replied Jun 15, 2011

Surely it should, my bad. Do you want to make a pull request ?

Contributor

daeltar replied Jun 15, 2011

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