You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Ken (JIRA)" <ji...@apache.org> on 2012/10/11 21:19:03 UTC

[jira] [Created] (CONFIGURATION-510) XMLPropertyListConfiguration parses Map members containing commas when instructed not to

Ken created CONFIGURATION-510:
---------------------------------

             Summary: XMLPropertyListConfiguration parses Map members containing commas when instructed not to
                 Key: CONFIGURATION-510
                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-510
             Project: Commons Configuration
          Issue Type: Bug
          Components: Format
    Affects Versions: 1.9
            Reporter: Ken


XMLPropertyListConfiguration does not properly handle String properties containing commas when added to the plist as part of a Map.

The following code demonstrates the problem.

public class FooBar extends Object {

   public static final void main(String[] args) throws Exception {
      XMLPropertyListConfiguration config =new XMLPropertyListConfiguration();
      config.setListDelimiter('\0');
      config.setDelimiterParsingDisabled(true);

      config.setProperty("one", "this, that");

      Map<String, String> map =new HashMap<String, String>();
      map.put("two", "this, that");
      config.addProperty("foo", map);

      StringWriter strwriter =new StringWriter();
      config.save(strwriter);
      System.out.println(strwriter.toString());
   }
}

This code produces the following plist.

<?xml version="1.0"?>
<!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
<plist version="1.0">
    <dict>
        <key>one</key>
        <string>this, that</string>

        <key>foo</key>
        <dict>
            <key>two</key>
            <array>
                <string>this</string>
                <string>that</string>
            </array>
        </dict>
    </dict>
</plist>

Note that "this, that" has been split into two properties, despite the list delimiter being set away from the comma default and delimiter parsing disabled.

The problem is in XMLPropertyListConfiguration's printValue(PrintWriter, int, Object) method.  This method create a MapConfiguration containing the (transformed) "foo" Map.

   printValue(out, indentLevel, new MapConfiguration(map));

The XMLPropertyListConfiguration's delimiter-related settings are never passed to the MapConfiguration, which retains the enabled and comma defaults.

The following patch to XMLPropertyListConfiguration.java makes Commons Configuration more correct.

413c413,416
<             printValue(out, indentLevel, new MapConfiguration(map));
---
>             MapConfiguration mapconfig =new MapConfiguration(map);
>             mapconfig.setDefaultListDelimiter(getDefaultListDelimiter());
>             mapconfig.setDelimiterParsingDisabled(isDelimiterParsingDisabled());
>             printValue(out, indentLevel, mapconfig);

With this patch applied, the program outputs the expected plist.

<?xml version="1.0"?>
<!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
<plist version="1.0">
    <dict>
        <key>one</key>
        <string>this, that</string>

        <key>foo</key>
        <dict>
            <key>two</key>
            <string>this, that</string>
        </dict>
    </dict>
</plist>

While this patch may provide correct functionality for most use cases, it is still not correct.  Major structural work needs to be done to fix the problem.

The documentation for AbstractConfiguration's setDefaultListDelimiter(char) method states, "This value will be used only when creating new configurations. Those already created will not be affected by this change".  AbstractConfiguration's documentation for setListDelimiter(char) states, "Note: this change will only be effective for new parsings. If you want it to take effect for all loaded properties use the no arg constructor and call this method before setting the source."

MapConfiguration extends AbstractConfiguration, yet does not honor the inherited contracts.  That MapConfiguration does not have a no arg constructor is fair enough, though would be somewhat limiting if it did follow the contracts.  That invocations of setDefaultListDelimiter() and setListDelimiter() effect previously added properties is simply wrong.  Unless state is maintained for each added property (which would be a messy implementation), the parsing must not be executed within the getProperty(String) method.  Instead, values must be parsed upon being added to the MapConfiguration.

The same problem exists within XMLPropertyListConfiguration.  The delegated parsing at issue in the bug report is invoked from the printValue(PrintWriter, int, Object) method, which is far too late (again, barring state-saving), to honor the contract that added properties are parsed, or not, according to the delimiter settings in place at the time each property is added.



--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CONFIGURATION-510) XMLPropertyListConfiguration parses Map members containing commas when instructed not to

Posted by "Oliver Heger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CONFIGURATION-510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13475282#comment-13475282 ] 

