Add unit test and fix for APSTUD-4068 Theme export not escaping certa…

…in characters, creating an invalid theme file

    - escape '<', '>' and '&' before writing out XML
    - use precompiled patterns, map, StringBuffer, appendReplacement for sanitizeHTML
    - Add newMap, addToMap methods on CollectionsUtil and add unit tests for them.

.gitignore

@@ -19,6 +19,6 @@ native/**/Debug/
 native/**/Release/
 native/**/UpgradeLog*.XML
 native/**/_UpgradeReport_Files/
-tests/com.aptana.scripting.tests/application-bundles/**/cache.yml
-tests/com.aptana.scripting.tests/user-bundles/**/cache.yml
-tests/com.aptana.scripting.tests/project-bundles/**/cache.yml
+tests/com.aptana.scripting.tests/application-bundles/**/cache*.yml
+tests/com.aptana.scripting.tests/user-bundles/**/cache*.yml
+tests/com.aptana.scripting.tests/project-bundles/**/cache*.yml

plugins/com.aptana.core/src/com/aptana/core/util/CollectionsUtil.java

@@ -11,10 +11,12 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import com.aptana.core.IFilter;
@@ -354,6 +356,62 @@ public static final <T> Set<T> newSet(T... items)
 		return result;
 	}
 
+	/**
+	 * Convert a list of items into a Set. An empty set is returned if items is null
+	 * 
+	 * @param <T>
+	 *            Any type of object
+	 * @param items
+	 *            A variable length list of items of type T
+	 * @return Returns a new HashSet<T> or an empty set
+	 */
+	public static final <T> Map<T, T> newMap(T... items)
+	{
+		Map<T, T> result;
+
+		if (items != null)
+		{
+			result = new HashMap<T, T>();
+			addToMap(result, items);
+		}
+		else
+		{
+			result = Collections.emptyMap();
+		}
+
+		return result;
+	}
+
+	/**
+	 * Add a varargs list of items into a map. It is expected that items be in "key, value, key2, value2, etc.."
+	 * ordering. If the map or items are null then no action is performed. Note that the destination map has no
+	 * requirements other than it must be a Map of the source item's type. This allows the destination to be used, for
+	 * example, as an accumulator.<br>
+	 * <br>
+	 * Note that this method is not thread safe. Users of this method will need to maintain type safety against the map.
+	 * 
+	 * @param map
+	 *            A map to which items will be added
+	 * @param items
+	 *            A list of items to add
+	 */
+	public static final <T, U extends T> Map<T, T> addToMap(Map<T, T> map, U... items)
+	{
+		if (map != null && items != null)
+		{
+			if (items.length % 2 != 0)
+			{
+				throw new IllegalArgumentException("Length of list of items must be multiple of 2"); //$NON-NLS-1$
+			}
+			for (int i = 0; i < items.length; i += 2)
+			{
+				map.put(items[i], items[i + 1]);
+			}
+		}
+
+		return map;
+	}
+
 	/**
 	 * Given a list of elements of type <T>, remove the duplicates from the list in place
 	 * 

plugins/com.aptana.core/src/com/aptana/core/util/StringUtil.java

@@ -15,6 +15,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.StringTokenizer;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.eclipse.core.runtime.IStatus;
@@ -35,6 +36,13 @@ public class StringUtil
 	 */
 	public static final Pattern LINE_SPLITTER = Pattern.compile("\r?\n|\r"); //$NON-NLS-1$
 
+	/**
+	 * Map to sanitize html/xml to entities.
+	 */
+	private static final Map<String, String> SANITIZE_MAP = CollectionsUtil.newMap(
+			"&", "&amp;", "<", "&lt;", ">", "&gt;"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$
+	private static final Pattern SANITIZE_PATTERN = Pattern.compile("[&<>]"); //$NON-NLS-1$
+
 	private StringUtil()
 	{
 	}
@@ -286,15 +294,22 @@ public static String md5(String lowerCase)
 		return null;
 	}
 
