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

Filename start with non-latin charaters would disappear after download #474

Closed
billzt opened this issue Mar 17, 2016 · 12 comments
Closed

Filename start with non-latin charaters would disappear after download #474

billzt opened this issue Mar 17, 2016 · 12 comments

Comments

@billzt
Copy link

billzt commented Mar 17, 2016

Bug: If filenames start with non-latin charaters, the non-latin charater part would disappear after download. ("chars_only_latin" has been set to false,)

Reproduce: Prepare four blank text files named:

blank空白.txt
blank空白blank.txt
空白blank.txt
空白.txt

All of them could be normally uploaded. After download, the filename of blank空白.txt and blank空白blank.txt is OK, However the filename of 空白blank.txt and 空白.txt is wrong:

"空白blank.txt" becomes "blank.txt"
"空白.txt" becomes ".txt"
@simogeo
Copy link
Owner

simogeo commented Mar 17, 2016

@billzt
Copy link
Author

billzt commented Mar 17, 2016

chars_only_latin has already been set to false. This bug occurs.

@simogeo
Copy link
Owner

simogeo commented Mar 17, 2016

I'll add a test to give a name if string is empty. Stay tuned

@simogeo
Copy link
Owner

simogeo commented Mar 17, 2016

should be ok, please see last commit 4dc16b0 and download last version

@simogeo simogeo closed this as completed Mar 17, 2016
@billzt
Copy link
Author

billzt commented Mar 18, 2016

OMG! In the latest version, all files start with non-latin charaters become "unsupportedCharsReplacement" after upload! It is even not so convenient as previous version!

@simogeo
Copy link
Owner

simogeo commented Mar 18, 2016

What would you like ? I opted for an explicit message but if you want I can make it easy to customize.

Do you have other proposition ? Or maybe, you want to provide a fix ?

@billzt
Copy link
Author

billzt commented Mar 19, 2016

The best option is too keep the original filename as it is. Of course now we could use a workaround: Add some symbol(for example, a underscore symbol) before the filename. Fox example, it a file called "空白.txt", then we could change it to "_空白.txt". Then it is OK now.

@psolom
Copy link
Collaborator

psolom commented Mar 19, 2016

Hey. I don't know the reason of the problem when filenames started from non-latin chars, but I have just checked this case on my fork and works fine. File "空白blank.txt" was uploaded as is. But be aware that I am still working on my fork and believe it is not stable enough so far. I invite you to test my fork if you want. Check the roadmap for details.

@psolom psolom reopened this Apr 26, 2016
@psolom
Copy link
Collaborator

psolom commented Apr 26, 2016

Hey, I have faced with the same problem with cyrillic chars on Linux (it was fine on Windows). Further investigation led me to this. The case is that basename() / pathinfo() are locale dependent functions in PHP. To solve the problem you have to set locale as:

setlocale(LC_ALL, 'en_US.UTF-8');

I was experimenting with strings which begin with Cyrillic and Chinese chars (as in the topic) and came to the conclusion that there is no difference what language is set for locale. Both en_US.UTF-8 and ru_RU.UTF-8 works fine for the strings I checked. The main thing is to add .UTF-8 part. So I believe setlocale(LC_ALL, 'en_US.UTF-8') could be used as general solution.

@simogeo As you know my fork is too different from the main repo. I want you to check this case for the current version of the filemanager to be sure this works. Would you do this?

@simogeo
Copy link
Owner

simogeo commented Apr 26, 2016

well spotted @servocoder : indeed, it works. I will PR soon waiting for a stable fork

@psolom
Copy link
Collaborator

psolom commented Apr 26, 2016

Not sure we should hardcode locale for end user. It may overrides a locale, that user set above in the code. Perhaps just to specify the case in docs. What do you think?

@simogeo
Copy link
Owner

simogeo commented Apr 26, 2016

should not be hard coded for sure, that's why I added the following comment @todo make this dynamic with config json file
will do that soon and let you know

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

No branches or pull requests

3 participants