openlp-core team mailing list archive

[Meinert Jordan <meinertjordan@xxxxxxxxxx>]

Meinert Jordan has proposed merging lp:~m2j/openlp/work into lp:openlp.

Requested reviews:
  Tim Bentley (trb143)
  Jon Tibble (meths)

For more details, see:
https://code.launchpad.net/~m2j/openlp/work/+merge/85949

Hmm, I implemented file name recovery from the XML file content for v1.x themes. As I thought about more and more strange inputs, the code finally changed quite much. (I do not really like the fact, that I changed so much, even it was basically working, but the result seems quite solid)

Well, I tested it with couple of strange theme files on Linux and checked potentially system dependent calls on a windows console. I removed the file_is_unicode() call, as it does not really save redundancy and made the code more opaque.

-- 
https://code.launchpad.net/~m2j/openlp/work/+merge/85949
Your team OpenLP Core is subscribed to branch lp:openlp.

=== modified file 'openlp/core/ui/filerenamedialog.py'
--- openlp/core/ui/filerenamedialog.py	2011-06-12 16:02:52 +0000
+++ openlp/core/ui/filerenamedialog.py	2011-12-15 20:24:03 +0000
@@ -41,7 +41,7 @@
         self.dialogLayout.addWidget(self.fileNameLabel, 0, 0)
         self.fileNameEdit = QtGui.QLineEdit(fileRenameDialog)
         self.fileNameEdit.setValidator(QtGui.QRegExpValidator(
-            QtCore.QRegExp(r'[^/\\?*|<>\[\]":<>+%]+'), self))
+            QtCore.QRegExp(r'[^/\\?*|<>\[\]":+%]+'), self))
         self.fileNameEdit.setObjectName(u'fileNameEdit')
         self.dialogLayout.addWidget(self.fileNameEdit, 0, 1)
         self.buttonBox = create_accept_reject_button_box(fileRenameDialog, True)

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2011-12-12 18:00:12 +0000
+++ openlp/core/ui/servicemanager.py	2011-12-15 20:24:03 +0000
@@ -43,8 +43,7 @@
     context_menu_action, context_menu_separator, find_and_set_in_combo_box
 from openlp.core.ui import ServiceNoteForm, ServiceItemEditForm, StartTimeForm
 from openlp.core.ui.printserviceform import PrintServiceForm
-from openlp.core.utils import AppLocation, delete_file, file_is_unicode, \
-    split_filename
+from openlp.core.utils import AppLocation, delete_file, split_filename
 from openlp.core.utils.actions import ActionList, CategoryOrder
 
 class ServiceManagerList(QtGui.QTreeWidget):
@@ -636,8 +635,11 @@
         try:
             zip = zipfile.ZipFile(fileName)
             for zipinfo in zip.infolist():