Oliver Heger commented on CONFIGURATION-510:
--------------------------------------------

Many thanks for this detailed report.

The problem you describe - the output generated by {{XMLPropertyListConfiguration}} -  is certainly a bug. So I think the patch should be applied to fix this. (Would it be possible for you to convert the test program to a unit test case and add it to {{TestXMLPropertyListConfiguration}}? You can attach a combined patch with the test case and the fix to this issue.)

Regarding your comments about {{MapConfiguration}}, I am not sure whether it should be allowed to parse for delimiters at a later stage or not. This configuration implementation is a bit of a special case as it exposes an already existing {{Map}} object through the {{Configuration}} interface. Thus there is no control over the entries in the map, and if list splitting is to be supported, there is no earlier point in time when this could be done. At least this behavior is documented in the Javadocs.
                
> XMLPropertyListConfiguration parses Map members containing commas when instructed not to
> ----------------------------------------------------------------------------------------
>
>                 Key: CONFIGURATION-510
>                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-510
>             Project: Commons Configuration
>          Issue Type: Bug
>          Components: Format
>    Affects Versions: 1.9
>            Reporter: Ken
>
> XMLPropertyListConfiguration does not properly handle String properties containing commas when added to the plist as part of a Map.
> The following code demonstrates the problem.
> {code:title=Sample Code}
> public class FooBar extends Object {
>    public static final void main(String[] args) throws Exception {
>       XMLPropertyListConfiguration config =new XMLPropertyListConfiguration();
>       config.setListDelimiter('\0');
>       config.setDelimiterParsingDisabled(true);
>       config.setProperty("one", "this, that");
>       Map<String, String> map =new HashMap<String, String>();
>       map.put("two", "this, that");
>       config.addProperty("foo", map);
>       StringWriter strwriter =new StringWriter();
>       config.save(strwriter);
>       System.out.println(strwriter.toString());
>    }
> }
> {code}
> This code produces the following plist.
> {code:xml}
> <?xml version="1.0"?>
> <!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
> <plist version="1.0">
>     <dict>
>         <key>one</key>
>         <string>this, that</string>
>         <key>foo</key>
>         <dict>
>             <key>two</key>
>             <array>
>                 <string>this</string>
>                 <string>that</string>
>             </array>
>         </dict>
>     </dict>
> </plist>
> {code}
> Note that "this, that" has been split into two properties, despite the list delimiter being set away from the comma default and delimiter parsing disabled.
> The problem is in XMLPropertyListConfiguration's printValue(PrintWriter, int, Object) method.  This method create a MapConfiguration containing the (transformed) "foo" Map.
> {code:title=Existing XMLPropertyListConfiguration code}
>    printValue(out, indentLevel, new MapConfiguration(map));
> {code}
> The XMLPropertyListConfiguration's delimiter-related settings are never passed to the MapConfiguration, which retains the enabled and comma defaults.
> The following patch to XMLPropertyListConfiguration.java makes Commons Configuration more correct.
> {code:title=XMLPropertyListConfiguration.java Patch}
> 413c413,416
> <             printValue(out, indentLevel, new MapConfiguration(map));
> ---
> >             MapConfiguration mapconfig =new MapConfiguration(map);
> >             mapconfig.setDefaultListDelimiter(getDefaultListDelimiter());
> >             mapconfig.setDelimiterParsingDisabled(isDelimiterParsingDisabled());
> >             printValue(out, indentLevel, mapconfig);
> {code}
> With this patch applied, the program outputs the expected plist.
> {code:xml}
> <?xml version="1.0"?>
> <!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
> <plist version="1.0">
>     <dict>
>         <key>one</key>
>         <string>this, that</string>
>         <key>foo</key>
>         <dict>
>             <key>two</key>
>             <string>this, that</string>
>         </dict>
>     </dict>
> </plist>
> {code}
> While this patch may provide correct functionality for most use cases, it is still not correct.  Major structural work needs to be done to fix the problem.
> The documentation for AbstractConfiguration's setDefaultListDelimiter(char) method states, "This value will be used only when creating new configurations. Those already created will not be affected by this change".  AbstractConfiguration's documentation for setListDelimiter(char) states, "Note: this change will only be effective for new parsings. If you want it to take effect for all loaded properties use the no arg constructor and call this method before setting the source."
> MapConfiguration extends AbstractConfiguration, yet does not honor the inherited contracts.  That MapConfiguration does not have a no arg constructor is fair enough, though would be somewhat limiting if it did follow the contracts.  That invocations of setDefaultListDelimiter() and setListDelimiter() effect previously added properties is simply wrong.  Unless state is maintained for each added property (which would be a messy implementation), the parsing must not be executed within the getProperty(String) method.  Instead, values must be parsed upon being added to the MapConfiguration.
> The same problem exists within XMLPropertyListConfiguration.  The delegated parsing at issue in the bug report is invoked from the printValue(PrintWriter, int, Object) method, which is far too late (again, barring state-saving), to honor the contract that added properties are parsed, or not, according to the delimiter settings in place at the time each property is added.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CONFIGURATION-510) XMLPropertyListConfiguration parses Map members containing commas when instructed not to

