Repository: zaproxy/zaproxy
Issue: Use of commas within regex for contexts renders them unloadable 

## Summary
When regular expressions are created using braces, the GUI will save the user input during the current working session and behave as expected. However, if the user restarts OWASP Zap and attempts to import the context into a new session, the resulting action creates an exception. This is based upon how the rendered context file is created where braces and commas are involved.

## System Information
* OWASP ZAP version 2.11.0
* Tested on both Ubuntu and OSX

## Steps to Reproduce
1. Launch OWASP Zap.
2. Double-click on the Default Context to launch the Session Properties modal. Select "Include In Context".
3. Click the "Add..." button and enter the following regular expression: `http[s]{0,1}:\\/\\/[a-zA-Z0-9.-]{1,}(google.com)\\/{0,1}(.*)`. Click the "Add" button when done.
4. Click the "OK" button to close the Session Properties modal.
5. Right-click on the Default Context and select "Export Context..".
6. Save the context somewhere on your local drive.
7. Locate the context file on your local drive and view the contents. For the example above, here is the first few nodes of the XML content:
    ```<configuration>
        <context>
            <name>Default Context</name>
            <desc/>
            <inscope>true</inscope>
            <incregexes>http[s]{0</incregexes>
            <incregexes>1}:\\/\\/[a-zA-Z0-9.-]{1</incregexes>
            <incregexes>}(google.com)\\/{0</incregexes>
            <incregexes>1}(.*)</incregexes>
8. Exit OWASP Zap and re-open the application.
9. Click the "Import Context..." icon and select the context you just created.
10. _*NOTICE:* You now see a "Failed to import Context" error suggesting that the issue is the first node containing some portion of the regular expression (i.e. `http[s]{0`).
11. Repeat the same process as above, exept this time use the following regular expression: `http[s]?:\\/\\/[a-zA-Z0-9.-]+(google.com)\\/?(.*)`. Note that the difference is that quantifiers used have the same meaning but use different symbols (i.e. https://docs.microsoft.com/en-us/dotnet/standard/base-types/quantifiers-in-regular-expressions). The resulting XML now looks like this when viewing the contents of the new context file:
      ```<configuration>
          <context>
              <name>New Context</name>
              <desc/>
              <inscope>true</inscope>
              <incregexes>http[s]?:\\/\\/[a-zA-Z0-9.-]+(google.com)\\/?(.*)</incregexes>

## Log Output (from zap.log)
```
java.util.regex.PatternSyntaxException: Unclosed counted closure near index 9
http[s]{0
	at java.util.regex.Pattern.error(Pattern.java:2027) ~[?:?]
	at java.util.regex.Pattern.closure(Pattern.java:3304) ~[?:?]
	at java.util.regex.Pattern.sequence(Pattern.java:2213) ~[?:?]
	at java.util.regex.Pattern.expr(Pattern.java:2068) ~[?:?]
	at java.util.regex.Pattern.compile(Pattern.java:1782) ~[?:?]
	at java.util.regex.Pattern.<init>(Pattern.java:1428) ~[?:?]
	at java.util.regex.Pattern.compile(Pattern.java:1094) ~[?:?]
	at org.zaproxy.zap.model.Context.validateRegex(Context.java:364) ~[zap-2.11.0.jar:2.11.0]
	at org.zaproxy.zap.model.Context.addIncludeInContextRegex(Context.java:402) ~[zap-2.11.0.jar:2.11.0]
	at org.parosproxy.paros.model.Session.importContext(Session.java:1542) ~[zap-2.11.0.jar:2.11.0]
	at org.parosproxy.paros.control.MenuFileControl.importContext(MenuFileControl.java:594) [zap-2.11.0.jar:2.11.0]
	at org.parosproxy.paros.view.SiteMapPanel$3.actionPerformed(SiteMapPanel.java:283) [zap-2.11.0.jar:2.11.0]
	at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1967) [?:?]
	at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2308) [?:?]
	at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:405) [?:?]
	at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:262) [?:?]
	at javax.swing.plaf.basic.BasicButtonListener.mouseReleased(BasicButtonListener.java:279) [?:?]
	at java.awt.AWTEventMulticaster.mouseReleased(AWTEventMulticaster.java:297) [?:?]
	at java.awt.Component.processMouseEvent(Component.java:6635) [?:?]
	at javax.swing.JComponent.processMouseEvent(JComponent.java:3342) [?:?]
	at java.awt.Component.processEvent(Component.java:6400) [?:?]
	at java.awt.Container.processEvent(Container.java:2263) [?:?]
	at java.awt.Component.dispatchEventImpl(Component.java:5011) [?:?]
	at java.awt.Container.dispatchEventImpl(Container.java:2321) [?:?]
	at java.awt.Component.dispatchEvent(Component.java:4843) [?:?]
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4918) [?:?]
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4547) [?:?]
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4488) [?:?]
	at java.awt.Container.dispatchEventImpl(Container.java:2307) [?:?]
	at java.awt.Window.dispatchEventImpl(Window.java:2772) [?:?]
	at java.awt.Component.dispatchEvent(Component.java:4843) [?:?]
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:772) [?:?]
	at java.awt.EventQueue$4.run(EventQueue.java:721) [?:?]
	at java.awt.EventQueue$4.run(EventQueue.java:715) [?:?]
	at java.security.AccessController.doPrivileged(Native Method) ~[?:?]
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) [?:?]
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:95) [?:?]
	at java.awt.EventQueue$5.run(EventQueue.java:745) [?:?]
	at java.awt.EventQueue$5.run(EventQueue.java:743) [?:?]
	at java.security.AccessController.doPrivileged(Native Method) ~[?:?]
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) [?:?]
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:742) [?:?]
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203) [?:?]
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) [?:?]
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113) [?:?]
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109) [?:?]
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) [?:?]
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:90) [?:?]
```

## Bug Fix Suggestion
Both syntax structures should be honored. Serialization of the regular expression might help resolve characters being accidentally interpreted by the underlying code.

## Success
Regular expressions which utilize braces can now be accepted as input for the development of contexts.

Changed files:
zap/src/main/java/org/zaproxy/zap/utils/ZapXmlConfiguration.java, zap/src/test/java/org/zaproxy/zap/utils/ZapXmlConfigurationUnitTest.java

diff --git a/zap/src/main/java/org/zaproxy/zap/utils/ZapXmlConfiguration.java b/zap/src/main/java/org/zaproxy/zap/utils/ZapXmlConfiguration.java
index 17cdedb6b..0dd34945f 100644
--- a/zap/src/main/java/org/zaproxy/zap/utils/ZapXmlConfiguration.java
+++ b/zap/src/main/java/org/zaproxy/zap/utils/ZapXmlConfiguration.java
@@ -55,6 +55,7 @@ public class ZapXmlConfiguration extends XMLConfiguration {
 
         super.setEncoding("UTF-8");
         super.setDelimiterParsingDisabled(true);
+        super.setListDelimiter('\\0');
     }
 
     /**
@@ -83,6 +84,19 @@ public class ZapXmlConfiguration extends XMLConfiguration {
         load();
     }
 
+    /**
+     * Creates a new instance of {@code ZapXmlConfiguration}. The configuration is loaded from the
+     * specified {@code url}.
+     *
+     * @param url the URL
+     * @throws ConfigurationException if loading the configuration fails
+     */
+    public ZapXmlConfiguration(URL url) throws ConfigurationException {
+        this();
+        setURL(url);
+        load();
+    }
+
     /**
      * Creates a new instance of {@code ZapXmlConfiguration}. The configuration is loaded from the
      * specified {@code file}.
@@ -134,19 +148,6 @@ public class ZapXmlConfiguration extends XMLConfiguration {
         return transformer;
     }
 
-    /**
-     * Creates a new instance of {@code ZapXmlConfiguration}. The configuration is loaded from the
-     * specified {@code url}.
-     *
-     * @param url the URL
-     * @throws ConfigurationException if loading the configuration fails
-     */
-    public ZapXmlConfiguration(URL url) throws ConfigurationException {
-        this();
-        setURL(url);
-        load();
-    }
-
     /**
      * Calling this method has <strong>no</strong> effect. The character encoding used is always the
      * same, UTF-8.
diff --git a/zap/src/test/java/org/zaproxy/zap/utils/ZapXmlConfigurationUnitTest.java b/zap/src/test/java/org/zaproxy/zap/utils/ZapXmlConfigurationUnitTest.java
index e8494b698..94cd6d09a 100644
--- a/zap/src/test/java/org/zaproxy/zap/utils/ZapXmlConfigurationUnitTest.java
+++ b/zap/src/test/java/org/zaproxy/zap/utils/ZapXmlConfigurationUnitTest.java
@@ -25,8 +25,11 @@ import static org.hamcrest.Matchers.is;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.StandardCharsets;
+import java.util.List;
+import org.apache.commons.configuration.ConfigurationException;
 import org.junit.jupiter.api.Test;
 
 /** Unit test for {@link ZapXmlConfiguration}. */
@@ -43,6 +46,92 @@ class ZapXmlConfigurationUnitTest {
                     + "    </d>\\n"
                     + "</a>\\n";
 
+    @Test
+    void shouldBeInstantiatedWithDelimeterParsingDisabledAndSetNullChar() {
+        // Given
+        ZapXmlConfiguration config = new ZapXmlConfiguration();
+        // When
+        boolean isParsingDisabled = config.isDelimiterParsingDisabled();
+        char delimChar = config.getListDelimiter();
+        // Then
+        assertThat(isParsingDisabled, is(equalTo(true)));
+        assertThat(delimChar, is(equalTo('\\0')));
+    }
+
+    @Test
+    void shouldSetVariousPropertiesAsExpected()
+            throws ConfigurationException, UnsupportedEncodingException {
+        // Given
+        ZapXmlConfiguration config = new ZapXmlConfiguration();
+        String aString = "Some string 🙂.";
+        String bString = "";
+        String cString = null;
+        int aInt = 120;
+        boolean aBoolean = true;
+        // When
+        config.setProperty("aString", aString);
+        config.setProperty("bString", bString);
+        config.setProperty("cString", cString);
+        config.setProperty("aInt", aInt);
+        config.setProperty("aBoolean", aBoolean);
+        // Then
+        List<Object> aStringList = config.getList("aString");
+        assertThat(aStringList.size(), is(equalTo(1)));
+        assertThat(aStringList.get(0), is(equalTo(aString)));
+
+        List<Object> bStringList = config.getList("bString");
+        assertThat(bStringList.size(), is(equalTo(1)));
+        assertThat(bStringList.get(0), is(equalTo(bString)));
+
+        List<Object> cStringList = config.getList("cString");
+        assertThat(cStringList.size(), is(equalTo(0)));
+
+        List<Object> aIntList = config.getList("aInt");
+        int aActualInt = config.getInt("aInt");
+        assertThat(aIntList.size(), is(equalTo(1)));
+        // Int is string when handled via list
+        assertThat(aIntList.get(0), is(equalTo("120")));
+        assertThat(aActualInt, is(equalTo(120)));
+
+        List<Object> aBooleanList = config.getList("aBoolean");
+        boolean aActualBoolean = config.getBoolean("aBoolean");
+        assertThat(aBooleanList.size(), is(equalTo(1)));
+        // Boolean is string when handled via list
+        assertThat(aBooleanList.get(0), is(equalTo("true")));
+        assertThat(aActualBoolean, is(equalTo(true)));
+    }
+
+    @Test
+    void shouldSetListOfPropertiesAsExpected() {
+        // Given
+        ZapXmlConfiguration config = new ZapXmlConfiguration();
+        String aString = "Some string 🙂.{2,2}";
+        String bString = "Some other string";
+        List<String> aList = List.of(aString, bString);
+        // When
+        config.setProperty("aList", aList);
+        // Then
+        List<Object> readList = config.getList("aList");
+        assertThat(readList.size(), is(equalTo(2)));
+        assertThat(readList.get(0), is(equalTo(aString)));
+    }
+
+    @Test
+    void shouldSetListOfNonPrimitivesAsExpected() throws IOException, ConfigurationException {
+        // Given
+        ZapXmlConfiguration config = new ZapXmlConfiguration();
+        CustomObject coOne = new CustomObject("coOne");
+        String stringTwo = "foo,bar";
+        CustomObject coTwo = new CustomObject(stringTwo);
+        List<CustomObject> customObjects = List.of(coOne, coTwo);
+        // When
+        config.setProperty("customObjects", customObjects);
+        // Then
+        List<Object> readList = config.getList("customObjects");
+        assertThat(readList.size(), is(equalTo(2)));
+        assertThat(((CustomObject) readList.get(1)).getValue(), is(equalTo(stringTwo)));
+    }
+
     @Test
     void shouldSaveNewConfigurationIndented() throws Exception {
         // Given
@@ -78,4 +167,21 @@ class ZapXmlConfigurationUnitTest {
                 .toString(StandardCharsets.UTF_8.name())
                 .replaceAll(System.lineSeparator(), "\\n");
     }
+
+    private static class CustomObject {
+        private final String value;
+
+        CustomObject(String value) {
+            this.value = value;
+        }
+
+        public String getValue() {
+            return value;
+        }
+
+        @Override
+        public String toString() {
+            return value;
+        }
+    }
 }