From c3e107284173784d79af39c87af88f65f2f9cb73 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Wed, 27 Dec 2023 14:05:22 +0300 Subject: [PATCH] added quote to urls, adjusted tests to cover this Signed-off-by: Alexander Piskun --- CHANGELOG.md | 1 + nc_py_api/files/files.py | 54 ++++++++++---------- tests/actual_tests/conftest.py | 3 ++ tests/actual_tests/files_test.py | 84 ++++++++++++++++++++++++-------- 4 files changed, 96 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d43dc11..c0dec7ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### Fixed - files: proper url encoding of special chars in `mkdir` and `delete` methods. #191 Thanks to @tobenary +- files: proper url encoding of special chars in all other `DAV` methods. #194 ## [0.7.1 - 2022-12-21] diff --git a/nc_py_api/files/files.py b/nc_py_api/files/files.py index 6fa76f09..1418ad42 100644 --- a/nc_py_api/files/files.py +++ b/nc_py_api/files/files.py @@ -86,7 +86,7 @@ def find(self, req: list, path: str | FsNode = "") -> list[FsNode]: def download(self, path: str | FsNode) -> bytes: """Downloads and returns the content of a file.""" path = path.user_path if isinstance(path, FsNode) else path - response = self._session.adapter_dav.get(dav_get_obj_path(self._session.user, path)) + response = self._session.adapter_dav.get(quote(dav_get_obj_path(self._session.user, path))) check_error(response, f"download: user={self._session.user}, path={path}") return response.content @@ -138,7 +138,7 @@ def upload(self, path: str | FsNode, content: bytes | str) -> FsNode: """ path = path.user_path if isinstance(path, FsNode) else path full_path = dav_get_obj_path(self._session.user, path) - response = self._session.adapter_dav.put(full_path, content=content) + response = self._session.adapter_dav.put(quote(full_path), content=content) check_error(response, f"upload: user={self._session.user}, path={path}, size={len(content)}") return FsNode(full_path.strip("/"), **etag_fileid_from_response(response)) @@ -219,11 +219,11 @@ def move(self, path_src: str | FsNode, path_dest: str | FsNode, overwrite=False) full_dest_path = dav_get_obj_path( self._session.user, path_dest.user_path if isinstance(path_dest, FsNode) else path_dest ) - dest = self._session.cfg.dav_endpoint + full_dest_path + dest = self._session.cfg.dav_endpoint + quote(full_dest_path) headers = Headers({"Destination": dest, "Overwrite": "T" if overwrite else "F"}, encoding="utf-8") response = self._session.adapter_dav.request( "MOVE", - dav_get_obj_path(self._session.user, path_src), + quote(dav_get_obj_path(self._session.user, path_src)), headers=headers, ) check_error(response, f"move: user={self._session.user}, src={path_src}, dest={dest}, {overwrite}") @@ -241,11 +241,11 @@ def copy(self, path_src: str | FsNode, path_dest: str | FsNode, overwrite=False) full_dest_path = dav_get_obj_path( self._session.user, path_dest.user_path if isinstance(path_dest, FsNode) else path_dest ) - dest = self._session.cfg.dav_endpoint + full_dest_path + dest = self._session.cfg.dav_endpoint + quote(full_dest_path) headers = Headers({"Destination": dest, "Overwrite": "T" if overwrite else "F"}, encoding="utf-8") response = self._session.adapter_dav.request( "COPY", - dav_get_obj_path(self._session.user, path_src), + quote(dav_get_obj_path(self._session.user, path_src)), headers=headers, ) check_error(response, f"copy: user={self._session.user}, src={path_src}, dest={dest}, {overwrite}") @@ -277,7 +277,7 @@ def setfav(self, path: str | FsNode, value: int | bool) -> None: path = path.user_path if isinstance(path, FsNode) else path root = build_setfav_req(value) webdav_response = self._session.adapter_dav.request( - "PROPPATCH", dav_get_obj_path(self._session.user, path), content=element_tree_as_str(root) + "PROPPATCH", quote(dav_get_obj_path(self._session.user, path)), content=element_tree_as_str(root) ) check_error(webdav_response, f"setfav: path={path}, value={value}") @@ -301,7 +301,7 @@ def trashbin_restore(self, path: str | FsNode) -> None: headers = Headers({"Destination": dest}, encoding="utf-8") response = self._session.adapter_dav.request( "MOVE", - f"/trashbin/{self._session.user}/{path}", + quote(f"/trashbin/{self._session.user}/{path}"), headers=headers, ) check_error(response, f"trashbin_restore: user={self._session.user}, src={path}, dest={dest}") @@ -313,7 +313,7 @@ def trashbin_delete(self, path: str | FsNode, not_fail=False) -> None: :param not_fail: if set to ``True`` and the object is not found, it does not raise an exception. """ path = path.user_path if isinstance(path, FsNode) else path - response = self._session.adapter_dav.delete(f"/trashbin/{self._session.user}/{path}") + response = self._session.adapter_dav.delete(quote(f"/trashbin/{self._session.user}/{path}")) if response.status_code == 404 and not_fail: return check_error(response) @@ -431,7 +431,7 @@ def _listdir( root, dav_path = build_listdir_req(user, path, properties, prop_type) webdav_response = self._session.adapter_dav.request( "PROPFIND", - dav_path, + quote(dav_path), content=element_tree_as_str(root), headers={"Depth": "infinity" if depth == -1 else str(depth)}, ) @@ -440,16 +440,17 @@ def _listdir( ) def __download2stream(self, path: str, fp, **kwargs) -> None: - with self._session.adapter_dav.stream("GET", dav_get_obj_path(self._session.user, path)) as response: + with self._session.adapter_dav.stream("GET", quote(dav_get_obj_path(self._session.user, path))) as response: check_error(response, f"download_stream: user={self._session.user}, path={path}") for data_chunk in response.iter_raw(chunk_size=kwargs.get("chunk_size", 5 * 1024 * 1024)): fp.write(data_chunk) def __upload_stream(self, path: str, fp, chunk_size: int) -> FsNode: - _dav_path = dav_get_obj_path(self._session.user, "nc-py-api-" + random_string(56), root_path="/uploads") + _tmp_path = "nc-py-api-" + random_string(56) + _dav_path = quote(dav_get_obj_path(self._session.user, _tmp_path, root_path="/uploads")) _v2 = bool(self._session.cfg.options.upload_chunk_v2 and chunk_size >= 5 * 1024 * 1024) full_path = dav_get_obj_path(self._session.user, path) - headers = Headers({"Destination": self._session.cfg.dav_endpoint + full_path}, encoding="utf-8") + headers = Headers({"Destination": self._session.cfg.dav_endpoint + quote(full_path)}, encoding="utf-8") if _v2: response = self._session.adapter_dav.request("MKCOL", _dav_path, headers=headers) else: @@ -548,7 +549,7 @@ async def find(self, req: list, path: str | FsNode = "") -> list[FsNode]: async def download(self, path: str | FsNode) -> bytes: """Downloads and returns the content of a file.""" path = path.user_path if isinstance(path, FsNode) else path - response = await self._session.adapter_dav.get(dav_get_obj_path(await self._session.user, path)) + response = await self._session.adapter_dav.get(quote(dav_get_obj_path(await self._session.user, path))) check_error(response, f"download: user={await self._session.user}, path={path}") return response.content @@ -602,7 +603,7 @@ async def upload(self, path: str | FsNode, content: bytes | str) -> FsNode: """ path = path.user_path if isinstance(path, FsNode) else path full_path = dav_get_obj_path(await self._session.user, path) - response = await self._session.adapter_dav.put(full_path, content=content) + response = await self._session.adapter_dav.put(quote(full_path), content=content) check_error(response, f"upload: user={await self._session.user}, path={path}, size={len(content)}") return FsNode(full_path.strip("/"), **etag_fileid_from_response(response)) @@ -683,11 +684,11 @@ async def move(self, path_src: str | FsNode, path_dest: str | FsNode, overwrite= full_dest_path = dav_get_obj_path( await self._session.user, path_dest.user_path if isinstance(path_dest, FsNode) else path_dest ) - dest = self._session.cfg.dav_endpoint + full_dest_path + dest = self._session.cfg.dav_endpoint + quote(full_dest_path) headers = Headers({"Destination": dest, "Overwrite": "T" if overwrite else "F"}, encoding="utf-8") response = await self._session.adapter_dav.request( "MOVE", - dav_get_obj_path(await self._session.user, path_src), + quote(dav_get_obj_path(await self._session.user, path_src)), headers=headers, ) check_error(response, f"move: user={await self._session.user}, src={path_src}, dest={dest}, {overwrite}") @@ -705,11 +706,11 @@ async def copy(self, path_src: str | FsNode, path_dest: str | FsNode, overwrite= full_dest_path = dav_get_obj_path( await self._session.user, path_dest.user_path if isinstance(path_dest, FsNode) else path_dest ) - dest = self._session.cfg.dav_endpoint + full_dest_path + dest = self._session.cfg.dav_endpoint + quote(full_dest_path) headers = Headers({"Destination": dest, "Overwrite": "T" if overwrite else "F"}, encoding="utf-8") response = await self._session.adapter_dav.request( "COPY", - dav_get_obj_path(await self._session.user, path_src), + quote(dav_get_obj_path(await self._session.user, path_src)), headers=headers, ) check_error(response, f"copy: user={await self._session.user}, src={path_src}, dest={dest}, {overwrite}") @@ -741,7 +742,7 @@ async def setfav(self, path: str | FsNode, value: int | bool) -> None: path = path.user_path if isinstance(path, FsNode) else path root = build_setfav_req(value) webdav_response = await self._session.adapter_dav.request( - "PROPPATCH", dav_get_obj_path(await self._session.user, path), content=element_tree_as_str(root) + "PROPPATCH", quote(dav_get_obj_path(await self._session.user, path)), content=element_tree_as_str(root) ) check_error(webdav_response, f"setfav: path={path}, value={value}") @@ -770,7 +771,7 @@ async def trashbin_restore(self, path: str | FsNode) -> None: headers = Headers({"Destination": dest}, encoding="utf-8") response = await self._session.adapter_dav.request( "MOVE", - f"/trashbin/{await self._session.user}/{path}", + quote(f"/trashbin/{await self._session.user}/{path}"), headers=headers, ) check_error(response, f"trashbin_restore: user={await self._session.user}, src={path}, dest={dest}") @@ -782,7 +783,7 @@ async def trashbin_delete(self, path: str | FsNode, not_fail=False) -> None: :param not_fail: if set to ``True`` and the object is not found, it does not raise an exception. """ path = path.user_path if isinstance(path, FsNode) else path - response = await self._session.adapter_dav.delete(f"/trashbin/{await self._session.user}/{path}") + response = await self._session.adapter_dav.delete(quote(f"/trashbin/{await self._session.user}/{path}")) if response.status_code == 404 and not_fail: return check_error(response) @@ -900,7 +901,7 @@ async def _listdir( root, dav_path = build_listdir_req(user, path, properties, prop_type) webdav_response = await self._session.adapter_dav.request( "PROPFIND", - dav_path, + quote(dav_path), content=element_tree_as_str(root), headers={"Depth": "infinity" if depth == -1 else str(depth)}, ) @@ -910,17 +911,18 @@ async def _listdir( async def __download2stream(self, path: str, fp, **kwargs) -> None: async with self._session.adapter_dav.stream( - "GET", dav_get_obj_path(await self._session.user, path) + "GET", quote(dav_get_obj_path(await self._session.user, path)) ) as response: check_error(response, f"download_stream: user={await self._session.user}, path={path}") async for data_chunk in response.aiter_raw(chunk_size=kwargs.get("chunk_size", 5 * 1024 * 1024)): fp.write(data_chunk) async def __upload_stream(self, path: str, fp, chunk_size: int) -> FsNode: - _dav_path = dav_get_obj_path(await self._session.user, "nc-py-api-" + random_string(56), root_path="/uploads") + _tmp_path = "nc-py-api-" + random_string(56) + _dav_path = quote(dav_get_obj_path(await self._session.user, _tmp_path, root_path="/uploads")) _v2 = bool(self._session.cfg.options.upload_chunk_v2 and chunk_size >= 5 * 1024 * 1024) full_path = dav_get_obj_path(await self._session.user, path) - headers = Headers({"Destination": self._session.cfg.dav_endpoint + full_path}, encoding="utf-8") + headers = Headers({"Destination": self._session.cfg.dav_endpoint + quote(full_path)}, encoding="utf-8") if _v2: response = await self._session.adapter_dav.request("MKCOL", _dav_path, headers=headers) else: diff --git a/tests/actual_tests/conftest.py b/tests/actual_tests/conftest.py index 2a8e2df3..5d57fdb5 100644 --- a/tests/actual_tests/conftest.py +++ b/tests/actual_tests/conftest.py @@ -39,6 +39,7 @@ def init_filesystem_for_user(nc_any, rand_bytes): /test_12345_text.txt /test_generated_image.png **Favorite** /test_dir_tmp + /test_###_dir """ clean_filesystem_for_user(nc_any) im = BytesIO() @@ -48,6 +49,7 @@ def init_filesystem_for_user(nc_any, rand_bytes): nc_any.files.makedirs("/test_dir/subdir") nc_any.files.mkdir("/test_dir/test_empty_child_dir/") nc_any.files.mkdir("/test_dir_tmp") + nc_any.files.mkdir("/test_###_dir") def init_folder(folder: str = ""): nc_any.files.upload(path.join(folder, "test_empty_text.txt"), content=b"") @@ -72,6 +74,7 @@ def clean_filesystem_for_user(nc_any): "test_64_bytes.bin", "test_12345_text.txt", "test_generated_image.png", + "test_###_dir", ] for i in clean_up_list: nc_any.files.delete(i, not_fail=True) diff --git a/tests/actual_tests/files_test.py b/tests/actual_tests/files_test.py index 1a25828e..9018e85a 100644 --- a/tests/actual_tests/files_test.py +++ b/tests/actual_tests/files_test.py @@ -107,6 +107,17 @@ async def test_list_empty_dir_async(anc_any): _test_list_empty_dir(result, await anc_any.user) +def test_list_spec_dir(nc_any): + r = nc_any.files.listdir("test_###_dir", exclude_self=False) + assert r[0].full_path.find("test_###_dir") != -1 + + +@pytest.mark.asyncio(scope="session") +async def test_list_spec_dir_async(anc_any): + r = await anc_any.files.listdir("test_###_dir", exclude_self=False) + assert r[0].full_path.find("test_###_dir") != -1 + + def test_list_dir_wrong_args(nc_any): with pytest.raises(ValueError): nc_any.files.listdir(depth=0, exclude_self=True) @@ -358,44 +369,55 @@ async def test_file_upload_chunked_async(anc, chunk_size): assert upload_crc == download_crc -def test_file_upload_file(nc_any): +@pytest.mark.parametrize("dest_path", ("test_dir_tmp/test_file_upload_file", "test_###_dir/test_file_upload_file")) +def test_file_upload_file(nc_any, dest_path): content = randbytes(113) with NamedTemporaryFile() as tmp_file: tmp_file.write(content) tmp_file.flush() - nc_any.files.upload_stream("test_dir_tmp/test_file_upload_file", tmp_file.name) - assert nc_any.files.download("test_dir_tmp/test_file_upload_file") == content + r = nc_any.files.upload_stream(dest_path, tmp_file.name) + assert nc_any.files.download(dest_path) == content + assert nc_any.files.by_id(r.file_id).full_path.find(dest_path) != -1 @pytest.mark.asyncio(scope="session") -async def test_file_upload_file_async(anc_any): +@pytest.mark.parametrize("dest_path", ("test_dir_tmp/test_file_upload_file", "test_###_dir/test_file_upload_file")) +async def test_file_upload_file_async(anc_any, dest_path): content = randbytes(113) with NamedTemporaryFile() as tmp_file: tmp_file.write(content) tmp_file.flush() - await anc_any.files.upload_stream("test_dir_tmp/test_file_upload_file_async", tmp_file.name) - assert await anc_any.files.download("test_dir_tmp/test_file_upload_file_async") == content + r = await anc_any.files.upload_stream(dest_path, tmp_file.name) + assert await anc_any.files.download(dest_path) == content + assert (await anc_any.files.by_id(r.file_id)).full_path.find(dest_path) != -1 -@pytest.mark.parametrize("dest_path", ("test_dir_tmp/upl_chunk_v2", "test_dir_tmp/upl_chunk_v2_ü")) +@pytest.mark.parametrize( + "dest_path", ("test_dir_tmp/upl_chunk_v2", "test_dir_tmp/upl_chunk_v2_ü", "test_dir_tmp/upl_chunk_v2_11###33") +) def test_file_upload_chunked_v2(nc_any, dest_path): with NamedTemporaryFile() as tmp_file: tmp_file.seek(7 * 1024 * 1024) tmp_file.write(b"\0") tmp_file.flush() - nc_any.files.upload_stream(dest_path, tmp_file.name) + r = nc_any.files.upload_stream(dest_path, tmp_file.name) assert len(nc_any.files.download(dest_path)) == 7 * 1024 * 1024 + 1 + assert nc_any.files.by_id(r.file_id).full_path.find(dest_path) != -1 @pytest.mark.asyncio(scope="session") -@pytest.mark.parametrize("dest_path", ("test_dir_tmp/upl_chunk_v2_async", "test_dir_tmp/upl_chunk_v2_ü_async")) +@pytest.mark.parametrize( + "dest_path", + ("test_dir_tmp/upl_chunk_v2_async", "test_dir_tmp/upl_chunk_v2_ü_async", "test_dir_tmp/upl_chunk_v2_11###33"), +) async def test_file_upload_chunked_v2_async(anc_any, dest_path): with NamedTemporaryFile() as tmp_file: tmp_file.seek(7 * 1024 * 1024) tmp_file.write(b"\0") tmp_file.flush() - await anc_any.files.upload_stream(dest_path, tmp_file.name) + r = await anc_any.files.upload_stream(dest_path, tmp_file.name) assert len(await anc_any.files.download(dest_path)) == 7 * 1024 * 1024 + 1 + assert (await anc_any.files.by_id(r.file_id)).full_path.find(dest_path) != -1 @pytest.mark.parametrize("file_name", ("test_file_upload_del", "test_file_upload_del/", "test_file_upload_del//")) @@ -527,7 +549,7 @@ def test_favorites(nc_any): favorites = nc_any.files.list_by_criteria(["favorite"]) favorites = [i for i in favorites if i.name != "test_generated_image.png"] assert not favorites - files = ("test_dir_tmp/fav1.txt", "test_dir_tmp/fav2.txt", "test_dir_tmp/fav3.txt") + files = ("test_dir_tmp/fav1.txt", "test_dir_tmp/fav2.txt", "test_dir_tmp/fav##3.txt") for n in files: nc_any.files.upload(n, content=n) nc_any.files.setfav(n, True) @@ -551,7 +573,7 @@ async def test_favorites_async(anc_any): favorites = await anc_any.files.list_by_criteria(["favorite"]) favorites = [i for i in favorites if i.name != "test_generated_image.png"] assert not favorites - files = ("test_dir_tmp/fav1.txt", "test_dir_tmp/fav2.txt", "test_dir_tmp/fav3.txt") + files = ("test_dir_tmp/fav1.txt", "test_dir_tmp/fav2.txt", "test_dir_tmp/fav##3.txt") for n in files: await anc_any.files.upload(n, content=n) await anc_any.files.setfav(n, True) @@ -566,7 +588,10 @@ async def test_favorites_async(anc_any): assert not favorites -@pytest.mark.parametrize("dest_path", ("test_dir_tmp/test_64_bytes.bin", "test_dir_tmp/test_64_bytes_ü.bin")) +@pytest.mark.parametrize( + "dest_path", + ("test_dir_tmp/test_64_bytes.bin", "test_dir_tmp/test_64_bytes_ü.bin", "test_###_dir/test_64_bytes_ü.bin"), +) def test_copy_file(nc_any, rand_bytes, dest_path): copied_file = nc_any.files.copy("test_64_bytes.bin", dest_path) assert copied_file.file_id @@ -582,7 +607,10 @@ def test_copy_file(nc_any, rand_bytes, dest_path): @pytest.mark.asyncio(scope="session") -@pytest.mark.parametrize("dest_path", ("test_dir_tmp/test_64_bytes.bin", "test_dir_tmp/test_64_bytes_ü.bin")) +@pytest.mark.parametrize( + "dest_path", + ("test_dir_tmp/test_64_bytes.bin", "test_dir_tmp/test_64_bytes_ü.bin", "test_###_dir/test_64_bytes_ü.bin"), +) async def test_copy_file_async(anc_any, rand_bytes, dest_path): copied_file = await anc_any.files.copy("test_64_bytes.bin", dest_path) assert copied_file.file_id @@ -597,7 +625,10 @@ async def test_copy_file_async(anc_any, rand_bytes, dest_path): await anc_any.files.delete(copied_file) -@pytest.mark.parametrize("dest_path", ("test_dir_tmp/dest move test file", "test_dir_tmp/dest move test file-ä")) +@pytest.mark.parametrize( + "dest_path", + ("test_dir_tmp/dest move test file", "test_dir_tmp/dest move test file-ä", "test_###_dir/dest move test file-ä"), +) def test_move_file(nc_any, dest_path): src = "test_dir_tmp/src move test file" content = b"content of the file" @@ -625,7 +656,10 @@ def test_move_file(nc_any, dest_path): @pytest.mark.asyncio(scope="session") -@pytest.mark.parametrize("dest_path", ("test_dir_tmp/dest move test file", "test_dir_tmp/dest move test file-ä")) +@pytest.mark.parametrize( + "dest_path", + ("test_dir_tmp/dest move test file", "test_dir_tmp/dest move test file-ä", "test_###_dir/dest move test file-ä"), +) async def test_move_file_async(anc_any, dest_path): src = "test_dir_tmp/src move test file" content = b"content of the file" @@ -921,7 +955,9 @@ def test_fs_node_last_modified_time(): assert fs_node.info.last_modified == datetime(2022, 4, 5, 1, 2, 3) -@pytest.mark.parametrize("file_path", ("test_dir_tmp/trashbin_test", "test_dir_tmp/trashbin_test-ä")) +@pytest.mark.parametrize( + "file_path", ("test_dir_tmp/trashbin_test", "test_dir_tmp/trashbin_test-ä", "test_dir_tmp/trashbin_test-1##3") +) def test_trashbin(nc_any, file_path): r = nc_any.files.trashbin_list() assert isinstance(r, list) @@ -969,7 +1005,9 @@ def test_trashbin(nc_any, file_path): @pytest.mark.asyncio(scope="session") -@pytest.mark.parametrize("file_path", ("test_dir_tmp/trashbin_test", "test_dir_tmp/trashbin_test-ä")) +@pytest.mark.parametrize( + "file_path", ("test_dir_tmp/trashbin_test", "test_dir_tmp/trashbin_test-ä", "test_dir_tmp/trashbin_test-1##3") +) async def test_trashbin_async(anc_any, file_path): r = await anc_any.files.trashbin_list() assert isinstance(r, list) @@ -1016,7 +1054,10 @@ async def test_trashbin_async(anc_any, file_path): assert not r -@pytest.mark.parametrize("dest_path", ("/test_dir_tmp/file_versions.txt", "/test_dir_tmp/file_versions-ä.txt")) +@pytest.mark.parametrize( + "dest_path", + ("/test_dir_tmp/file_versions.txt", "/test_dir_tmp/file_versions-ä.txt", "test_dir_tmp/file_versions-1##3"), +) def test_file_versions(nc_any, dest_path): if nc_any.check_capabilities("files.versioning"): pytest.skip("Need 'Versions' App to be enabled.") @@ -1036,7 +1077,10 @@ def test_file_versions(nc_any, dest_path): @pytest.mark.asyncio(scope="session") -@pytest.mark.parametrize("dest_path", ("/test_dir_tmp/file_versions.txt", "/test_dir_tmp/file_versions-ä.txt")) +@pytest.mark.parametrize( + "dest_path", + ("/test_dir_tmp/file_versions.txt", "/test_dir_tmp/file_versions-ä.txt", "test_dir_tmp/file_versions-1##3"), +) async def test_file_versions_async(anc_any, dest_path): if await anc_any.check_capabilities("files.versioning"): pytest.skip("Need 'Versions' App to be enabled.")