skshetry / webdav4

WebDAV client library with a fsspec based filesystem and a CLI.
https://skshetry.github.io/webdav4
MIT License
61 stars 17 forks source link

put_file raise 400 bad request when uploading to root folder #188

Open Astropilot opened 1 month ago

Astropilot commented 1 month ago

Dear skshetry,

Here is the context, we have in our company a nextcloud server and we are using shared folders feature to download/upload files in those folders. We are using the fsspec backend.

Here is a reproduction example:

from pathlib import Path
from webdav4.fsspec import WebdavFileSystem

manager = WebdavFileSystem(
    "https://[REDACTED]/public.php/webdav",
    auth=("SHARED_FOLDER_TOKEN", ""),
)
manager.put_file(Path("test.txt"), "test.txt")

The given exception:

Traceback (most recent call last):
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\client.py", line 365, in _request
    http_resp.raise_for_status()
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\httpx\_models.py", line 761, in raise_for_status
    raise HTTPStatusError(message, request=request, response=self)
httpx.HTTPStatusError: Client error '400 Bad Request' for url 'https://[REDACTED]/public.php/webdav/'
For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/400

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "C:\Users\marti\Documents\XXXX\Apps\test_ffspec\main.py", line 11, in <module>
    manager.put_file(Path("test.txt"), "test.txt")
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\fsspec.py", line 374, in put_file
    return self.upload_fileobj(
           ^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\fsspec.py", line 338, in upload_fileobj
    self.mkdirs(os.path.dirname(rpath), exist_ok=True)
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\fsspec\spec.py", line 1545, in mkdirs
    return self.makedirs(path, exist_ok=exist_ok)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\fsspec.py", line 259, in makedirs
    return self._mkdir(path, exist_ok=exist_ok)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\fsspec.py", line 229, in _mkdir
    return self.client.mkdir(path)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\client.py", line 454, in mkdir
    http_resp = self.with_retry(call)
                ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\func_utils.py", line 47, in wrapped_function
    return func()
           ^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\func_utils.py", line 70, in wrapped
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\client.py", line 376, in request
    http_resp = self._request(method, path, **kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\marti\AppData\Local\pypoetry\Cache\virtualenvs\test-ffspec-BQnAja9W-py3.12\Lib\site-packages\webdav4\client.py", line 367, in _request
    raise HTTPError(http_resp) from exc
webdav4.client.HTTPError: received 400 (Bad Request)

The issue is that the put_file is failed because it tries to create the folders for the path before. I didn't specify a path because I want it to be uploaded at the root, but the mkdirs is still called for an empty path resulting in Nextcloud rejecting the request, here is the content of the 400 response:

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:o="http://owncloud.org/ns">
  <s:exception>OCA\DAV\Connector\Sabre\Exception\InvalidPath</s:exception>
  <s:message>Empty filename is not allowed</s:message>
  <o:retry xmlns:o="o:">false</o:retry>
  <o:reason xmlns:o="o:">Empty filename is not allowed</o:reason>
</d:error>

Adding a leading slash does not resolve the issue either. When uploading to subfolders it is working properly, so it's seems to be specific for the root usecase.

When commenting the call to mkdirs the upload works fine:

    def upload_fileobj(  # noqa: PLR0913
        self,
        fobj: BinaryIO,
        rpath: str,
        callback: Optional["Callback"] = None,
        overwrite: bool = True,
        size: Optional[int] = None,
        **kwargs: Any,
    ) -> None:
        """Upload contents from the fileobj to the remote path."""
        rpath = self._strip_protocol(rpath)
-        self.mkdirs(os.path.dirname(rpath), exist_ok=True)
+        # self.mkdirs(os.path.dirname(rpath), exist_ok=True)

If fixing the mkdirs method to work with root folder is not possible, would it be correct to assume that trying to upload in the root folder should not call mkdirs as it should always exist? It would mean changing the method to be something like that:

    def upload_fileobj(  # noqa: PLR0913
        self,
        fobj: BinaryIO,
        rpath: str,
        callback: Optional["Callback"] = None,
        overwrite: bool = True,
        size: Optional[int] = None,
        **kwargs: Any,
    ) -> None:
        """Upload contents from the fileobj to the remote path."""
        rpath = self._strip_protocol(rpath)
        directory_name = os.path.dirname(rpath)
        if directory_name != "" and directory_name != "/":
            self.mkdirs(os.path.dirname(rpath), exist_ok=True)

I can make a PR if needed.

Kind regards, Astropilot