-/**
+	/**
 	 * Sanitizes raw HTML to escape '&', '<' and '>' so that it is suitable for embedding into HTML.
 	 * 
 	 * @param raw
 	 * @return
 	 */
 	public static String sanitizeHTML(String raw)
 	{
-		return raw.replaceAll("&", "&amp;").replaceAll("<", "&lt;"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
+		StringBuffer sb = new StringBuffer();
+		Matcher m = SANITIZE_PATTERN.matcher(raw);
+		while (m.find())
+		{
+			m.appendReplacement(sb, SANITIZE_MAP.get(m.group()));
+		}
+		m.appendTail(sb);
+		return sb.toString();
 	}
 
 	/**

plugins/com.aptana.theme/src/com/aptana/theme/ThemeExporter.java

@@ -17,6 +17,7 @@
 import org.eclipse.swt.SWT;
 
 import com.aptana.core.logging.IdeLog;
+import com.aptana.core.util.StringUtil;
 
 /**
  * @author cwilliams
@@ -42,7 +43,7 @@ public void export(File themeFile, Theme theme)
 		buffer.append("<plist version=\"1.0\">\n");
 		buffer.append("<dict>\n");
 		buffer.append("  <key>name</key>\n");
-		buffer.append("  <string>").append(theme.getName()).append("</string>\n");
+		buffer.append("  <string>").append(escape(theme.getName())).append("</string>\n");
 		buffer.append("  <key>uuid</key>\n");
 		buffer.append("  <string>").append(UUID.nameUUIDFromBytes(theme.getName().getBytes())).append("</string>\n");
 		buffer.append("  <key>settings</key>\n");
@@ -71,9 +72,9 @@ public void export(File themeFile, Theme theme)
 		{
 			buffer.append("    <dict>\n");
 			buffer.append("      <key>name</key>\n");
-			buffer.append("      <string>").append(rule.getName()).append("</string>\n");
+			buffer.append("      <string>").append(escape(rule.getName())).append("</string>\n");
 			buffer.append("      <key>scope</key>\n");
-			buffer.append("      <string>").append(rule.getScopeSelector().toString()).append("</string>\n");
+			buffer.append("      <string>").append(escape(rule.getScopeSelector().toString())).append("</string>\n");
 			buffer.append("      <key>settings</key>\n");
 			buffer.append("      <dict>\n");
 
@@ -147,4 +148,14 @@ public void export(File themeFile, Theme theme)
 		}
 	}
 
+	/**
+	 * Escapes special characters (i.e. '<' and '>' which need to be encoded for XML).
+	 * @param string
+	 * @return
+	 */
+	private String escape(String raw)
+	{
+		return StringUtil.sanitizeHTML(raw);
+	}
+
 }

tests/com.aptana.core.tests/src/com/aptana/core/util/CollectionsUtilTest.java

@@ -11,6 +11,7 @@
 import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import junit.framework.TestCase;
@@ -474,4 +475,104 @@ public void testMapNullMapper2()
 		CollectionsUtil.map(source, destination, null);
 		assertTrue(destination.isEmpty());
 	}
