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

bugfix(JstlLocalization): fix parsing utf8 strings #1020

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

Conversation

khajavi
Copy link

@khajavi khajavi commented Aug 27, 2015

Issue solved: caelum/mamute#190

@Turini
Copy link
Member

Turini commented Aug 27, 2015

Hi @khajavi, thanks for sending us this PR. I'm just not sure if adding it to the
framework's core would be a good ideia, since ISO-8859 are expected when
reading properties files. Let's see what another devs say about that. cc/@lucascs

anyway, you're able to fix it easily by specializing JstlLocalization in your app:

@Specializes @RequestScoped
public class CustomJstlLocalization extends JstlLocalization {

    @Override @Produces
    public ResourceBundle getBundle(Locale locale) { /* custom code */ }
}

@csokol
Copy link
Contributor

csokol commented Aug 28, 2015

Apparently is possible to load UTF-8 encoded data from properties file. I think this is useful for languages with non ocidental alphabets (arabic, russian, etc).

@khajavi is having an issue because of that in Mamute while trying to translate the website content to his language, check caelum/mamute#190.

We can add this custom component to Mamute core, but I think that other VRaptor users might have the same problem in the future. What do you think?

I'm not sure if this is the best solution yet (forcing utf-8 properties for everyone) and we should probably improve this code and add tests before merging but I think it is worth of discussing a solution in this PR.

@cpfeiffer
Copy link

Even if it's possible to read property files as UTF-8, it would cause problems with all other tools that read/write these files, e.g. IDE editors, which assume the standard latin1 encoding.

Even if the encoding is latin1, it's still possible to embed unicode characters with the unicode escape syntax.

I'm not sure how comfortable it is to create such files, though. The eclipse property file editor for example actually creates unicode escape sequences when entering non-latin1 characters in to a property file.

@lucascs
Copy link
Member

lucascs commented Sep 1, 2015

Be extremely careful with this!

properties file are ISO-8859-1 by specification, every properties tool will consider that.

Also you simply cannot change the default to UTF-8 because all projects already running VRaptor today are certainly using ISO-8859-1 for properties files.

final ClassLoader classLoader = loader;
final boolean reloadFlag = reload;
InputStream stream;
try {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use try-with-resources here.

@Turini
Copy link
Member

Turini commented Sep 1, 2015

@khajavi we left a few suggestions on your code, can you take a look on it?
after that we can 🚀 this pr

@angeliski
Copy link
Member

Hey @khajavi Can I help you to improve the code? I would like to approve that pull request. :)

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.

6 participants