Better handling of XML errors in PO files

Rather than let an exception kill itstool, just issue a warning
and use the original-language node. Added --strict to error out
for XML errors in PO files.

https://bugs.freedesktop.org/show_bug.cgi?id=41254

3 files changed, 59 insertions, 14 deletions

diff --git a/itstool.in b/itstool.in
index d0510a9..4c731f5 100755
--- a/itstool.in
+++ b/itstool.in
@@ -318,7 +318,11 @@ class Document (object):
                     self._localrules.append(child)
                 pre_process(child)
         pre_process(self._doc)
-        self._check_errors()
+        try:
+            self._check_errors()
+        except libxml2.parserError as e:
+            sys.stderr.write('Error: Could not parse document:\n%s\n' % str(e))
+            sys.exit(1)
         self._msgs = messages
         self._its_translate_nodes = {}
         self._its_within_text_nodes = {}
@@ -583,7 +587,7 @@ class Document (object):
             for node in xml_child_iter(self._itst_credits[1]):
                 self._append_credits(self._itst_credits[0], node, trdata)
 
-    def merge_translations(self, translations, language, node=None):
+    def merge_translations(self, translations, language, node=None, strict=False):
         is_root = False
         if node is None:
             is_root = True
@@ -607,9 +611,9 @@ class Document (object):
             self.translate_attrs(node, node)
             children = [child for child in xml_child_iter(node)]
             for child in children:
-                self.merge_translations(translations, language, node=child)
+                self.merge_translations(translations, language, node=child, strict=strict)
         else:
-            newnode = self.get_translated(node, translations)
+            newnode = self.get_translated(node, translations, strict=strict)
             if newnode != node:
                 self.translate_attrs(node, newnode)
                 node.replaceNode(newnode)
@@ -663,7 +667,7 @@ class Document (object):
             if newcontent:
                 newnode.setProp(attr.name, translations.ugettext(attr.get_content()))
 
-    def get_translated (self, node, translations):
+    def get_translated (self, node, translations, strict=False):
         msg = self._msgs.get_message_by_node(node)
         if msg is None:
             return node
@@ -697,6 +701,15 @@ class Document (object):
         ctxt.replaceEntities(0)
         ctxt.parseDocument()
         trnode = ctxt.doc().getRootElement()
+        try:
+            self._check_errors()
+        except libxml2.parserError as e:
+            if strict:
+                raise
+            else:
+                sys.stderr.write('Warning: Could not merge translation for msgid:\n%s\n' % msgstr)
+                self._xml_err = ''
+                return node
         def scan_node(node):
             children = [child for child in xml_child_iter(node)]
             for child in children:
@@ -705,10 +718,10 @@ class Document (object):
                 if child.ns() is not None and child.ns().content == NS_BLANK:
                     ph_node = msg.get_placeholder(child.name).node
                     if self.has_child_elements(ph_node):
-                        self.merge_translations(translations, None, ph_node)
+                        self.merge_translations(translations, None, ph_node, strict=strict)
                         child.replaceNode(ph_node)
                     else:
-                        repl = self.get_translated(ph_node, translations)
+                        repl = self.get_translated(ph_node, translations, strict=strict)
                         child.replaceNode(repl)
                 scan_node(child)
         scan_node(trnode)
@@ -914,6 +927,11 @@ if __name__ == '__main__':
                        default=None,
                        metavar='OUT',
                        help='output PO files to file OUT or XML files in directory OUT')