Posted by "Ken (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CONFIGURATION-510?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ken updated CONFIGURATION-510:
------------------------------

    Description: 
XMLPropertyListConfiguration does not properly handle String properties containing commas when added to the plist as part of a Map.

The following code demonstrates the problem.

{code:title=Sample Code}
public class FooBar extends Object {

   public static final void main(String[] args) throws Exception {
      XMLPropertyListConfiguration config =new XMLPropertyListConfiguration();
      config.setListDelimiter('\0');
      config.setDelimiterParsingDisabled(true);

      config.setProperty("one", "this, that");

      Map<String, String> map =new HashMap<String, String>();
      map.put("two", "this, that");
      config.addProperty("foo", map);

      StringWriter strwriter =new StringWriter();
      config.save(strwriter);
      System.out.println(strwriter.toString());
   }
}
{code}

This code produces the following plist.

{code:xml}
<?xml version="1.0"?>
<!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
<plist version="1.0">
    <dict>
        <key>one</key>
        <string>this, that</string>

        <key>foo</key>
        <dict>
            <key>two</key>
            <array>
                <string>this</string>
                <string>that</string>
            </array>
        </dict>
    </dict>
</plist>
{code}

Note that "this, that" has been split into two properties, despite the list delimiter being set away from the comma default and delimiter parsing disabled.

The problem is in XMLPropertyListConfiguration's printValue(PrintWriter, int, Object) method.  This method create a MapConfiguration containing the (transformed) "foo" Map.

{code:title=Existing XMLPropertyListConfiguration code}
   printValue(out, indentLevel, new MapConfiguration(map));
{code}

The XMLPropertyListConfiguration's delimiter-related settings are never passed to the MapConfiguration, which retains the enabled and comma defaults.

The following patch to XMLPropertyListConfiguration.java makes Commons Configuration more correct.

{code:title=XMLPropertyListConfiguration.java Patch}
413c413,416
<             printValue(out, indentLevel, new MapConfiguration(map));
---
>             MapConfiguration mapconfig =new MapConfiguration(map);
>             mapconfig.setDefaultListDelimiter(getDefaultListDelimiter());
>             mapconfig.setDelimiterParsingDisabled(isDelimiterParsingDisabled());
>             printValue(out, indentLevel, mapconfig);
{code}

With this patch applied, the program outputs the expected plist.

{code:xml}
<?xml version="1.0"?>
<!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
<plist version="1.0">
    <dict>
        <key>one</key>
        <string>this, that</string>

        <key>foo</key>
        <dict>
            <key>two</key>
            <string>this, that</string>
        </dict>
    </dict>
</plist>
{code}

While this patch may provide correct functionality for most use cases, it is still not correct.  Major structural work needs to be done to fix the problem.

The documentation for AbstractConfiguration's setDefaultListDelimiter(char) method states, "This value will be used only when creating new configurations. Those already created will not be affected by this change".  AbstractConfiguration's documentation for setListDelimiter(char) states, "Note: this change will only be effective for new parsings. If you want it to take effect for all loaded properties use the no arg constructor and call this method before setting the source."

