Skip to content

Commit

Permalink
XX-10518 Encoding of Display Names containg German language Special C…
Browse files Browse the repository at this point in the history
…haracters is not correct for Polycom phones

- changed XML filter to escape only the 5 xml entities and leave the other special characters as they are, since XML is encoded as UTF-8 anyway
- changed a test to take into account this change
- fixed various tests overlooked by previous commits
  • Loading branch information
Cristi Ciuc Starasciuc committed Nov 22, 2012
1 parent a88a4b9 commit ec646cd
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 26 deletions.
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@


public final class PhoneTestDriver { public final class PhoneTestDriver {
public static final String SIPFOUNDRY_ORG = "sipfoundry.org"; public static final String SIPFOUNDRY_ORG = "sipfoundry.org";
public static final String USERNAME = "juser";
public static final String SHARED_USER = "sharedUser";
public static final String SHARED = "Shared";
public static final String PASS = "1234";


private final IMocksControl m_phoneContextControl; private final IMocksControl m_phoneContextControl;


Expand Down Expand Up @@ -139,17 +143,42 @@ public static PhoneTestDriver supplyTestData(Phone phone, boolean phonebookManag
List<User> users = new ArrayList<User>(); List<User> users = new ArrayList<User>();


User firstUser = new User(); User firstUser = new User();
firstUser.setUserName("juser"); firstUser.setUserName(USERNAME);
firstUser.setFirstName("Joe"); firstUser.setFirstName("Joe");
firstUser.setLastName("User"); firstUser.setLastName("User");
firstUser.setSipPassword("1234"); firstUser.setSipPassword(PASS);
firstUser.setIsShared(false); firstUser.setIsShared(false);
users.add(firstUser); users.add(firstUser);


if (includeSharedLine) { if (includeSharedLine) {
User secondUser = new User(); User secondUser = new User();
secondUser.setUserName("sharedUser"); secondUser.setUserName(SHARED_USER);
secondUser.setFirstName("Shared"); secondUser.setFirstName(SHARED);
secondUser.setLastName(firstUser.getLastName());
secondUser.setSipPassword(firstUser.getSipPassword());
secondUser.setIsShared(true);
users.add(secondUser);
}

return new PhoneTestDriver(phone, users, phonebookManagementEnabled, speedDial, sendCheckSyncToMac);
}

public static PhoneTestDriver supplyTestDataWithSpecialChars(Phone phone, boolean phonebookManagementEnabled,
boolean speedDial, boolean includeSharedLine, boolean sendCheckSyncToMac) {
List<User> users = new ArrayList<User>();

User firstUser = new User();
firstUser.setUserName(USERNAME);
firstUser.setFirstName("Joe<>?'\"");
firstUser.setLastName("Schröder");
firstUser.setSipPassword(PASS);
firstUser.setIsShared(false);
users.add(firstUser);

if (includeSharedLine) {
User secondUser = new User();
secondUser.setUserName(SHARED_USER);
secondUser.setFirstName(SHARED);
secondUser.setLastName(firstUser.getLastName()); secondUser.setLastName(firstUser.getLastName());
secondUser.setSipPassword(firstUser.getSipPassword()); secondUser.setSipPassword(firstUser.getSipPassword());
secondUser.setIsShared(true); secondUser.setIsShared(true);
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
*/ */
package org.sipfoundry.sipxconfig.setting; package org.sipfoundry.sipxconfig.setting;


import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang.StringUtils;


/**
* This filter will escape the 5 XML entities; It will leave other characters as they are, since Polycom xmls are
* UTF encoded. (see XX-10518)
*/
public class XmlEscapeValueFilter implements SettingValueFilter { public class XmlEscapeValueFilter implements SettingValueFilter {
public static final XmlEscapeValueFilter FILTER = new XmlEscapeValueFilter(); public static final XmlEscapeValueFilter FILTER = new XmlEscapeValueFilter();


Expand All @@ -20,7 +24,11 @@ public SettingValue filter(SettingValue sv) {
if (value == null) { if (value == null) {
return sv; return sv;
} }
String escapedXml = StringEscapeUtils.escapeXml(value); String escapedXml = StringUtils.replace(value, "&", "&amp;");
escapedXml = StringUtils.replace(escapedXml, "<", "&lt;");
escapedXml = StringUtils.replace(escapedXml, ">", "&gt;");
escapedXml = StringUtils.replace(escapedXml, "'", "&apos;");
escapedXml = StringUtils.replace(escapedXml, "\"", "&quot;");
if (value.equals(escapedXml)) { if (value.equals(escapedXml)) {
return sv; return sv;
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import org.sipfoundry.sipxconfig.commserver.LocationsManager; import org.sipfoundry.sipxconfig.commserver.LocationsManager;
import org.sipfoundry.sipxconfig.device.Device; import org.sipfoundry.sipxconfig.device.Device;
import org.sipfoundry.sipxconfig.device.DeviceTimeZone; import org.sipfoundry.sipxconfig.device.DeviceTimeZone;
import org.sipfoundry.sipxconfig.device.FileSystemProfileLocation;
import org.sipfoundry.sipxconfig.device.TimeZoneManager; import org.sipfoundry.sipxconfig.device.TimeZoneManager;
import org.sipfoundry.sipxconfig.device.VelocityProfileGenerator; import org.sipfoundry.sipxconfig.device.VelocityProfileGenerator;
import org.sipfoundry.sipxconfig.domain.Domain; import org.sipfoundry.sipxconfig.domain.Domain;
Expand Down Expand Up @@ -230,6 +231,15 @@ public static MemoryProfileLocation setVelocityProfileGenerator(Device device, S
return location; return location;
} }


public static FileSystemProfileLocation setFsVelocityProfileGenerator(Device device, String etcDir) {
FileSystemProfileLocation location = new FileSystemProfileLocation();
VelocityProfileGenerator profileGenerator = new VelocityProfileGenerator();
profileGenerator.setVelocityEngine(getVelocityEngine(etcDir));
profileGenerator.setTemplateRoot(etcDir);
device.setProfileGenerator(profileGenerator);
return location;
}

public static String getTestDirectory() { public static String getTestDirectory() {
return TestHelper.getTestOutputDirectory(); return TestHelper.getTestOutputDirectory();
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@


import junit.framework.TestCase; import junit.framework.TestCase;


import org.apache.commons.lang.StringUtils;
import org.dom4j.Document; import org.dom4j.Document;
import org.dom4j.DocumentException; import org.dom4j.DocumentException;
import org.dom4j.Element; import org.dom4j.Element;
Expand Down Expand Up @@ -114,7 +115,7 @@ private void assertCodecConfigurationForModel(CodecGroupType codecGroup, String
if (-1 != i) { if (-1 != i) {
option = option.substring(0, i); option = option.substring(0, i);
} }
major_supported_codecs.add(option); major_supported_codecs.add(StringUtils.remove(option,"_"));
} }


// Loop though the audioProfiles for the model. There should be one for major supported codec type. // Loop though the audioProfiles for the model. There should be one for major supported codec type.
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@


import org.dom4j.DocumentException; import org.dom4j.DocumentException;
import org.sipfoundry.sipxconfig.common.User; import org.sipfoundry.sipxconfig.common.User;
import org.sipfoundry.sipxconfig.device.DeviceVersion;
import org.sipfoundry.sipxconfig.phone.PhoneTestDriver; import org.sipfoundry.sipxconfig.phone.PhoneTestDriver;
import org.sipfoundry.sipxconfig.setting.Setting; import org.sipfoundry.sipxconfig.setting.Setting;
import org.sipfoundry.sipxconfig.setting.type.MultiEnumSetting; import org.sipfoundry.sipxconfig.setting.type.MultiEnumSetting;
Expand All @@ -39,14 +40,21 @@ public class CodecGroupsTest extends TestCase {
* *
* @see http://wiki.sipfoundry.org/display/xecsuser/Polycom#Polycom-Codecgroup * @see http://wiki.sipfoundry.org/display/xecsuser/Polycom#Polycom-Codecgroup
*/ */
public void testAllCodecGroups() throws DocumentException { public void test20CodecGroups() throws DocumentException {


for (CodecGroupType codec_group : CodecGroupType.values() ) { for (CodecGroupType codec_group : CodecGroupType.values() ) {
assertCodecGroup(codec_group); if (codec_group == CodecGroupType.VVX_500) {
continue;
}
assertCodecGroup(codec_group, PolycomModel.VER_3_2_X);
} }
} }


private void assertCodecGroup(CodecGroupType codecGroup) throws DocumentException { public void test40CodecGroups() throws DocumentException {
assertCodecGroup(CodecGroupType.VVX_500, PolycomModel.VER_4_0_X);
}

private void assertCodecGroup(CodecGroupType codecGroup, DeviceVersion version) throws DocumentException {


// Initialize a phone with the codec group under test. // Initialize a phone with the codec group under test.
PolycomModel model = new PolycomModel(); PolycomModel model = new PolycomModel();
Expand All @@ -55,7 +63,7 @@ private void assertCodecGroup(CodecGroupType codecGroup) throws DocumentExceptio
model.setSupportedFeatures(features); model.setSupportedFeatures(features);
PolycomPhone phone = new PolycomPhone(); PolycomPhone phone = new PolycomPhone();
phone.setModel(model); phone.setModel(model);
phone.setDeviceVersion(PolycomModel.VER_4_0_X); phone.setDeviceVersion(version);
PhoneTestDriver.supplyTestData(phone, new LinkedList<User>()); PhoneTestDriver.supplyTestData(phone, new LinkedList<User>());


// The adaptor setting for the multi-enum setting. // The adaptor setting for the multi-enum setting.
Expand Down Expand Up @@ -187,7 +195,10 @@ private CodecGroupType(String name) {
VVX_1500_SET.add("Lin16.48ksps"); VVX_1500_SET.add("Lin16.48ksps");
CODECGROUP_OPTION_MAP.put(CodecGroupType.VVX_1500, VVX_1500_SET); CODECGROUP_OPTION_MAP.put(CodecGroupType.VVX_1500, VVX_1500_SET);


HashSet<String> VVX_500_SET = new HashSet<String>(OTHERS_SET); HashSet<String> VVX_500_SET = new HashSet<String>();
VVX_500_SET.add("G711_Mu");
VVX_500_SET.add("G711_A");
VVX_500_SET.add("G729_AB");
VVX_500_SET.add("G722"); VVX_500_SET.add("G722");
CODECGROUP_OPTION_MAP.put(CodecGroupType.VVX_500, VVX_500_SET); CODECGROUP_OPTION_MAP.put(CodecGroupType.VVX_500, VVX_500_SET);


Expand Down Expand Up @@ -262,7 +273,7 @@ private CodecGroupType(String name) {
CODECGROUP_SELECTED_MAP.put(CodecGroupType.VVX_1500, VVX_1500_LIST); CODECGROUP_SELECTED_MAP.put(CodecGroupType.VVX_1500, VVX_1500_LIST);


ArrayList<String> VVX_500_LIST = new ArrayList<String>(); ArrayList<String> VVX_500_LIST = new ArrayList<String>();
VVX_500_LIST.add("G711Mu"); VVX_500_LIST.add("G711_Mu");
CODECGROUP_SELECTED_MAP.put(CodecGroupType.VVX_500, VVX_500_LIST); CODECGROUP_SELECTED_MAP.put(CodecGroupType.VVX_500, VVX_500_LIST);
} }


Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public void testGenerateSyslogProfile() throws Exception {
public void testGenerateNetworkProfileDisableOverwrite() throws Exception { public void testGenerateNetworkProfileDisableOverwrite() throws Exception {
phone.setSettingValue("network/device.net/overwrite", "0"); phone.setSettingValue("network/device.net/overwrite", "0");
phone.beforeProfileGeneration(); phone.beforeProfileGeneration();
phone.beforeProfileGeneration();
ProfileContext cfg = new DeviceConfiguration(phone); ProfileContext cfg = new DeviceConfiguration(phone);


m_pg.generate(m_location, cfg, null, "profile"); m_pg.generate(m_location, cfg, null, "profile");
Expand All @@ -74,7 +75,6 @@ public void testGenerateNetworkProfileDisableOverwrite() throws Exception {


public void testGenerateNetworkProfileEnableOverwrite() throws Exception { public void testGenerateNetworkProfileEnableOverwrite() throws Exception {
phone.setSettingValue("network/device.net/overwrite", "1"); phone.setSettingValue("network/device.net/overwrite", "1");
phone.beforeProfileGeneration();
ProfileContext cfg = new DeviceConfiguration(phone); ProfileContext cfg = new DeviceConfiguration(phone);


m_pg.generate(m_location, cfg, null, "profile"); m_pg.generate(m_location, cfg, null, "profile");
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
import static org.easymock.classextension.EasyMock.expectLastCall; import static org.easymock.classextension.EasyMock.expectLastCall;
import static org.easymock.classextension.EasyMock.replay; import static org.easymock.classextension.EasyMock.replay;


import java.io.File;
import java.io.FileReader;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;


import org.apache.commons.io.FileUtils;
import org.dom4j.Attribute; import org.dom4j.Attribute;
import org.dom4j.Document; import org.dom4j.Document;
import org.dom4j.dom.DOMDocumentFactory; import org.dom4j.dom.DOMDocumentFactory;
Expand All @@ -26,6 +29,7 @@
import org.easymock.classextension.IMocksControl; import org.easymock.classextension.IMocksControl;
import org.sipfoundry.sipxconfig.common.SpecialUser.SpecialUserType; import org.sipfoundry.sipxconfig.common.SpecialUser.SpecialUserType;
import org.sipfoundry.sipxconfig.common.User; import org.sipfoundry.sipxconfig.common.User;
import org.sipfoundry.sipxconfig.device.FileSystemProfileLocation;
import org.sipfoundry.sipxconfig.device.ProfileGenerator; import org.sipfoundry.sipxconfig.device.ProfileGenerator;
import org.sipfoundry.sipxconfig.moh.MohAddressFactory; import org.sipfoundry.sipxconfig.moh.MohAddressFactory;
import org.sipfoundry.sipxconfig.permission.PermissionManagerImpl; import org.sipfoundry.sipxconfig.permission.PermissionManagerImpl;
Expand Down Expand Up @@ -58,11 +62,14 @@ protected void setUp() throws Exception {
* Test 2.x profile generation. * Test 2.x profile generation.
*/ */
public void testGenerateProfileVersion20() throws Exception { public void testGenerateProfileVersion20() throws Exception {
FileSystemProfileLocation location = TestHelper.setFsVelocityProfileGenerator(phone, TestHelper.getEtcDir());;
location.setParentDir(TestHelper.getTestOutputDirectory());

PolycomModel model = new PolycomModel(); PolycomModel model = new PolycomModel();
model.setMaxLineCount(6); model.setMaxLineCount(6);
phone.setModel(model); phone.setModel(model);


m_testDriver = PhoneTestDriver.supplyTestData(phone, true, false, true, true); m_testDriver = PhoneTestDriver.supplyTestDataWithSpecialChars(phone, true, false, true, true);
m_testDriver.getPrimaryLine().setSettingValue("reg/label", "Joe & Joe"); m_testDriver.getPrimaryLine().setSettingValue("reg/label", "Joe & Joe");


// XCF-3581: No longer automatically generating phone emergency dial routing. These // XCF-3581: No longer automatically generating phone emergency dial routing. These
Expand All @@ -76,15 +83,18 @@ public void testGenerateProfileVersion20() throws Exception {
phone.beforeProfileGeneration(); phone.beforeProfileGeneration();
PhoneConfiguration cfg = new PhoneConfiguration(phone); PhoneConfiguration cfg = new PhoneConfiguration(phone);


m_pg.generate(m_location, cfg, null, "profile"); m_pg.generate(location, cfg, null, "profile.XXX");


System.out.println("*** BEGIN actual profile content. ***"); System.out.println("*** BEGIN actual profile content. ***");
dumpXml(m_location.getReader(), System.out); dumpXml(new FileReader(TestHelper.getTestOutputDirectory()+"/profile.XXX"), System.out);
System.out.println("*** END actual profile content. ***"); System.out.println("*** END actual profile content. ***");
System.out.println("*** BEGIN expected profile content. ***"); System.out.println("*** BEGIN expected profile content. ***");
dumpXml(getClass().getResourceAsStream("expected-phone.cfg.xml"), System.out); dumpXml(getClass().getResourceAsStream("expected-phone.cfg.xml"), System.out);
System.out.println("*** END expected profile content. ***"); System.out.println("*** END expected profile content. ***");
assertPolycomXmlEquals(getClass().getResourceAsStream("expected-phone.cfg.xml"), m_location.getReader());
//Use string comparison here since we want to capture special chars like German umlaut, etc;
//using xml comparison via assertXmlEquals will reconvert the characters encoded
assertEquals(FileUtils.readFileToString(new File(getClass().getResource("expected-phone.cfg.xml").getFile())), FileUtils.readFileToString(new File(TestHelper.getTestOutputDirectory()+"/profile.XXX")));
} }


/** /**
Expand Down Expand Up @@ -135,7 +145,6 @@ public void testGenerateSpecialUserRegistrationWhenNoConfiguredLines() throws Ex


// This displaces the PhoneContext set by supplyTestData() above. // This displaces the PhoneContext set by supplyTestData() above.
phone.setPhoneContext(phoneContext); phone.setPhoneContext(phoneContext);

phone.beforeProfileGeneration(); phone.beforeProfileGeneration();
PhoneConfiguration cfg = new PhoneConfiguration(phone); PhoneConfiguration cfg = new PhoneConfiguration(phone);
m_pg.generate(m_location, cfg, null, "profile"); m_pg.generate(m_location, cfg, null, "profile");
Expand Down Expand Up @@ -238,7 +247,6 @@ public void testGenerateProfileVersion20WithoutVoicemailPermission() throws Exce
line.setSettingValue("line-dialplan/digitmap/routing.1/address", "emergency-gateway.example.org"); line.setSettingValue("line-dialplan/digitmap/routing.1/address", "emergency-gateway.example.org");
line.setSettingValue("line-dialplan/digitmap/routing.1/port", "5440"); line.setSettingValue("line-dialplan/digitmap/routing.1/port", "5440");
line.setSettingValue("line-dialplan/digitmap/routing.1/emergency.1.value", "911,9911"); line.setSettingValue("line-dialplan/digitmap/routing.1/emergency.1.value", "911,9911");

phone.beforeProfileGeneration(); phone.beforeProfileGeneration();
PhoneConfiguration cfg = new PhoneConfiguration(phone); PhoneConfiguration cfg = new PhoneConfiguration(phone);


Expand Down Expand Up @@ -269,12 +277,17 @@ public void testGenerateSipxPhoneWithExternalLine() throws Exception {
Line externalLine = phone.createLine(); Line externalLine = phone.createLine();
phone.addLine(externalLine); phone.addLine(externalLine);
externalLine.setLineInfo(li); externalLine.setLineInfo(li);

phone.beforeProfileGeneration(); phone.beforeProfileGeneration();
PhoneConfiguration cfg = new PhoneConfiguration(phone); PhoneConfiguration cfg = new PhoneConfiguration(phone);


m_pg.generate(m_location, cfg, null, "profile"); m_pg.generate(m_location, cfg, null, "profile");


assertPolycomXmlEquals(getClass().getResourceAsStream("expected-external-line-sipx-phone.cfg.xml"), m_location.getReader()); assertPolycomXmlEquals(getClass().getResourceAsStream("expected-external-line-sipx-phone.cfg.xml"), m_location.getReader());
} }

@Override
protected void tearDown() throws Exception {
new File(TestHelper.getTestOutputDirectory()+"/profile.XXX").delete();
super.tearDown();
}
} }
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ public void testGenerateProfile20() throws Exception {
// file are used, some of which are non-blank. The value below means G711Mu is // file are used, some of which are non-blank. The value below means G711Mu is
// de-selected. // de-selected.
phone.setSettingValue("voice/codecPref/OTHERS", "G711A|G729AB"); phone.setSettingValue("voice/codecPref/OTHERS", "G711A|G729AB");

phone.beforeProfileGeneration(); phone.beforeProfileGeneration();
ProfileContext cfg = new SipConfiguration(phone); ProfileContext cfg = new SipConfiguration(phone);


Expand Down
Original file line number Original file line Diff line number Diff line change
@@ -1,10 +1,17 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?> <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!-- <!--
XML comments are ignored by assertXMLEqual().
--> "Override" Per-phone Configuration File
Generated by sipXecs: http://www.sipfoundry.org/
Instructions for integrating new configuration file content into sipXconfig:
http://wiki.sipfoundry.org/display/xecsdev/Adding+Polycom+SoundPoint+IP+New+Configuration+File+Content+into+sipXconfig
-->
<phone1> <phone1>
<reg <reg
reg.1.displayName="Joe User" reg.1.displayName="Joe&lt;&gt;?&apos;&quot; Schröder"
reg.1.address="juser" reg.1.address="juser"
reg.1.label="Joe &amp; Joe" reg.1.label="Joe &amp; Joe"
reg.1.type="private" reg.1.type="private"
Expand Down Expand Up @@ -47,7 +54,7 @@
reg.1.acd-login-logout="0" reg.1.acd-login-logout="0"
reg.1.acd-agent-available="0" reg.1.acd-agent-available="0"
reg.1.proxyRequire="" reg.1.proxyRequire=""
reg.2.displayName="Shared User" reg.2.displayName="Shared Schröder"
reg.2.address="sharedUser" reg.2.address="sharedUser"
reg.2.label="" reg.2.label=""
reg.2.type="shared" reg.2.type="shared"
Expand Down

0 comments on commit ec646cd

Please sign in to comment.