New subject: [PATCH v2 7/8] python: Add failing tests for long disk path

17 Jun 2019

On Mon, Jun 17, 2019 at 5:30 PM Pavel Bar pbar@redhat.com wrote:
...
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_LEN




What 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.
It is indeed better to move path out of the pytest.raises() block. However
it is
unlikely that we will fail with ValueError in that line.
...
...

   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

    

Re: [PATCH v2 7/8] python: Add failing tests for long disk path

Signed-off-by: Nir Soffer nsoffer@redhat.com

pid=os.getpid())

Signed-off-by: Nir Soffer nsoffer@redhat.com

pid=os.getpid())