Java save / read data from XML - parsing exceptions

Question

My little chat client software uses a XML file to store chat history, my class for handling reading / writing is:

public class History {

    public String filePath;

    public History(String filePath) {
        this.filePath = filePath;
    }

    public String stripNonValidXMLCharacters(String in ) {
        StringBuffer out = new StringBuffer(); // Used to hold the output.
        char current; // Used to reference the current character.

        if ( in == null || ("".equals( in ))) return ""; // vacancy test.
        for (int i = 0; i < in .length(); i++) {
            current = in .charAt(i); // NOTE: No IndexOutOfBoundsException caught here; it should not happen.
            if ((current == 0x9) ||
                (current == 0xA) ||
                (current == 0xD) ||
                ((current >= 0x20) && (current <= 0xD7FF)) ||
                ((current >= 0xE000) && (current <= 0xFFFD)) ||
                ((current >= 0x10000) && (current <= 0x10FFFF)))
                out.append(current);
        }
        return out.toString();
    }

    public synchronized void addMessage(String from, String agentName, String msg, String time, String channel) {
        try {
            DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance();
            DocumentBuilder docBuilder = docFactory.newDocumentBuilder();
            org.w3c.dom.Document doc = docBuilder.parse(filePath);

            Node data = doc.getFirstChild();

            org.w3c.dom.Element root = doc.createElement(channel);
            org.w3c.dom.Element message = doc.createElement("message");
            org.w3c.dom.Element _sender = doc.createElement("sender");
            _sender.setTextContent(stripNonValidXMLCharacters(from));
            org.w3c.dom.Element _content = doc.createElement("content");
            _content.setTextContent(stripNonValidXMLCharacters(msg));
            org.w3c.dom.Element _recipient = doc.createElement("recipient");
            _recipient.setTextContent(stripNonValidXMLCharacters(agentName));
            org.w3c.dom.Element _time = doc.createElement("time");
            _time.setTextContent(stripNonValidXMLCharacters(time));


            message.appendChild(_sender);
            message.appendChild(_content);
            message.appendChild(_recipient);
            message.appendChild(_time);
            root.appendChild(message);

            data.appendChild(root);

            TransformerFactory transformerFactory = TransformerFactory.newInstance();
            Transformer transformer = transformerFactory.newTransformer();
            DOMSource source = new DOMSource(doc);
            StreamResult result = new StreamResult(new File(filePath));
            transformer.transform(source, result);

        } catch (Exception ex) {
            System.out.println(ex.getStackTrace());
            // This is being trown randomly
        }
    }

    public synchronized void getHistory(String channel) {
        try {
            File fXmlFile = new File(filePath);
            DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
            DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
            org.w3c.dom.Document doc = dBuilder.parse(fXmlFile);
            doc.getDocumentElement().normalize();

            NodeList nList = doc.getElementsByTagName(channel);

            for (int temp = 0; temp < nList.getLength(); temp++) {
                Node nNode = nList.item(temp);
                if (nNode.getNodeType() == Node.ELEMENT_NODE) {
                    org.w3c.dom.Element eElement = (org.w3c.dom.Element) nNode;

                    if (getTagValue("sender", eElement).contains("Serv1")) {
                        printServerMsg(getTagValue("content", eElement).replace("%27", "'"), "", getTagValue("time", eElement));
                    } else {
                        printMsg(getTagValue("content", eElement).replace("%27", "'"), getTagValue("sender", eElement), getTagValue("time", eElement));
                    }

                }
            }
        } catch (Exception ex) {
            System.out.println("Filling Exception");

            // This is also being trown
        }
    }

    public String getTagValue(String sTag, org.w3c.dom.Element eElement) {
        NodeList nlList = eElement.getElementsByTagName(sTag).item(0).getChildNodes();
        Node nValue = (Node) nlList.item(0);
        return nValue.getNodeValue();
    }
}

Exception when writing i am getting is: `INVALID_CHARACTER_ERR: An invalid or illegal XML character is specified.``

And when reading the exception Filling exception The exception here is java.lang.NullPointerException

Any ideas how i can guarantee that this wont happend? Problem is this basically breaks the whole client currently

Answer 1

Instead of

System.out.println("Filling Exception");

ALWAYS use either

e.printStackTrace();

Or a proper logging framework, like log4j, and log the error properly:

log.error("Filling Exception", e);

Why is this important?

Because by doing this, you could have provided the full stack trace for us...

Also, for escaping Strings to be used as XML content, it is wise to use an already well written, proven utility like the Apache commons StringEscapeUtils instead of reinventing the wheel

EDIT

From OPs comment

Oh thanks, i can now see that the error is in getTagValue()

There are multiple possibilities for NPE here, but the cuplrit is in this line

NodeList nlList = eElement.getElementsByTagName(sTag).item(0).getChildNodes();

the following can be null

eElement.getElementsByTagName(sTag).item(0)

if there is no tag by that name in the document.

From the documentation:

Returns the indexth item in the collection. If index is greater than or equal to the number of nodes in the list, this returns null.

So I'd rewrite the method like this:

public String getTagValue(String sTag, org.w3c.dom.Element eElement) {
    Node n = eElement.getElementsByTagName(sTag).item(0);
    if(e !=null) {
        NodeList nlList = n.getChildNodes();
        Node nValue = (Node) nlList.item(0);
        if(nValue != null) {
            return nValue.getNodeValue();
        }
    }
    return ""; // or return null; if that is more applicable to the use case
}

Also be careful that now this returns an empty string, which might or might not be good: it can go unnoticed that said node does not exist...