On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof@gmail.com> wrote:Sanlock allow up to 1023 characters in a disk path. We used to truncate
invalid long path silently instead of failing. Add failing tests for the
expected behaviour.
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
tests/constants.py | 4 +++
tests/python_test.py | 79 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+)
diff --git a/tests/constants.py b/tests/constants.py
index 4f45d13..2371fed 100644
--- a/tests/constants.py
+++ b/tests/constants.py
@@ -21,5 +21,9 @@ RINDEX_DISK_MAGIC = 0x01042018
# src/rindex_disk.h
# Copied from the docs module comment.
RINDEX_ENTRY_SIZE = 64
RINDEX_ENTRIES_SECTORS = 2000
+
+# src/sanlock.h
+
+SANLK_PATH_LEN = 1024
diff --git a/tests/python_test.py b/tests/python_test.py
index cd8a82f..5624f60 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -550,10 +550,21 @@ def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno():
sanlock.write_resource(b"ls_name", name, disks)
+@pytest.mark.xfail(reason="path truncated silently")
+def test_write_resource_path_length(no_sanlock_daemon):
+ with pytest.raises(ValueError):
+ path = "x" * constants.SANLK_PATH_LENWhat about moving the "path" initialization line 2 lines up?Here and in many more places where "path" local variable is initialized and then passed to a method call.I know that from the Java best practices perspective it's better to wrap only code that can throw an exception with the exception handling block.
+ sanlock.write_resource(b"ls_name", b"res_name", [(path, 0)])
+
+ with raises_sanlock_errno():
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ sanlock.write_resource(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
disks = [(path, 0)]
@@ -562,10 +573,21 @@ def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding
with raises_sanlock_errno():
sanlock.release(b"ls_name", name, disks)
+@pytest.mark.xfail(reason="path truncated silently")
+def test_release_resource_path_length(no_sanlock_daemon):
+ with pytest.raises(ValueError):
+ path = "x" * constants.SANLK_PATH_LEN
+ sanlock.release(b"ls_name", b"res_name", [(path, 0)])
+
+ with raises_sanlock_errno():
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ sanlock.release(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
disks = [(path, 0)]
@@ -574,10 +596,21 @@ def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, enco
with raises_sanlock_errno():
sanlock.read_resource_owners(b"ls_name", name, disks)
+@pytest.mark.xfail(reason="path truncated silently")
+def test_read_resource_owners_path_length(no_sanlock_daemon):
+ with pytest.raises(ValueError):
+ path = "x" * constants.SANLK_PATH_LEN
+ sanlock.read_resource_owners(b"ls_name", b"res_name", [(path, 0)])
+
+ with raises_sanlock_errno():
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ sanlock.read_resource_owners(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
def test_get_hosts_parse_args(no_sanlock_daemon, name):
with raises_sanlock_errno():
sanlock.get_hosts(name, 1)
@@ -624,10 +657,23 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno(errno.ENOENT):
sanlock.init_resource(b"ls_name", name, disks)
with raises_sanlock_errno(errno.ENOENT):
sanlock.init_resource(name, b"res_name", disks)
+
+@pytest.mark.xfail(reason="path truncated silently")
+def test_init_resource_path_length(no_sanlock_daemon):
+ with pytest.raises(ValueError):
+ path = "x" * constants.SANLK_PATH_LEN
+ sanlock.init_resource(b"ls_name", b"res_name", [(path, 0)])
+
+ # init_resource access storage directly.
+ with raises_sanlock_errno(errno.ENAMETOOLONG):
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ sanlock.init_resource(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno(errno.ENOENT):
sanlock.get_alignment(path)
@@ -643,10 +689,21 @@ def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno():
sanlock.read_resource(path)
+@pytest.mark.xfail(reason="path truncated silently")
+def test_read_resource_path_length(no_sanlock_daemon):
+ with pytest.raises(ValueError):
+ path = "x" * constants.SANLK_PATH_LEN
+ sanlock.read_resource(path)
+
+ with raises_sanlock_errno():
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ sanlock.read_resource(path)
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
disks = [(path, 0)]
@@ -656,10 +713,21 @@ def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno():
sanlock.request(name, b"res_name", disks)
+@pytest.mark.xfail(reason="path truncated silently")
+def test_request_path_length(no_sanlock_daemon):
+ with pytest.raises(ValueError):
+ path = "x" * constants.SANLK_PATH_LEN
+ sanlock.request(b"ls_name", b"res_name", [(path, 0)])
+
+ with raises_sanlock_errno():
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ sanlock.request(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
disks = [(path, 0)]
@@ -667,5 +735,16 @@ def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno():
sanlock.acquire(b"ls_name", name, disks, pid=os.getpid())
with raises_sanlock_errno():
sanlock.acquire(name, b"res_name", disks, pid=os.getpid())
+
+
+@pytest.mark.xfail(reason="path truncated silently")
+def test_acquire_path_length(no_sanlock_daemon):
+ with pytest.raises(ValueError):
+ path = "x" * constants.SANLK_PATH_LEN
+ sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid())
+
+ with raises_sanlock_errno():
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid())
--
2.17.2