jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.83k stars 1.12k forks source link

exporting a Map to XML causes a DOMException #2068

Closed stephengold closed 1 year ago

stephengold commented 1 year ago

The jme3-plugins sub-project provides an XMLExporter class to export savables to XML files.

However, if the Savable includes a non-empty Map, invoking the save() method causes CoreDocumentImpl.createAttribute() to throw a DOMException.

Test code:

        Map<String, String> map = new HashMap<>();
        map.put("key", "value");
        rootNode.setUserData("map", map);

        String xmlPath = "TestExport.xml";
        File xmlFile = new File(xmlPath);
        JmeExporter exporter = XMLExporter.getInstance();
        try {
            exporter.save(rootNode, xmlFile);
        } catch (IOException exception) {
            logger.log(Level.SEVERE, exception.getMessage(), exception);
        }

Typical result:

Aug 29, 2023 5:24:53 PM com.jme3.app.LegacyApplication handleError
SEVERE: Uncaught exception thrown in Thread[jME3 Main,5,main]
org.w3c.dom.DOMException: INVALID_CHARACTER_ERR: An invalid or illegal XML character is specified.
    at java.xml/com.sun.org.apache.xerces.internal.dom.CoreDocumentImpl.createAttribute(CoreDocumentImpl.java:573)
    at java.xml/com.sun.org.apache.xerces.internal.dom.ElementImpl.setAttribute(ElementImpl.java:503)
    at com.jme3.export.xml.DOMOutputCapsule.write(DOMOutputCapsule.java:165)
    at com.jme3.scene.UserData.writeList(UserData.java:258)
    at com.jme3.scene.UserData.write(UserData.java:172)
    at com.jme3.export.xml.DOMOutputCapsule.write(DOMOutputCapsule.java:555)
    at com.jme3.export.xml.DOMOutputCapsule.writeStringSavableMap(DOMOutputCapsule.java:728)
    at com.jme3.scene.Spatial.write(Spatial.java:1626)
    at com.jme3.scene.Node.write(Node.java:757)
    at com.jme3.export.xml.DOMOutputCapsule.write(DOMOutputCapsule.java:555)
    at com.jme3.export.xml.XMLExporter.save(XMLExporter.java:70)
    at com.jme3.export.xml.XMLExporter.save(XMLExporter.java:88)
    at com.jme3.export.JmeExporter.save(JmeExporter.java:62)

Note that the BinaryExporter class seems to handle maps just fine.

stephengold commented 1 year ago

added TestIssue2068 to master-branch jme3-examples at b5623697acf2597f7d69cad6ca48f8e11899dcc8

JosiahGoeman commented 1 year ago

Some additional info: It seems to happen because UserData.writeList() writes the size of the list under a key starting with the listName argument. https://github.com/jMonkeyEngine/jmonkeyengine/blob/230f1575bf9935b47cb8ecdcd8ddc7379c56e75b/jme3-core/src/main/java/com/jme3/scene/UserData.java#L258

UserData.write() calls UserData.writeList() with numbers for the listName when writing lists, arrays, and maps (all affected by this bug.) XML attribute names (or any names I think...) cannot begin with a number.

codex128 commented 1 year ago

This problem could be fixed by substituting "0" and "1" for "A" and "B" in the read and write methods, except that would break backwards compatibility for binary files. I noticed the xml includes format_version property on the root node... could that be utilized to keep backwrds compatibility?

riccardobl commented 1 year ago

I noticed the xml includes format_version property on the root node... could that be utilized to keep backwrds compatibility?

Why not? But i think a more solid approach would be to prefix every key with "jme-" in the xml importer/exporter