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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@
*/
package br.com.caelum.vraptor.core;

import static com.google.common.base.Objects.firstNonNull;

import java.util.Locale;
import java.util.MissingResourceException;
import java.util.ResourceBundle;
import br.com.caelum.vraptor.util.EmptyBundle;
import br.com.caelum.vraptor.util.SafeResourceBundle;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.enterprise.context.RequestScoped;
import javax.enterprise.inject.Produces;
Expand All @@ -29,12 +28,11 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.jsp.jstl.core.Config;
import javax.servlet.jsp.jstl.fmt.LocalizationContext;
import java.util.Locale;
import java.util.MissingResourceException;
import java.util.ResourceBundle;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import br.com.caelum.vraptor.util.EmptyBundle;
import br.com.caelum.vraptor.util.SafeResourceBundle;
import static com.google.common.base.Objects.firstNonNull;

/**
* The default implementation of bundle provider uses JSTL's api to access user information on the bundle to be used.
Expand Down Expand Up @@ -75,7 +73,7 @@ private ResourceBundle extractUnsafeBundle(Object bundle, Locale locale) {
String baseName = firstNonNull((String) bundle, DEFAULT_BUNDLE_NAME);

try {
return ResourceBundle.getBundle(baseName, locale);
return ResourceBundle.getBundle(baseName, locale, new UTF8Control());
Copy link
Member

Choose a reason for hiding this comment

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

instead of changing the expected/default behaviour,
what do you think about creating an CDI alternative for utf8 bundle producer?
so users can enable it via beans.xml configuration

http://docs.oracle.com/javaee/6/tutorial/doc/gjsdf.html

Copy link
Author

Choose a reason for hiding this comment

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

Turini,
Sorry for the late reply.
I agree with @csokol @cpfeiffer @lucascs that changing the ISO-8859-1 default to utf8 is not good solution.
I need to learn about CDI to be able give expert opinion.

} catch (MissingResourceException e) {
logger.warn("couldn't find message bundle, creating an empty one", e);
return new EmptyBundle();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
https://gist.github.com/Hasacz89/d93955ec91afc73a06e3
*/

package br.com.caelum.vraptor.core;

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.URL;
import java.net.URLConnection;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.Locale;
import java.util.PropertyResourceBundle;
import java.util.ResourceBundle;

public class UTF8Control extends ResourceBundle.Control {
Copy link
Member

Choose a reason for hiding this comment

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

please, add @Vetoed (since it's not a CDI managed bean)

Copy link
Author

Choose a reason for hiding this comment

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

Hi Turini,
I'm new in Java EE, so I don't know about CDI, I need to learn about it.
Thanks for review.

public ResourceBundle newBundle
(String baseName, Locale locale, String format, ClassLoader loader, boolean reload)
throws IllegalAccessException, InstantiationException, IOException {
// The below is a copy of the default implementation.
String bundleName = toBundleName(baseName, locale);
ResourceBundle bundle = null;
switch (format) {
case "java.class":
try {
@SuppressWarnings("unchecked")
Class<? extends ResourceBundle> bundleClass
= (Class<? extends ResourceBundle>) loader.loadClass(bundleName);

// If the class isn't a ResourceBundle subclass, throw a
// ClassCastException.
if (ResourceBundle.class.isAssignableFrom(bundleClass)) {
bundle = bundleClass.newInstance();
} else {
throw new ClassCastException(bundleClass.getName()
+ " cannot be cast to ResourceBundle");
}
} catch (ClassNotFoundException e) {
}

break;
case "java.properties":
final String resourceName = toResourceName(bundleName, "properties");
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.

stream = AccessController.doPrivileged(
new PrivilegedExceptionAction<InputStream>() {
public InputStream run() throws IOException {
InputStream is = null;
if (reloadFlag) {
URL url = classLoader.getResource(resourceName);
if (url != null) {
URLConnection connection = url.openConnection();
if (connection != null) {
// Disable caches to get fresh data for
// reloading.
connection.setUseCaches(false);
is = connection.getInputStream();
}
}
} else {
is = classLoader.getResourceAsStream(resourceName);
}
return is;
}
});
} catch (PrivilegedActionException e) {
throw (IOException) e.getException();
}
if (stream != null) {
try {
bundle = new PropertyResourceBundle(new InputStreamReader(stream, "UTF-8"));
} finally {
stream.close();
}
}

break;
default:
throw new IllegalArgumentException("unknown format: " + format);
}
return bundle;
}
}