summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPrashanth Pai <ppai@redhat.com>2016-05-30 15:08:48 +0530
committerPrashanth Pai <ppai@redhat.com>2016-06-15 15:02:18 +0530
commitb111d50347076336b3e655178d967f8e5c8c9913 (patch)
treeb4ad1944d0379f560fae379efd27d69e7c12eb65
parent759471ddcd76306b952bb2ee28f211afc9e24f3a (diff)
Add validation decorators
As glfs and glfd are pointers to memory locations, passing invalid values of glfs and glfd to the libgfapi C library can result in segfault. This patch introduces decorators that validate glfs and glfd before calling correspoding C APIs. Change-Id: I4e86bd8e436e23cd41f75f428d246939c820bb9c Signed-off-by: Prashanth Pai <ppai@redhat.com>
-rw-r--r--gluster/exceptions.py4
-rwxr-xr-xgluster/gfapi.py78
-rw-r--r--gluster/utils.py62
-rw-r--r--test/functional/libgfapi-python-tests.py23
-rw-r--r--test/unit/gluster/test_gfapi.py31
-rw-r--r--test/unit/gluster/test_utils.py70
6 files changed, 252 insertions, 16 deletions
diff --git a/gluster/exceptions.py b/gluster/exceptions.py
index 2bb4732..962e69f 100644
--- a/gluster/exceptions.py
+++ b/gluster/exceptions.py
@@ -16,3 +16,7 @@
class LibgfapiException(Exception):
pass
+
+
+class VolumeNotMounted(LibgfapiException):
+ pass
diff --git a/gluster/gfapi.py b/gluster/gfapi.py
index 6fed798..121d442 100755
--- a/gluster/gfapi.py
+++ b/gluster/gfapi.py
@@ -17,6 +17,7 @@ import stat
import errno
from gluster import api
from gluster.exceptions import LibgfapiException
+from gluster.utils import validate_mount, validate_glfd
# TODO: Move this utils.py
python_mode_to_os_flags = {}
@@ -53,18 +54,24 @@ class File(object):
self.fd = fd
self.originalpath = path
self._mode = mode
- self._closed = False
+ self._validate_init()
def __enter__(self):
- if self.fd is None:
- # __enter__ should only be called within the context
- # of a 'with' statement when opening a file through
- # Volume.open()
- raise ValueError("I/O operation on closed file")
+ # __enter__ should only be called within the context
+ # of a 'with' statement when opening a file through
+ # Volume.fopen()
+ self._validate_init()
return self
def __exit__(self, type, value, tb):
- self.close()
+ if self.fd:
+ self.close()
+
+ def _validate_init(self):
+ if self.fd is None:
+ raise ValueError("I/O operation on invalid fd")
+ elif not isinstance(self.fd, int):
+ raise ValueError("I/O operation on invalid fd")
@property
def fileno(self):
@@ -72,7 +79,6 @@ class File(object):
Return the internal file descriptor (glfd) that is used by the
underlying implementation to request I/O operations.
"""
- # TODO: Make self.fd private (self._fd)
return self.fd
@property
@@ -98,22 +104,22 @@ class File(object):
Bool indicating the current state of the file object. This is a
read-only attribute; the close() method changes the value.
"""
- return self._closed
+ return not self.fd
+ @validate_glfd
def close(self):
"""
Close the file. A closed file cannot be read or written any more.
- Calling close() more than once is allowed.
:raises: OSError on failure
"""
- if not self._closed:
- ret = api.glfs_close(self.fd)
- if ret < 0:
- err = ctypes.get_errno()
- raise OSError(err, os.strerror(err))
- self._closed = True
+ ret = api.glfs_close(self.fd)
+ if ret < 0:
+ err = ctypes.get_errno()
+ raise OSError(err, os.strerror(err))
+ self.fd = None
+ @validate_glfd
def discard(self, offset, length):
"""
This is similar to UNMAP command that is used to return the
@@ -133,6 +139,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def dup(self):
"""
Return a duplicate of File object. This duplicate File class instance
@@ -146,6 +153,7 @@ class File(object):
raise OSError(err, os.strerror(err))
return File(dupfd, self.originalpath)
+ @validate_glfd
def fallocate(self, mode, offset, length):
"""
This is a Linux-specific sys call, unlike posix_fallocate()
@@ -164,6 +172,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def fchmod(self, mode):
"""
Change this file's mode
@@ -176,6 +185,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def fchown(self, uid, gid):
"""
Change this file's owner and group id
@@ -189,6 +199,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def fdatasync(self):
"""
Flush buffer cache pages pertaining to data, but not the ones
@@ -201,6 +212,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def fgetsize(self):
"""
Return the size of a file, as reported by fstat()
@@ -209,6 +221,7 @@ class File(object):
"""
return self.fstat().st_size
+ @validate_glfd
def fgetxattr(self, key, size=0):
"""
Retrieve the value of the extended attribute identified by key
@@ -235,6 +248,7 @@ class File(object):
raise OSError(err, os.strerror(err))
return buf.value[:rc]
+ @validate_glfd
def flistxattr(self, size=0):
"""
Retrieve list of extended attributes for the file.
@@ -275,6 +289,7 @@ class File(object):
xattrs.sort()
return xattrs
+ @validate_glfd
def fsetxattr(self, key, value, flags=0):
"""
Set extended attribute of file.
@@ -295,6 +310,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def fremovexattr(self, key):
"""
Remove a extended attribute of the file.
@@ -307,6 +323,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def fstat(self):
"""
Returns Stat object for this file.
@@ -321,6 +338,7 @@ class File(object):
raise OSError(err, os.strerror(err))
return s
+ @validate_glfd
def fsync(self):
"""
Flush buffer cache pages pertaining to data and metadata.
@@ -332,6 +350,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def ftruncate(self, length):
"""
Truncated the file to a size of length bytes.
@@ -348,6 +367,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def lseek(self, pos, how):
"""
Set the read/write offset position of this file.
@@ -367,6 +387,7 @@ class File(object):
raise OSError(err, os.strerror(err))
return ret
+ @validate_glfd
def read(self, size=-1):
"""
Read at most size bytes from the file.
@@ -389,6 +410,7 @@ class File(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_glfd
def readinto(self, buf):
"""
Read up to len(buf) bytes into buf which must be a bytearray.
@@ -413,6 +435,7 @@ class File(object):
raise OSError(err, os.strerror(err))
return nread
+ @validate_glfd
def write(self, data, flags=0):
"""
Write data to the file.
@@ -434,6 +457,7 @@ class File(object):
raise OSError(err, os.strerror(err))
return ret
+ @validate_glfd
def zerofill(self, offset, length):
"""
Fill 'length' number of bytes with zeroes starting from 'offset'.
@@ -619,6 +643,7 @@ class Volume(object):
self.log_file = log_file
self.log_level = log_level
+ @validate_mount
def access(self, path, mode):
"""
Use the real uid/gid to test for access to path.
@@ -635,6 +660,7 @@ class Volume(object):
else:
return False
+ @validate_mount
def chdir(self, path):
"""
Change the current working directory to the given path.
@@ -647,6 +673,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def chmod(self, path, mode):
"""
Change mode of path
@@ -660,6 +687,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def chown(self, path, uid, gid):
"""
Change owner and group id of path
@@ -701,6 +729,7 @@ class Volume(object):
"""
return self.stat(path).st_ctime
+ @validate_mount
def getcwd(self):
"""
Returns current working directory.
@@ -726,6 +755,7 @@ class Volume(object):
"""
return self.stat(path).st_size
+ @validate_mount
def getxattr(self, path, key, size=0):
"""
Retrieve the value of the extended attribute identified by key
@@ -805,6 +835,7 @@ class Volume(object):
dir_list.append(name)
return dir_list
+ @validate_mount
def listxattr(self, path, size=0):
"""
Retrieve list of extended attribute keys for the specified path.
@@ -846,6 +877,7 @@ class Volume(object):
xattrs.sort()
return xattrs
+ @validate_mount
def lstat(self, path):
"""
Return stat information of path. If path is a symbolic link, then it
@@ -884,6 +916,7 @@ class Volume(object):
return
self.mkdir(path, mode)
+ @validate_mount
def mkdir(self, path, mode=0777):
"""
Create a directory named path with numeric mode mode.
@@ -896,6 +929,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def fopen(self, path, mode='r'):
"""
Similar to Python's built-in File object returned by Python's open()
@@ -936,6 +970,7 @@ class Volume(object):
raise OSError(err, os.strerror(err))
return File(fd, path=path, mode=mode)
+ @validate_mount
def open(self, path, flags, mode=0777):
"""
Similar to Python's os.open()
@@ -965,6 +1000,7 @@ class Volume(object):
return fd
+ @validate_mount
def opendir(self, path):
"""
Open a directory.
@@ -979,6 +1015,7 @@ class Volume(object):
raise OSError(err, os.strerror(err))
return Dir(fd)
+ @validate_mount
def readlink(self, path):
"""
Return a string representing the path to which the symbolic link
@@ -1005,6 +1042,7 @@ class Volume(object):
"""
return self.unlink(path)
+ @validate_mount
def removexattr(self, path, key):
"""
Remove a extended attribute of the path.
@@ -1018,6 +1056,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def rename(self, src, dst):
"""
Rename the file or directory from src to dst. If dst is a directory,
@@ -1031,6 +1070,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def rmdir(self, path):
"""
Remove (delete) the directory path. Only works when the directory is
@@ -1112,6 +1152,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def setxattr(self, path, key, value, flags=0):
"""
Set extended attribute of the path.
@@ -1134,6 +1175,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def stat(self, path):
"""
Returns stat information of path.
@@ -1147,6 +1189,7 @@ class Volume(object):
raise OSError(err, os.strerror(err))
return s
+ @validate_mount
def statvfs(self, path):
"""
Returns information about a mounted glusterfs volume. path is the
@@ -1167,6 +1210,7 @@ class Volume(object):
raise OSError(err, os.strerror(err))
return s
+ @validate_mount
def symlink(self, source, link_name):
"""
Create a symbolic link 'link_name' which points to 'source'
@@ -1178,6 +1222,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def unlink(self, path):
"""
Delete the file 'path'
@@ -1189,6 +1234,7 @@ class Volume(object):
err = ctypes.get_errno()
raise OSError(err, os.strerror(err))
+ @validate_mount
def utime(self, path, times):
"""
Set the access and modified times of the file specified by path. If
diff --git a/gluster/utils.py b/gluster/utils.py
new file mode 100644
index 0000000..833302e
--- /dev/null
+++ b/gluster/utils.py
@@ -0,0 +1,62 @@
+# Copyright (c) 2016 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+import errno
+from functools import wraps
+from gluster.exceptions import VolumeNotMounted
+
+
+def validate_mount(func):
+ """
+ Decorator to assert that volume is initialized and mounted before any
+ further I/O calls are invoked by methods.
+
+ :param func: method to be decorated and checked.
+ """
+ def _exception(volname):
+ raise VolumeNotMounted('Volume "%s" not mounted.' % (volname))
+
+ @wraps(func)
+ def wrapper(*args, **kwargs):
+ self = args[0]
+ if self.fs and self._mounted:
+ return func(*args, **kwargs)
+ else:
+ return _exception(self.volname)
+ wrapper.__wrapped__ = func
+
+ return wrapper
+
+
+def validate_glfd(func):
+ """
+ Decorator to assert that glfd is valid.
+
+ :param func: method to be decorated and checked.
+ """
+ def _exception():
+ raise OSError(errno.EBADF, os.strerror(errno.EBADF))
+
+ @wraps(func)
+ def wrapper(*args, **kwargs):
+ self = args[0]
+ if self.fd:
+ return func(*args, **kwargs)
+ else:
+ return _exception()
+ wrapper.__wrapped__ = func
+
+ return wrapper
diff --git a/test/functional/libgfapi-python-tests.py b/test/functional/libgfapi-python-tests.py
index c29f400..4e339e0 100644
--- a/test/functional/libgfapi-python-tests.py
+++ b/test/functional/libgfapi-python-tests.py
@@ -127,6 +127,29 @@ class FileOpsTest(unittest.TestCase):
self.assertRaises(OSError, self.vol.open, "file",
12345)
+ def test_double_close(self):
+ name = uuid4().hex
+ f = self.vol.fopen(name, 'w')
+ f.close()
+ for i in range(2):
+ try:
+ f.close()
+ except OSError as err:
+ self.assertEqual(err.errno, errno.EBADF)
+ else:
+ self.fail("Expecting OSError")
+
+ def test_glfd_decorators_IO_on_invalid_glfd(self):
+ name = uuid4().hex
+ with self.vol.fopen(name, 'w') as f:
+ f.write("Valar Morghulis")
+ try:
+ s = f.read()
+ except OSError as err:
+ self.assertEqual(err.errno, errno.EBADF)
+ else:
+ self.fail("Expecting OSError")
+
def test_fopen_err(self):
# mode not string
self.assertRaises(TypeError, self.vol.fopen, "file", os.O_WRONLY)
diff --git a/test/unit/gluster/test_gfapi.py b/test/unit/gluster/test_gfapi.py
index 3934a6f..86fa621 100644
--- a/test/unit/gluster/test_gfapi.py
+++ b/test/unit/gluster/test_gfapi.py
@@ -11,6 +11,7 @@
import unittest
import gluster
+import inspect
import os
import stat
import time
@@ -70,6 +71,36 @@ class TestFile(unittest.TestCase):
def tearDown(self):
gluster.gfapi.api.glfs_close = self._saved_glfs_close
+ def test_validate_init(self):
+ self.assertRaises(ValueError, File, None)
+ self.assertRaises(ValueError, File, "not_int")
+
+ try:
+ with File(None) as f:
+ pass
+ except ValueError:
+ pass
+ else:
+ self.fail("Expecting ValueError")
+
+ try:
+ with File("not_int") as f:
+ pass
+ except ValueError:
+ pass
+ else:
+ self.fail("Expecting ValueError")
+
+ def test_validate_glfd_decorator_applied(self):
+ for method_name, method_instance in \
+ inspect.getmembers(File, predicate=inspect.ismethod):
+ if not method_name.startswith('_'):
+ try:
+ wrapper_attribute = method_instance.__wrapped__.__name__
+ self.assertEqual(wrapper_attribute, method_name)
+ except AttributeError:
+ self.fail("Method File.%s isn't decorated" % (method_name))
+
def test_fchmod_success(self):
mock_glfs_fchmod = Mock()
mock_glfs_fchmod.return_value = 0
diff --git a/test/unit/gluster/test_utils.py b/test/unit/gluster/test_utils.py
new file mode 100644
index 0000000..eb7a15f
--- /dev/null
+++ b/test/unit/gluster/test_utils.py
@@ -0,0 +1,70 @@
+# Copyright (c) 2016 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+import unittest
+
+from gluster import utils
+from gluster.exceptions import VolumeNotMounted
+
+
+class TestUtils(unittest.TestCase):
+
+ def test_validate_mount(self):
+
+ class _FakeVol(object):
+
+ def __init__(self):
+ self.fs = None
+ self._mounted = None
+ self.volname = "vol1"
+
+ @utils.validate_mount
+ def test_method(self):
+ return
+
+ v = _FakeVol()
+ try:
+ v.test_method()
+ except VolumeNotMounted as err:
+ self.assertEqual(str(err), 'Volume "vol1" not mounted.')
+ else:
+ self.fail("Expected VolumeNotMounted exception.")
+
+ v.fs = 12345
+ v._mounted = True
+ # Shouldn't raise exception.
+ v.test_method()
+
+ def test_validate_glfd(self):
+
+ class _FakeFile(object):
+
+ def __init__(self, fd, path=None):
+ self.fd = fd
+
+ @utils.validate_glfd
+ def test_method(self):
+ return
+
+ def close(self):
+ self.fd = None
+
+ f = _FakeFile(1234)
+ f.close()
+ self.assertTrue(f.fd is None)
+ self.assertRaises(OSError, f.test_method)
+
+ f.fd = 1234
+ # Shouldn't raise exception.
+ f.test_method()