+    options.add_option('-s', '--strict',
+                       action='store_true',
+                       dest='strict',
+                       default=False,
+                       help='Exit with error when PO files contain broken XML')
     options.add_option('-v', '--version',
                        action='store_true',
                        dest='version',
@@ -971,7 +989,11 @@ if __name__ == '__main__':
             if opts.itsfile is not None:
                 for itsfile in opts.itsfile:
                     doc.apply_its_file(itsfile)
-            doc.merge_translations(translations, opts.lang)
+            try:
+                doc.merge_translations(translations, opts.lang, strict=opts.strict)
+            except Exception as e:
+                sys.stderr.write('Error: Could not merge translations:\n%s\n' % str(e))
+                sys.exit(1)
             fout = out
             if isinstance(fout, basestring):
                 fout = file(os.path.join(fout, os.path.basename(filename)), 'w')
diff --git a/tests/Translate3.ll.wrong.xml b/tests/Translate3.ll.wrong.xml
new file mode 100644
index 0000000..3c8c7b2
--- /dev/null
+++ b/tests/Translate3.ll.wrong.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="utf-8"?>
+<book xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:its="http://www.w3.org/2005/11/its" xsi:noNamespaceSchemaLocation="EX-ways-to-use-its-5.xsd" its:version="1.0" lang="test">
+ <head>
+  <title>La vie d'un simple homme</title>
+ </head>
+ <body>
+  <p>Everything started when Zebulon discovered that he had 
+   a <its:span translate="no">doppelgänger</its:span> who was a
+   serious baseball <its:span translate="no">aficionado</its:span>.</p>
+ </body>
+</book>
+<!-- timestamp $Id: Translate3.xml,v 1.2 2006/12/19 06:24:35 fsasaki Exp $ -->
diff --git a/tests/run_tests.py b/tests/run_tests.py
index 906d638..3447b8d 100644
--- a/tests/run_tests.py
+++ b/tests/run_tests.py
@@ -44,7 +44,7 @@ class ItstoolTests(unittest.TestCase):
             self.assertFilesEqual(os.path.join(TEST_DIR, "test.pot"), os.path.join(TEST_DIR, reference_pot))
         return result
 
-    def _test_translation_process(self, start_file, expected_status=0, po_file=None):
+    def _test_translation_process(self, start_file, expected_status=0, po_file=None, xml_file=None, options=None):
         start_file_base = os.path.splitext(start_file)[0]
         self._test_pot_generation(start_file)
 
@@ -52,16 +52,19 @@ class ItstoolTests(unittest.TestCase):
         if po_file is None:
             po_file = "%s.ll.po" % start_file_base
         self.run_command("cd %(dir)s && msgfmt -o test.mo %(po_file)s" % {'dir': TEST_DIR, 'po_file': po_file}) 
-        result = self.run_command("cd %(dir)s && python itstool_test -m %(mo)s -o %(res)s %(src)s" % {
+        result = self.run_command("cd %(dir)s && python itstool_test %(opt)s -m %(mo)s -o %(res)s %(src)s" % {
             'dir': ITSTOOL_DIR,
+            'opt': (options or ''),
             'mo' : os.path.join(TEST_DIR, "test.mo"),
             'res': os.path.join(TEST_DIR, "test.xml"),
             'src': os.path.join(TEST_DIR, start_file),
         }, expected_status)
+        if xml_file is None:
+            xml_file = "%s.ll.xml" % start_file_base
         if (expected_status == 0):
             self.assertFilesEqual(
                 os.path.join(TEST_DIR, "test.xml"),
-                os.path.join(TEST_DIR, "%s.ll.xml" % start_file_base)
+                os.path.join(TEST_DIR, xml_file)
             )
         return result
 
@@ -134,12 +137,20 @@ class ItstoolTests(unittest.TestCase):
     def test_bad_file(self):
         """ Test that a malformed XML generates a proper exception """
         res = self._test_pot_generation('Malformed.xml', expected_status=1)
-        self.assertTrue("libxml2.parserError" in res['errors'])
+        #self.assertTrue("libxml2.parserError" in res['errors'])
 
     def test_bad_translation(self):
         """ Test that bad XML syntax in translation generates a proper exception """
-        res = self._test_translation_process('Translate3.xml', expected_status=1, po_file='Translate3.ll.wrong.po')
-        self.assertTrue("libxml2.parserError" in res['errors'])
+        res = self._test_translation_process('Translate3.xml', expected_status=1,
+                                             po_file='Translate3.ll.wrong.po',
+                                             options='-s')
+        #self.assertTrue("libxml2.parserError" in res['errors'])
+
+    def test_bad_translation(self):
+        """ Test that bad XML syntax in translation is handled gracefully """
+        res = self._test_translation_process('Translate3.xml',
+                                             po_file='Translate3.ll.wrong.po',
+                                             xml_file='Translate3.ll.wrong.xml')
 
 
 class ITSTestRunner(unittest.TextTestRunner):