Issue with greek characters when updating from 1.10.2 to 1.11.3

charis · September 6, 2015, 1:36pm

Hello,

I’ve just updated from 1.10.2 to 1.11.3 using an existing utf-8 database that contained patients with name and other values in greek. Now, when a search is performed in some places in the legacy UI, the greek characters are displayed correctly while in other places (such as the person’s name and address) they are displayed with symbols such as σαβ etc In the previous version of openmrs there no such issue.

Any help would be greatly appreciated. thanks

raff · September 7, 2015, 12:16pm

Could you paste some screenshots of pages where things are displayed incorrectly and some screenshots that are displayed fine? I need some pointers on where to look.

burke · September 9, 2015, 3:47am

@charis, if you share screenshots and are working with real patient data, please make sure to use fake patients for the screenshots or to blur any identifying information. Identifiable patient informations (names, identifiers, addresses, specific dates, etc.) should be treated as confidential information and never be emailed or loaded into a website.

charis · September 13, 2015, 8:58am

here are some screenshots with fake patient data to show you were the issue is:

I can see the problem in names and addresses in the demographics tab of the Patient dashboard.
The name does not appear properly in the Patient header and inside the Edit form
I don’t see any problem in patient attributes

I hope this helps

charis · October 2, 2015, 2:24am

Hi Raff, I’ve pasted some screenshots a while ago. Is there anth else you might need ?

thanks

raff · October 2, 2015, 8:19am

@charis, hi, sorry I missed your post. I’ll look into that today and get back to you.

charis · October 2, 2015, 2:00pm

Np, thanks a lot . By mistake I had not replied to your post anyway.

raff · October 5, 2015, 11:57am

Just wanted to update you that I reproduced the bug and I’m trying to find a fix.

raff · October 5, 2015, 2:49pm

I’m stuck. The issue has been introduced in:

by replacing ${…} with <c:out value="${...}" />. C:out escapes xml characters, but for some reason it breaks the encoding of UTF-8 characters too. I tried setting <%@page pageEncoding="UTF-8" %>, but with no effect.

@dkayiwa, @wyclif, any ideas why it happens?

lluismf · October 5, 2015, 3:25pm

Even if the greek chars are escaped to HTML, the browser should interpret them.

I tried this in a JSP:

<c:out value='αβγ'/>    
αβγ
&alpha;&beta;&gamma;

And the result is

I believe the problem is that given and family name are stored in the DB already escaped. It’s a case of double escaping.

dkayiwa · October 6, 2015, 9:37am

I was also able to reproduce this.

After looking at the source code for spring bind, it already does the escaping by default because of our defaultHtmlEscape=true in web.xml. So removing the c:out looks harmless. I confirmed this by having a name with <script>alert("Yo")</script> which did not execute this JavaScript. Therefore, i have removed all c:out that are within spring:bind for nameLayout.jsp as per this commit:

On a side note, all this would not have happened if spring:bind's BindStatus.java used the overload of HtmlUtils.htmlEscape(String, String) that takes a second parameter of the character set (in this case UTF-8) used by the page context.

github.com

spring-projects/spring-framework/blob/master/spring-webmvc/src/main/java/org/springframework/web/servlet/support/BindStatus.java#L158


			// No BindingResult available as request attribute:
			// Probably forwarded directly to a form view.
			// Let's do the best we can: extract a plain target if appropriate.
			Object target = requestContext.getModelObject(beanName);
			if (target == null) {
				throw new IllegalStateException("Neither BindingResult nor plain target object for bean name '" +
						beanName + "' available as request attribute");
			}
			if (this.expression != null && !"*".equals(this.expression) && !this.expression.endsWith("*")) {
				BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(target);
				this.value = bw.getPropertyValue(this.expression);
				this.valueType = bw.getPropertyType(this.expression);
				this.actualValue = this.value;
			}
			this.errorCodes = new String[0];
			this.errorMessages = new String[0];
		}

		if (htmlEscape && this.value instanceof String) {
			this.value = HtmlUtils.htmlEscape((String) this.value);
		}

raff · October 6, 2015, 9:46am

@dkayiwa, so the issue was that both spring:bind and c:out escaped HTML (double escaped as @lluismf suggested). I don’t quite follow your second paragraph about the htmlEscape method.

We must backport the fix to 1.12.x and 1.11.x. Have you created an issue in JIRA, because I can’t find any reference to it? It feels like a serious bug for implementations so we should prioritize doing a quick maintenance release of 1.11.x with the fix.

dkayiwa · October 6, 2015, 10:16am

I have created the ticket at:

Regarding htmlEscape, when I tried to debug what spring:bind actually does, I noticed that it goes through BindStatus and executes the line:

github.com

spring-projects/spring-framework/blob/master/spring-webmvc/src/main/java/org/springframework/web/servlet/support/BindStatus.java#L158


			// No BindingResult available as request attribute:
			// Probably forwarded directly to a form view.
			// Let's do the best we can: extract a plain target if appropriate.
			Object target = requestContext.getModelObject(beanName);
			if (target == null) {
				throw new IllegalStateException("Neither BindingResult nor plain target object for bean name '" +
						beanName + "' available as request attribute");
			}
			if (this.expression != null && !"*".equals(this.expression) && !this.expression.endsWith("*")) {
				BeanWrapper bw = PropertyAccessorFactory.forBeanPropertyAccess(target);
				this.value = bw.getPropertyValue(this.expression);
				this.valueType = bw.getPropertyType(this.expression);
				this.actualValue = this.value;
			}
			this.errorCodes = new String[0];
			this.errorMessages = new String[0];
		}

		if (htmlEscape && this.value instanceof String) {
			this.value = HtmlUtils.htmlEscape((String) this.value);
		}

When this line is changed to:

this.value = HtmlUtils.htmlEscape((String) this.value, "UTF-8");

we do not get this problem.

The problem comes as a result of its using the one argument method below:

public static String htmlEscape(String input) {
        return htmlEscape(input, WebUtils.DEFAULT_CHARACTER_ENCODING);
    }
where WebUtils.DEFAULT_CHARACTER_ENCODING =  "ISO-8859-1"

charis · October 26, 2015, 3:15pm

Hello Raff,

do you have any idea when the fix will be applied ? Of course, I don’t expect an exact answer but a rough estimation based on how the developer team works… Is it a matter of weeks or months ?

thanks in advance Charis