Address corner case bugs in the check access and get file size helpers in fileio.py

mig/shared/fileio.py

@@ -1,3 +1,3 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
@@ -20,7 +20,7 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #
 # -- END_HEADER ---
 #
@@ -43,7 +43,7 @@
 # NOTE: We expose optimized walk function directly for ease and efficiency.
 #       Requires stand-alone scandir module on python 2 whereas the native os
 #       functions are built-in and optimized similarly on python 3+
 slow_walk, slow_listdir = False, False
 if sys.version_info[0] > 2:
     from os import walk, listdir
 else:
@@ -51,12 +51,12 @@
         from distutils.version import StrictVersion
         from scandir import walk, listdir, __version__ as scandir_version
         if StrictVersion(scandir_version) < StrictVersion("1.3"):
             # Important os.walk compatibility utf8 fixes were not added until 1.3
             raise ImportError(
                 "scandir version is too old: fall back to os.walk")
     except ImportError as err:
         # print("DEBUG: not using scandir: %s" % err)
         slow_walk = slow_listdir = True
         walk = os.walk
         listdir = os.listdir
 
@@ -71,7 +71,7 @@
     exit(1)
 
 
 def _auto_adjust_mode(data, mode):
     """Select suitable file open mode based on string type of data. I.e. whether
     to use binary or text mode depending on data in bytes or unicode format.
     """
@@ -127,7 +127,7 @@
             if offset > 0:
                 try:
                     filehandle.seek(offset)
                 except:
                     filehandle.seek(0, 2)
                     file_size = filehandle.tell()
                     for _ in range(offset - file_size):
@@ -150,7 +150,9 @@
         return False
 
 # TODO: toggle default force_string here when we have auto mode select?
+
+
 def write_chunk(path, chunk, offset, logger, mode='r+b', force_string=True):
     """Wrapper to handle writing of chunks with offset to path.
     Creates file first if it doesn't already exist.
     """
@@ -176,7 +178,7 @@
         old_umask = os.umask(umask)
 
     # TODO: enable this again once throuroughly tested and assured py2+3 safe
-    #mode = _auto_adjust_mode(content, mode)
+    # mode = _auto_adjust_mode(content, mode)
 
     retval = _write_chunk(path, content, offset=0, logger=logger, mode=mode,
                           make_parent=make_parent, create_file=False,
@@ -188,7 +190,7 @@
     return retval
 
 
 def write_file_lines(lines, path, logger, mode='w', make_parent=True, umask=None):
     """Wrapper to handle writing of lines of content to path"""
     return write_file(''.join(lines), path, logger, mode, make_parent, umask)
 
@@ -210,7 +212,7 @@
     return content
 
 
 def read_file_lines(path, logger, mode='r'):
     """Wrapper to handle reading lines of content from path"""
     contents = read_file(path, logger, mode)
     if contents is None:
@@ -224,7 +226,7 @@
     return lines
 
 
 def read_head_lines(path, lines, logger, mode='r'):
     """Read first lines from path"""
     if not logger:
         logger = null_logger("dummy")
@@ -249,7 +251,7 @@
         # NOTE: last line is likely truncated when read like this
         while pos < size and contents.count(b'\n') < lines:
             pos = head_fd.tell()
             # logger.debug("read %db at pos %d from %r" % (step_size, pos, path))
             contents += head_fd.read(step_size)
             step_size *= 2
         head_fd.close()
@@ -263,7 +265,7 @@
     return out_lines[:lines]
 
 
 def read_tail_lines(path, lines, logger, mode='r'):
     """Read last lines from path"""
     if not logger:
         logger = null_logger("dummy")
@@ -302,7 +304,7 @@
     return out_lines[-lines:]
 
 
 def get_file_size(path, logger):
     """Wrapper to handle getsize of path"""
     if not logger:
         logger = null_logger("dummy")
@@ -311,7 +313,7 @@
         return os.path.getsize(path)
     except Exception as err:
         logger.error("could not get size for %r: %s" % (path, err))
-        result = -1
+        return -1
 
 
 def delete_file(path, logger, allow_broken_symlink=False, allow_missing=False):
@@ -497,6 +499,7 @@
     """Write message to notify home"""
     if not logger:
         logger = null_logger("dummy")
+    filepath = 'UNDEFINED'
     try:
         (filedescriptor, filepath) = make_temp_file(
             suffix='.%s' % time.time(),
@@ -752,7 +755,7 @@
     argument decides if any symlinks in path are expanded before this check
     and it is on by default.
     """
-    return _check_access(path, os.O_RDONLY, parent_dir, follow_symlink)
+    return _check_access(path, os.R_OK, parent_dir, follow_symlink)
 
 
 def check_write_access(path, parent_dir=False, follow_symlink=True):
@@ -762,8 +765,7 @@
     decides if any symlinks in path are expanded before this check and it is
     on by default.
     """
-    # IMPORTANT: we need to use RDWR rather than WRONLY here.
-    return _check_access(path, os.O_RDWR, parent_dir, follow_symlink)
+    return _check_access(path, os.W_OK, parent_dir, follow_symlink)
 
 
 def make_temp_file(suffix='', prefix='tmp', dir=None, text=False):
@@ -787,8 +789,8 @@
         os.write(filehandle, force_utf8(contents))
         os.close(filehandle)
     except Exception as exc:
-        _logger.error("failed to write tempfile %r: %s" % (tmpname, exc))
         tmpname = None
+        _logger.error("failed to write tempfile %r: %s" % (tmpname, exc))
     return tmpname
 
 
@@ -884,7 +886,7 @@
         # Clean up
         try:
             lock_handle.close()
         except:
             pass
         # If non-blocking flock gave up an IOError will be raised and the
         # exception will have an errno attribute set to EACCES or EAGAIN.
@@ -928,7 +930,7 @@
     if close:
         try:
             lock_handle.close()
         except:
             pass
 
 
@@ -1019,7 +1021,7 @@
     chroot_exceptions.append(os.path.abspath(configuration.resource_home))
     if configuration.site_enable_seafile and configuration.seafile_mount:
         chroot_exceptions.append(os.path.abspath(configuration.seafile_mount))
     # Allow access to mig_system_storage used for merging multiple storage backends
     chroot_exceptions.append(os.path.abspath(configuration.mig_system_storage))
     return chroot_exceptions