hhuijser/liferay-portal — pull request #1507 by bensonlau (2013-09-12 23:02:37+00:00)
Title: Lps 38723 

1. Where I still have concerns is that instead of passing an escaped value of groupName (which I understand can be double escaped, which is a problem), we now always pass an unescaped value of groupName to the method notify...

a) what about just not escaping it at line 84 and leaving it as string groupName = group.getDescriptiveName?

I'm thinking that this would be the correct fix for this since https://github.com/bensonlau/liferay-portal-ee/blob/master/portal-impl/src/com/liferay/portlet/messageboards/service/impl/MBMessageLocalServiceImpl.java#L1864 uses similar logic in using group.getdescriptivename to get the 'site name'. group.getDescriptiveName in that class is not escaped and thus the email sent to users when a message board post is made does not show the name of the Site with special characters displaying incorrectly.

2) Doesn't that change the behavior of the subscriptionSender?
a) doesnt change the behavior fo the subscriptions just the values that are displayed in the email template used by subscriptionSender. Currently the problem is that the Site Name of "Tom & Jerry" would display the value as "Tom (ampersand entity code) Jerry" due to the way we are escaping in flagsrequestmessengerlistener.

3) Is it possible to fix the issue in the jsp? When we call HtmlUtil.escape(group.getDescriptiveName() in the jsp, can't we only do that when the group is not a user group, so we catch the double escape already over there?

a)
For the issue in the edit_entry.jsp, HtmlUtil.escape(group.getDescriptiveName isnt actually getting called. As you can see in https://github.com/bensonlau/liferay-portal-ee/blob/master/portal-web/docroot/html/portlet/flags/edit_entry.jsp#L117, we are escaping the className, contentTitle and contentURL variables. className, contentTitle, and contentURL are then used in flagsrequestmessagelistener.java. As you can see in https://github.com/bensonlau/liferay-portal-ee/blob/master/portal-impl/src/com/liferay/portlet/flags/messaging/FlagsRequestMessageListener.java#L156-L157, those values are being escaped in the jsp and thus when it is used in the java file are displaying the incorrect values.

I guess the issue to be fixed in the jsp would be to NOT escape the value of contentTitle. Also I dont htink it is necessary to escape the className either but please let me know if for some reason we should have that field escaped. Is it necessary to leave the escape method on the contentUrl? I know for URL's we want to keep from any xss issues so we typically leave that escaped.
--------------------------------------------------