MapConfiguration extends AbstractConfiguration, yet does not honor the inherited contracts.  That MapConfiguration does not have a no arg constructor is fair enough, though would be somewhat limiting if it did follow the contracts.  That invocations of setDefaultListDelimiter() and setListDelimiter() effect previously added properties is simply wrong.  Unless state is maintained for each added property (which would be a messy implementation), the parsing must not be executed within the getProperty(String) method.  Instead, values must be parsed upon being added to the MapConfiguration.

The same problem exists within XMLPropertyListConfiguration.  The delegated parsing at issue in the bug report is invoked from the printValue(PrintWriter, int, Object) method, which is far too late (again, barring state-saving), to honor the contract that added properties are parsed, or not, according to the delimiter settings in place at the time each property is added.



  was:
XMLPropertyListConfiguration does not properly handle String properties containing commas when added to the plist as part of a Map.

The following code demonstrates the problem.

public class FooBar extends Object {

   public static final void main(String[] args) throws Exception {
      XMLPropertyListConfiguration config =new XMLPropertyListConfiguration();
      config.setListDelimiter('\0');
      config.setDelimiterParsingDisabled(true);

      config.setProperty("one", "this, that");

      Map<String, String> map =new HashMap<String, String>();
      map.put("two", "this, that");
      config.addProperty("foo", map);

      StringWriter strwriter =new StringWriter();
      config.save(strwriter);
      System.out.println(strwriter.toString());
   }
}

This code produces the following plist.

<?xml version="1.0"?>
<!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
<plist version="1.0">
    <dict>
        <key>one</key>
        <string>this, that</string>

        <key>foo</key>
        <dict>
            <key>two</key>
            <array>
                <string>this</string>
                <string>that</string>
            </array>
        </dict>
    </dict>
</plist>

Note that "this, that" has been split into two properties, despite the list delimiter being set away from the comma default and delimiter parsing disabled.

The problem is in XMLPropertyListConfiguration's printValue(PrintWriter, int, Object) method.  This method create a MapConfiguration containing the (transformed) "foo" Map.

   printValue(out, indentLevel, new MapConfiguration(map));

The XMLPropertyListConfiguration's delimiter-related settings are never passed to the MapConfiguration, which retains the enabled and comma defaults.

The following patch to XMLPropertyListConfiguration.java makes Commons Configuration more correct.

413c413,416
<             printValue(out, indentLevel, new MapConfiguration(map));
---
>             MapConfiguration mapconfig =new MapConfiguration(map);
>             mapconfig.setDefaultListDelimiter(getDefaultListDelimiter());
>             mapconfig.setDelimiterParsingDisabled(isDelimiterParsingDisabled());
>             printValue(out, indentLevel, mapconfig);

With this patch applied, the program outputs the expected plist.

<?xml version="1.0"?>
<!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
<plist version="1.0">
    <dict>
        <key>one</key>
        <string>this, that</string>

        <key>foo</key>
        <dict>
            <key>two</key>
            <string>this, that</string>
        </dict>
    </dict>
</plist>

While this patch may provide correct functionality for most use cases, it is still not correct.  Major structural work needs to be done to fix the problem.

The documentation for AbstractConfiguration's setDefaultListDelimiter(char) method states, "This value will be used only when creating new configurations. Those already created will not be affected by this change".  AbstractConfiguration's documentation for setListDelimiter(char) states, "Note: this change will only be effective for new parsings. If you want it to take effect for all loaded properties use the no arg constructor and call this method before setting the source."

MapConfiguration extends AbstractConfiguration, yet does not honor the inherited contracts.  That MapConfiguration does not have a no arg constructor is fair enough, though would be somewhat limiting if it did follow the contracts.  That invocations of setDefaultListDelimiter() and setListDelimiter() effect previously added properties is simply wrong.  Unless state is maintained for each added property (which would be a messy implementation), the parsing must not be executed within the getProperty(String) method.  Instead, values must be parsed upon being added to the MapConfiguration.