-                ucsfile = file_is_unicode(zipinfo.filename)
-                if not ucsfile:
+                try:
+                    ucsfile = zipinfo.filename.decode(u'utf-8')
+                except UnicodeDecodeError:
+                    log.exception(u'Filename "%s" is not valid UTF-8' %
+                        zipinfo.filename.decode(u'utf-8', u'replace'))
                     critical_error_message_box(
                         message=translate('OpenLP.ServiceManager',
                         'File is not a valid service.\n'

=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2011-12-11 16:23:24 +0000
+++ openlp/core/ui/thememanager.py	2011-12-15 20:24:03 +0000
@@ -30,6 +30,7 @@
 import shutil
 import logging
 import locale
+import re
 
 from xml.etree.ElementTree import ElementTree, XML
 from PyQt4 import QtCore, QtGui
@@ -43,8 +44,7 @@
     context_menu_action, context_menu_separator
 from openlp.core.theme import Theme
 from openlp.core.ui import FileRenameForm, ThemeForm
-from openlp.core.utils import AppLocation, delete_file, file_is_unicode, \
-    get_filesystem_encoding
+from openlp.core.utils import AppLocation, delete_file, get_filesystem_encoding
 
 log = logging.getLogger(__name__)
 
@@ -147,6 +147,7 @@
         check_directory_exists(self.thumbPath)
         self.themeForm.path = self.path
         self.oldBackgroundImage = None
+        self.bad_v1_name_chars = re.compile(r'[%+\[\]]')
         # Last little bits of setting up
         self.configUpdated()
 
@@ -524,44 +525,50 @@
         filexml = None
         try:
             zip = zipfile.ZipFile(filename)
-            themename = None
-            for file in zip.namelist():
-                # Handle UTF-8 files
-                ucsfile = file_is_unicode(file)
-                if not ucsfile:
-                    # Handle native Unicode files from Windows
-                    ucsfile = file
-                osfile = unicode(QtCore.QDir.toNativeSeparators(ucsfile))
-                theme_dir = None
-                if osfile.endswith(os.path.sep):
-                    theme_dir = os.path.join(dir, osfile)
-                    check_directory_exists(theme_dir)
-                else:
-                    fullpath = os.path.join(dir, osfile)
-                    names = osfile.split(os.path.sep)
-                    if len(names) > 1:
-                        # not preview file
-                        if themename is None:
-                            themename = names[0]
-                        if theme_dir is None:
-                            theme_dir = os.path.join(dir, names[0])
-                            check_directory_exists(theme_dir)
-                        if os.path.splitext(ucsfile)[1].lower() in [u'.xml']:
-                            xml_data = zip.read(file)
-                            xml_data = file_is_unicode(xml_data)
-                            if not xml_data:
-                                break
-                            filexml = self._checkVersionAndConvert(xml_data)
-                            outfile = open(fullpath, u'w')
-                            outfile.write(filexml.encode(u'utf-8'))
-                        else:
-                            outfile = open(fullpath, u'wb')
-                            outfile.write(zip.read(file))
-        except (IOError, NameError, zipfile.BadZipfile):
-            critical_error_message_box(
-                translate('OpenLP.ThemeManager', 'Validation Error'),
-                translate('OpenLP.ThemeManager', 'File is not a valid theme.'))
+            xmlfile = filter(lambda name:
+                os.path.splitext(name)[1].lower() == u'.xml', zip.namelist())
+            if len(xmlfile) != 1:
+                log.exception(u'Theme contains "%s" XML files' % len(xmlfile))
+                raise Exception(u'validation')
+            xml_tree = ElementTree(element=XML(zip.read(xmlfile[0]))).getroot()
+            v1_background = xml_tree.find(u'BackgroundType')
+            if v1_background is not None:
+                (themename, filexml, outfile) = self.unzipVersion122(dir, zip,
+                    xmlfile[0], xml_tree, v1_background, outfile)
+            else:
+                themename = xml_tree.find(u'name').text.strip()
+                for name in zip.namelist():
+                    try:
+                        uname = unicode(name, u'utf-8')
+                    except UnicodeDecodeError:
+                        log.exception(u'Theme file contains non utf-8 filename'
+                            u' "%s"' % name.decode(u'utf-8', u'replace'))
+                        raise Exception(u'validation')
+                    uname = unicode(QtCore.QDir.toNativeSeparators(uname))
+                    splitname = uname.split(os.path.sep)
+                    if splitname[-1] == u'' or len(splitname) == 1:
+                        # is directory or preview file
+                        continue
+                    fullname = os.path.join(dir, uname)
+                    check_directory_exists(os.path.dirname(fullname))
+                    if os.path.splitext(uname)[1].lower() == u'.xml':
+                        filexml = unicode(zip.read(name), u'utf-8')
+                        outfile = open(fullname, u'w')
+                        outfile.write(filexml.encode(u'utf-8'))
+                    else:
+                        outfile = open(fullname, u'wb')
+                        outfile.write(zip.read(name))
+                    outfile.close()
+        except (IOError, zipfile.BadZipfile):
             log.exception(u'Importing theme from zip failed %s' % filename)
+            raise Exception(u'validation')
+        except Exception as info:
+            if unicode(info) == u'validation':
+                critical_error_message_box(translate('OpenLP.ThemeManager',
+                    'Validation Error'), translate('OpenLP.ThemeManager',
+                    'File is not a valid theme.'))
+            else:
+                raise
         finally:
             # Close the files, to be able to continue creating the theme.
             if zip:
@@ -582,6 +589,36 @@
                 log.exception(u'Theme file does not contain XML data %s' %
                     filename)
 
+    def unzipVersion122(self, dir, zip, xmlfile, xml_tree, background, outfile):
+        """
+        Unzip openlp.org 1.2x theme file and upgrade the theme xml. When calling
+        this method, please keep in mind, that some parameters are redundant.
+        """
+        themename = xml_tree.find(u'Name').text.strip()
+        themename = self.bad_v1_name_chars.sub(u'', themename)
+        themedir = os.path.join(dir, themename)
+        check_directory_exists(themedir)
+        filexml = unicode(zip.read(xmlfile), u'utf-8')
+        filexml = self._migrateVersion122(filexml)
+        outfile = open(os.path.join(themedir, themename + u'.xml'), u'w')
+        outfile.write(filexml.encode(u'utf-8'))
+        outfile.close()
+        if background.text.strip() == u'2':
+            imagename = xml_tree.find(u'BackgroundParameter1').text.strip()
+            # image file has same extension and is in subfolder
+            imagefile = filter(lambda name: os.path.splitext(name)[1].lower()
+                == os.path.splitext(imagename)[1].lower() and name.find(r'/'),
+                zip.namelist())
+            if len(imagefile) >= 1:
+                outfile = open(os.path.join(themedir, imagename), u'wb')
+                outfile.write(zip.read(imagefile[0]))
+                outfile.close()
+            else:
+                log.exception(u'Theme file does not contain image file "%s"' %
+                    imagename.decode(u'utf-8', u'replace'))
+                raise Exception(u'validation')
+        return (themename, filexml, outfile)
+
     def checkIfThemeExists(self, themeName):
         """
         Check if theme already exists and displays error message
@@ -692,22 +729,6 @@
         image = os.path.join(self.path, theme + u'.png')
         return image
 
-    def _checkVersionAndConvert(self, xml_data):
-        """
-        Check if a theme is from OpenLP version 1
-
-        ``xml_data``
-            Theme XML to check the version of
-        """
-        log.debug(u'checkVersion1 ')
-        theme = xml_data.encode(u'ascii', u'xmlcharrefreplace')
-        tree = ElementTree(element=XML(theme)).getroot()
-        # look for old version 1 tags
-        if tree.find(u'BackgroundType') is None:
-            return xml_data
-        else:
-            return self._migrateVersion122(xml_data)
-
     def _createThemeFromXml(self, themeXml, path):
         """
         Return a theme object using information parsed from XML
@@ -772,7 +793,7 @@
         """
         theme = Theme(xml_data)
         newtheme = ThemeXML()
-        newtheme.theme_name = theme.Name
+        newtheme.theme_name = self.bad_v1_name_chars.sub(u'', theme.Name)
         if theme.BackgroundType == 0:
             newtheme.background_type = \
                 BackgroundType.to_string(BackgroundType.Solid)

=== modified file 'openlp/core/utils/__init__.py'
--- openlp/core/utils/__init__.py	2011-12-11 15:31:44 +0000
+++ openlp/core/utils/__init__.py	2011-12-15 20:24:03 +0000
@@ -455,26 +455,6 @@
     log.debug(page)
     return page
 
-def file_is_unicode(filename):
-    """
-    Checks if a file is valid unicode and returns the unicode decoded file or
-    None.
-
-    ``filename``
-        File to check is valid unicode.
-    """
-    if not filename:
-        return None
-    ucsfile = None
-    try:
-        ucsfile = filename.decode(u'utf-8')
-    except UnicodeDecodeError:
-        log.exception(u'Filename "%s" is not valid UTF-8' %
-            filename.decode(u'utf-8', u'replace'))
-    if not ucsfile:
-        return None
-    return ucsfile
-
 def get_uno_command():
     """
     Returns the UNO command to launch an openoffice.org instance.
@@ -507,5 +487,5 @@
 
 __all__ = [u'AppLocation', u'get_application_version', u'check_latest_version',
     u'add_actions', u'get_filesystem_encoding', u'LanguageManager',
-    u'ActionList', u'get_web_page', u'file_is_unicode', u'get_uno_command',
-    u'get_uno_instance', u'delete_file', u'clean_filename']
+    u'ActionList', u'get_web_page', u'get_uno_command', u'get_uno_instance',
+    u'delete_file', u'clean_filename']