+
+	public void testNewMap()
+	{
+		Map<String, String> map = CollectionsUtil.newMap("item1", "item2");
+
+		assertNotNull(map);
+		assertEquals("The map should have only one item", 1, map.size());
+		assertTrue("'item1' should exist in the map", map.containsKey("item1"));
+		assertTrue("'item2' should exist in the map", map.containsValue("item2"));
+		assertEquals("item2", map.get("item1"));
+	}
+
+	public void testNewMapNullItems()
+	{
+		String[] items = null;
+		Map<String, String> map = CollectionsUtil.newMap(items);
+
+		assertNotNull(map);
+		assertTrue("Map should be empty", map.isEmpty());
+	}
+
+	public void testAddToMapSubclass()
+	{
+		Number doubleOne = 1.0;
+		Number intOne = 1;
+		Float floatOne = 1.0f;
+		Float floatTwo = 2.0f;
+
+		// generate initial map
+		Map<Number, Number> map = CollectionsUtil.newMap(doubleOne, intOne);
+
+		// add sub-type of Number
+		CollectionsUtil.addToMap(map, floatOne, floatTwo);
+
+		assertEquals("The map should have only two items", 2, map.size());
+		assertTrue("Map should contain double 1.0", map.containsKey(doubleOne));
+		assertTrue("Map should contain integer 1", map.containsValue(intOne));
+		assertEquals(intOne, map.get(doubleOne));
+		assertTrue("Map should contain float 1.0f", map.containsKey(floatOne));
+		assertTrue("Map should contain float 2.0f", map.containsValue(floatTwo));
+		assertEquals(floatTwo, map.get(floatOne));
+	}
+
+	public void testAddToMap()
+	{
+		Map<String, String> map = CollectionsUtil.newMap("a", "b");
+		assertNotNull(map);
+
+		CollectionsUtil.addToMap(map, "c", "d");
+		assertEquals("The map should have only two items", 2, map.size());
+		assertTrue("'a' should exist in the map", map.containsKey("a"));
+		assertEquals("b", map.get("a"));
+		assertTrue("'b' should exist in the map", map.containsValue("b"));
+		assertTrue("'c' should exist in the map", map.containsKey("c"));
+		assertEquals("d", map.get("c"));
+		assertTrue("'d' should exist in the map", map.containsValue("d"));
+	}
+
+	public void testAddToMapNullItems()
+	{
+		Map<String, String> map = CollectionsUtil.newMap("a", "b", "c", "d");
+		assertNotNull(map);
+
+		String[] items = null;
+		CollectionsUtil.addToMap(map, items);
+		assertEquals("The map should have only two items", 2, map.size());
+		assertTrue("'a' should exist in the map", map.containsKey("a"));
+		assertEquals("b", map.get("a"));
+		assertTrue("'b' should exist in the map", map.containsValue("b"));
+		assertTrue("'c' should exist in the map", map.containsKey("c"));
+		assertEquals("d", map.get("c"));
+		assertTrue("'d' should exist in the map", map.containsValue("d"));
+	}
+
+	public void testAddToMapNullMap()
+	{
+		try
+		{
+			CollectionsUtil.addToMap(null, "a", "b", "c");
+		}
+		catch (Throwable t)
+		{
+			fail(t.getMessage());
+		}
+	}
+
+	public void testAddUnevenItemsToMap()
+	{
+		Map<String, String> map = CollectionsUtil.newMap("a", "b");
+		assertNotNull(map);
+		try
+		{
+			CollectionsUtil.addToMap(map, "c", "d", "e");
+			fail("Didn't throw IllegalArgumentException when we tried to add uneven number of elements to a map.");
+		}
+		catch (IllegalArgumentException t)
+		{
+			assertTrue(true);
+		}
+	}
 }

tests/com.aptana.core.tests/src/com/aptana/core/util/StringUtilTest.java

@@ -25,7 +25,6 @@ public void testMd5()
 		assertEquals("a4c4da98a897d052baf31d4e5c0cce55", StringUtil.md5("cwilliams@aptana.com"));
 
 		assertNull(StringUtil.md5(null));
-
 	}
 
 	public void testSanitizeHTML()
@@ -35,7 +34,8 @@ public void testSanitizeHTML()
 
 	public void testSanitizeHTML2()
 	{
-		assertEquals("&lt;html>Heckle &amp; Jeckle&lt;/html>", StringUtil.sanitizeHTML("<html>Heckle & Jeckle</html>"));
+		assertEquals("&lt;html&gt;Heckle &amp; Jeckle&lt;/html&gt;",
+				StringUtil.sanitizeHTML("<html>Heckle & Jeckle</html>"));
 	}
 
 	public void testReplaceAll()