The same problem exists within XMLPropertyListConfiguration.  The delegated parsing at issue in the bug report is invoked from the printValue(PrintWriter, int, Object) method, which is far too late (again, barring state-saving), to honor the contract that added properties are parsed, or not, according to the delimiter settings in place at the time each property is added.



    
> XMLPropertyListConfiguration parses Map members containing commas when instructed not to
> ----------------------------------------------------------------------------------------
>
>                 Key: CONFIGURATION-510
>                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-510
>             Project: Commons Configuration
>          Issue Type: Bug
>          Components: Format
>    Affects Versions: 1.9
>            Reporter: Ken
>
> XMLPropertyListConfiguration does not properly handle String properties containing commas when added to the plist as part of a Map.
> The following code demonstrates the problem.
> {code:title=Sample Code}
> public class FooBar extends Object {
>    public static final void main(String[] args) throws Exception {
>       XMLPropertyListConfiguration config =new XMLPropertyListConfiguration();
>       config.setListDelimiter('\0');
>       config.setDelimiterParsingDisabled(true);
>       config.setProperty("one", "this, that");
>       Map<String, String> map =new HashMap<String, String>();
>       map.put("two", "this, that");
>       config.addProperty("foo", map);
>       StringWriter strwriter =new StringWriter();
>       config.save(strwriter);
>       System.out.println(strwriter.toString());
>    }
> }
> {code}
> This code produces the following plist.
> {code:xml}
> <?xml version="1.0"?>
> <!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
> <plist version="1.0">
>     <dict>
>         <key>one</key>
>         <string>this, that</string>
>         <key>foo</key>
>         <dict>
>             <key>two</key>
>             <array>
>                 <string>this</string>
>                 <string>that</string>
>             </array>
>         </dict>
>     </dict>
> </plist>
> {code}
> Note that "this, that" has been split into two properties, despite the list delimiter being set away from the comma default and delimiter parsing disabled.
> The problem is in XMLPropertyListConfiguration's printValue(PrintWriter, int, Object) method.  This method create a MapConfiguration containing the (transformed) "foo" Map.
> {code:title=Existing XMLPropertyListConfiguration code}
>    printValue(out, indentLevel, new MapConfiguration(map));
> {code}
> The XMLPropertyListConfiguration's delimiter-related settings are never passed to the MapConfiguration, which retains the enabled and comma defaults.
> The following patch to XMLPropertyListConfiguration.java makes Commons Configuration more correct.
> {code:title=XMLPropertyListConfiguration.java Patch}
> 413c413,416
> <             printValue(out, indentLevel, new MapConfiguration(map));
> ---
> >             MapConfiguration mapconfig =new MapConfiguration(map);
> >             mapconfig.setDefaultListDelimiter(getDefaultListDelimiter());
> >             mapconfig.setDelimiterParsingDisabled(isDelimiterParsingDisabled());
> >             printValue(out, indentLevel, mapconfig);
> {code}
> With this patch applied, the program outputs the expected plist.
> {code:xml}
> <?xml version="1.0"?>
> <!DOCTYPE plist SYSTEM "file://localhost/System/Library/DTDs/PropertyList.dtd">
> <plist version="1.0">
>     <dict>
>         <key>one</key>
>         <string>this, that</string>
>         <key>foo</key>
>         <dict>
>             <key>two</key>
>             <string>this, that</string>
>         </dict>
>     </dict>
> </plist>
> {code}
> While this patch may provide correct functionality for most use cases, it is still not correct.  Major structural work needs to be done to fix the problem.
> The documentation for AbstractConfiguration's setDefaultListDelimiter(char) method states, "This value will be used only when creating new configurations. Those already created will not be affected by this change".  AbstractConfiguration's documentation for setListDelimiter(char) states, "Note: this change will only be effective for new parsings. If you want it to take effect for all loaded properties use the no arg constructor and call this method before setting the source."
> MapConfiguration extends AbstractConfiguration, yet does not honor the inherited contracts.  That MapConfiguration does not have a no arg constructor is fair enough, though would be somewhat limiting if it did follow the contracts.  That invocations of setDefaultListDelimiter() and setListDelimiter() effect previously added properties is simply wrong.  Unless state is maintained for each added property (which would be a messy implementation), the parsing must not be executed within the getProperty(String) method.  Instead, values must be parsed upon being added to the MapConfiguration.
> The same problem exists within XMLPropertyListConfiguration.  The delegated parsing at issue in the bug report is invoked from the printValue(PrintWriter, int, Object) method, which is far too late (again, barring state-saving), to honor the contract that added properties are parsed, or not, according to the delimiter settings in place at the time each property is added.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira