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

accept utf8 filenames #1158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

almontasser
Copy link

No description provided.

@almontasser
Copy link
Author

almontasser commented Nov 19, 2022

This PR should fix #1015 #1104

@dumbasPL
Copy link

maybe It would be nice to be able to specify what encoding to use instead. Changing the default encoding will be a breaking change. What about people that don't want/can't use utf8?

@LinusU
Copy link
Member

LinusU commented Feb 1, 2023

Sorry for the late response on this.

It feels like this is a breaking change? In that case I don't see how we could get it into the 1.x release, but maybe we can do the same change in the 2.x branch? (#399)

@dumbasPL
Copy link

dumbasPL commented Feb 1, 2023

It feels like this is a breaking change?

yes, because currently, people are using the Buffer.from(filename, 'latin1').toString('utf8'); trick.
If we parse an utf8 string as latin1 it will spit out garbage.

That's why I suggested only adding an option to change the encoding as a config option but leaving the default one as is to not break existing setups.

but maybe we can do the same change in the 2.x branch?

sounds good as well

@Doc999tor
Copy link

@LinusU I created a different PR that doesn't break backward compatibility - it extends the multer's API with a default defParamCharset: 'latin1' value
Also, improved the translations
#1210

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

Successfully merging this pull request may close these issues.

4 participants