Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WebDAV: Fix PUT to overwrite file if already exists (v2) #107

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

WebDAV: Fix PUT to overwrite file if already exists (v2) #107

wants to merge 1 commit into from
+58 −8

Conversation

adrienverge
Copy link
Contributor

This is another implementation of my fix proposed at #92, with suggestions from @PixoDev. Instead of trashing + uploading, it replaces the file in place.

The WebDAV specification 1 states that in case of a PUT /…/file, if the resource /…/file already exists and is not a collection (= a directory), then it should be replaced in place (= overwritten).

This commit can be tested with a simple WebDAV client like curl:

# Write once:
curl -k -T my-file.txt https://webdav.local.internxt.com:3005
# Read:
curl -k https://webdav.local.internxt.com:3005/my-file.txt
# Overwrite (this should not fail):
curl -k -T my-file.txt https://webdav.local.internxt.com:3005

Note: I wrote this commit because otherwise the server from internxt webdav enable bugs with some WebDAV clients.

Footnotes

  1. http://www.webdav.org/specs/rfc4918.html#put-resources ↩

@adrienverge adrienverge mentioned this pull request Sep 14, 2024
Open
@adrienverge
Copy link
Contributor Author

Hello @PixoDev,

Thanks for you advice on #92 (comment)! I tried to follow it here:

  1. Check if the file exists in the Internxt API, you can check it by the UUID, you can check our existing API methods for Drive here https://github.com/internxt/sdk/blob/master/src/drive/storage/index.ts
  2. In case the file exists, encrypt and reupload the file, then replace the fileId with this https://github.com/internxt/sdk/blob/master/src/drive/storage/index.ts#L630

  1. I think I got what you mean and did things correctly, however I'm not 100% sure so your careful review would be welcome 🙏
    Please be tolerant, I did my best but it's hard because I don't have a test account of sandbox to test against, nor any documentation to read.
    Also, feel free to push edits to my PR if you want.

  2. When trying on an Internxt account, the 2nd step you proposed (storageClient.replaceFile()) fails with:

    Error: "<!DOCTYPE html>\n<html lang=\"en\">\n<head>\n<meta charset=\"utf-8\">\n<title>Error</title>\n</head>\n<body>\n<pre>Cannot PUT /api/files/40804a95-8155-47ab-a4d6-470c053da7b9</pre>\n</body>\n</html>\n"`.

    Any idea why? I don't know what type of back-end you have, but if it's S3-compatible, it should accept overwriting.

@adrienverge
WebDAV: Fix PUT to overwrite file if already exists (v2)
c5bbd26 
This is another implementation of my fix proposed at
internxt#92, with suggestions from @PixoDev.
Instead of trashing + uploading, it replaces the file in place.

The WebDAV specification [^1] states that in case of a `PUT /…/file`, if
the resource `/…/file` already exists and is not a collection (= a
directory), then it should be replaced in place (= overwritten).

This commit can be tested with a simple WebDAV client like `curl`:

    # Write once:
    curl -k -T my-file.txt https://webdav.local.internxt.com:3005
    # Read:
    curl -k https://webdav.local.internxt.com:3005/my-file.txt
    # Overwrite (this should not fail):
    curl -k -T my-file.txt https://webdav.local.internxt.com:3005

Note: I wrote this commit because otherwise the server from `internxt
webdav enable` bugs with some WebDAV clients.

[^1]: http://www.webdav.org/specs/rfc4918.html#put-resources
@adrienverge adrienverge force-pushed the feat/webdav-put-should-overwrite-existing-file-v2 branch from d7e27ce to c5bbd26 Compare September 30, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@adrienverge