From 44e02f756b0066f61e1bbea648f92ea23a0aac02 Mon Sep 17 00:00:00 2001 From: Florent Xicluna Date: Wed, 8 Feb 2012 16:33:04 +0100 Subject: [PATCH 1/2] =?UTF-8?q?[FIX]=C2=A0file=5Fopen=20should=20not=20sea?= =?UTF-8?q?rch=20zip=20files=20outside=20its=20root=20directory.=20=20Fix?= =?UTF-8?q?=20the=20returned=20value=20with=20pathinfo=3DTrue.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lp bug: https://launchpad.net/bugs/928507 fixed lp bug: https://launchpad.net/bugs/928376 fixed bzr revid: florent.xicluna@gmail.com-20120208153304-9443zx2z09bws10x --- openerp/tools/misc.py | 89 ++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 40 deletions(-) diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index a5c72900fc7..3c9210214b1 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -134,7 +134,7 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False): @param name name of the file @param mode file open mode @param subdir subdirectory - @param pathinfo if True returns tupple (fileobject, filepath) + @param pathinfo if True returns tuple (fileobject, filepath) @return fileobject if pathinfo is False else (fileobject, filepath) """ @@ -142,44 +142,51 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False): adps = addons.module.ad_paths rtp = os.path.normcase(os.path.abspath(config['root_path'])) - if name.replace(os.path.sep, '/').startswith('addons/'): + if os.path.isabs(name): + # It is an absolute path + # Is it below 'addons_path' or 'root_path'? + name = os.path.normcase(os.path.normpath(name)) + for root in adps + [rtp]: + if name.startswith(root): + base = root.rstrip(os.sep) + name = name[len(base) + 1:] + break + else: + # It is outside the OpenERP root: skip zipfile lookup. + base, name = os.path.split(name) + return _fileopen(name, mode=mode, basedir=base, pathinfo=pathinfo) + + if name.replace(os.sep, '/').startswith('addons/'): subdir = 'addons' - name = name[7:] + name2 = name[7:] + elif subdir: + name = os.path.join(subdir, name) + if name.replace(os.sep, '/').startswith('addons/'): + subdir = 'addons' + name2 = name[7:] + else: + name2 = name - # First try to locate in addons_path + # First, try to locate in addons_path if subdir: - subdir2 = subdir - if subdir2.replace(os.path.sep, '/').startswith('addons/'): - subdir2 = subdir2[7:] - - subdir2 = (subdir2 != 'addons' or None) and subdir2 - for adp in adps: try: - if subdir2: - fn = os.path.join(adp, subdir2, name) - else: - fn = os.path.join(adp, name) - fn = os.path.normpath(fn) - fo = file_open(fn, mode=mode, subdir=None, pathinfo=pathinfo) - if pathinfo: - return fo, fn - return fo + return _fileopen(name2, mode=mode, basedir=adp, + pathinfo=pathinfo) except IOError: pass - if subdir: - name = os.path.join(rtp, subdir, name) - else: - name = os.path.join(rtp, name) + # Second, try to locate in root_path + return _fileopen(name, mode=mode, basedir=rtp, pathinfo=pathinfo) - name = os.path.normpath(name) - # Check for a zipfile in the path - head = name - zipname = False +def _fileopen(path, mode, basedir, pathinfo): + head = os.path.normpath(path) + name = os.path.normpath(os.path.join(basedir, path)) name2 = False - while True: + zipname = False + # Check for a zipfile in the path, but stop at the 'basedir' level + while os.sep in head: head, tail = os.path.split(head) if not tail: break @@ -187,9 +194,10 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False): zipname = os.path.join(tail, zipname) else: zipname = tail - if zipfile.is_zipfile(head+'.zip'): + zpath = os.path.join(basedir, head + '.zip') + if zipfile.is_zipfile(zpath): from cStringIO import StringIO - zfile = zipfile.ZipFile(head+'.zip') + zfile = zipfile.ZipFile(zpath) try: fo = StringIO() fo.write(zfile.read(os.path.join( @@ -197,20 +205,21 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False): os.sep, '/'))) fo.seek(0) if pathinfo: - return fo, name + return (fo, name) return fo except Exception: - name2 = os.path.normpath(os.path.join(head + '.zip', zipname)) - pass - for i in (name2, name): - if i and os.path.isfile(i): - fo = file(i, mode) + name2 = os.path.normpath(os.path.join(zpath, zipname)) + # Look for a normal file + for fname in (name2, name): + if fname and os.path.isfile(fname): + fo = open(fname, mode) if pathinfo: - return fo, i + return (fo, fname) return fo - if os.path.splitext(name)[1] == '.rml': - raise IOError, 'Report %s doesn\'t exist or deleted : ' %str(name) - raise IOError, 'File not found : %s' % name + # Not found + if name.endswith('.rml'): + raise IOError('Report %r doesn\'t exist or deleted' % name) + raise IOError('File not found: %s' % name) #---------------------------------------------------------- From 69a5eca5b48740c44e35ac5b0285e22a17c5dd8d Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 8 Feb 2012 18:39:32 +0100 Subject: [PATCH 2/2] [IMP] Give precedence to module directories instead of zips while locating resources The previous behavior gave the precedence to zipped modules, without any apparent reason, and this is sub-optimal for several reasons: 1. The default is to have regular modules, not zipped modules, so looking first for a regular module is more efficient. 2. Keeping a zipped module next to a regular module with the same name is not a documented or supported feature. 3. Even if you were relying on this behavior having the extracted module take precedence is more practical: you could simply extract the zipped module to test a quick fix. We have another issue related to this feature because the code looking for zipped modules escapes the addons paths chroot and goes up to the filesystem root looking for a zip module at each step. This is described in bug 928376 and a fix for it should follow. lp bug: https://launchpad.net/bugs/928376 fixed bzr revid: odo@openerp.com-20120208173932-pwhz53vxxdzbo8ja --- openerp/modules/module.py | 25 ++++++++++++++----------- openerp/tools/misc.py | 24 +++++++++++++++--------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/openerp/modules/module.py b/openerp/modules/module.py index 58a4547ff07..3d3bc0d9adf 100644 --- a/openerp/modules/module.py +++ b/openerp/modules/module.py @@ -280,25 +280,28 @@ def get_module_as_zip(modulename, b64enc=True, src=True): def get_module_resource(module, *args): """Return the full path of a resource of the given module. - @param module: the module - @param args: the resource path components + :param module: module name + :param list(str) args: resource path components within module - @return: absolute path to the resource + :rtype: str + :return: absolute path to the resource TODO name it get_resource_path TODO make it available inside on osv object (self.get_resource_path) """ - a = get_module_path(module) - if not a: return False - resource_path = opj(a, *args) - if zipfile.is_zipfile( a +'.zip') : - zip = zipfile.ZipFile( a + ".zip") + mod_path = get_module_path(module) + if not mod_path: return False + resource_path = opj(mod_path, *args) + if os.path.isdir(mod_path): + # the module is a directory - ignore zip behavior + if os.path.exists(resource_path): + return resource_path + elif zipfile.is_zipfile(mod_path + '.zip'): + zip = zipfile.ZipFile( mod_path + ".zip") files = ['/'.join(f.split('/')[1:]) for f in zip.namelist()] resource_path = '/'.join(args) if resource_path in files: - return opj(a, resource_path) - elif os.path.exists(resource_path): - return resource_path + return opj(mod_path, resource_path) return False def get_module_icon(module): diff --git a/openerp/tools/misc.py b/openerp/tools/misc.py index a5c72900fc7..e0be5ffaef9 100644 --- a/openerp/tools/misc.py +++ b/openerp/tools/misc.py @@ -175,10 +175,22 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False): name = os.path.normpath(name) - # Check for a zipfile in the path + # Give higher priority to module directories, which is + # a more common case than zipped modules. + if os.path.isfile(name): + fo = file(name, mode) + if pathinfo: + return fo, name + return fo + + # Support for loading modules in zipped form. + # This will not work for zipped modules that are sitting + # outside of known addons paths. head = name zipname = False - name2 = False + # FIXME: implement chrooting inside addons paths and fix + # for incorrect path_info behavior. Work in progress by + # Florent X linked to bug 928376 while True: head, tail = os.path.split(head) if not tail: @@ -200,14 +212,8 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False): return fo, name return fo except Exception: - name2 = os.path.normpath(os.path.join(head + '.zip', zipname)) pass - for i in (name2, name): - if i and os.path.isfile(i): - fo = file(i, mode) - if pathinfo: - return fo, i - return fo + if os.path.splitext(name)[1] == '.rml': raise IOError, 'Report %s doesn\'t exist or deleted : ' %str(name) raise IOError, 'File not found : %s' % name