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.

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

Unexpected file names being saved causes data loss #7509

Closed
cyberduck opened this issue Oct 25, 2013 · 15 comments
Closed

Unexpected file names being saved causes data loss #7509

cyberduck opened this issue Oct 25, 2013 · 15 comments
Assignees
Labels
bug thirdparty Issue caused by third party webdav WebDAV Protocol Implementation
Milestone

Comments

@cyberduck
Copy link
Collaborator

a08a14e created the issue

I believe this issue to be affecting all OS platforms.

I use a WebDAV interface which is provided by SabreDAV in the forum software XenForo for editing XenForo templates.

If I create a template named webdav_test, I can find it in Cyberduck as webdav_test.html. I can then launch it in my external editor (PHP Storm 7). Cyberduck confirms the file has been downloaded. I then make an edit and Save. Cyberduck confirms the file has been uploaded.

What happens next is the template I am working on disappears.

It transpires that the error being thrown by XenForo is that the filename contains invalid characters. After bypassing that check, the error being thrown is the length of the filename is too long (over 50 characters).

After preventing these errors from stopping the save process, I can confirm the filename being saved is:

webdav_test9.html-5f13b83a-80eb-4cce-8b73-4fbbec7a96c1.html

The original file, webdav_test9.html has disappeared.

If I do not bypass the first two errors, actually nothing gets saved and the original file is deleted completely which unfortunately causes data loss.

I initially presumed this was due the new feature:

[Feature] Upload with temporary name when saving from external editor

However this is off by default and whether that is on or off, the result is the same.

It is worth noting that this has all been working as expected until 4.4 was released.

At risk of digressing...

I am currently finding it difficult to use either version 4.3.X or 4.4.X with PHP Storm 7 as an external editor.

I have it configured so that double clicking a file opens it in the external editor. It seems that doing so causes Cyberduck to download the file but PHP Storm doesn't open it. The issue seems worse in 4.3.X and slightly better in 4.4.X (if you forgive the data loss part! :-) )

@cyberduck
Copy link
Collaborator Author

@dkocher commented

Please post the transcript from the log drawer (⌘-L).

@cyberduck
Copy link
Collaborator Author

@dkocher commented

Replying to [7509 chris deeming]:

[Feature] Upload with temporary name when saving from external editor

However this is off by default and whether that is on or off, the result is the same.

The setting in Preferences for other uploads is not taken into account for edits. When editing files we always use the temporary upload name to avoid writing to a file that is currently being served from a web server for example. Also the point is actually to avoid data loss if the upload is interrupted.

@cyberduck
Copy link
Collaborator Author

@dkocher commented

We should have a test installation of sabredav to do some interoperability testing. If we can exclude this is a bug in Cyberduck but a failure moving the file in place with the temporary name we should file a bug with them.

@cyberduck
Copy link
Collaborator Author

@dkocher commented

I did a quick testing running SabreDAV on my local machine and could not reproduce the issue.

@cyberduck
Copy link
Collaborator Author

@dkocher commented

Please file a bug with XenForo. Make sure to include the transcript from the log drawer with the PUT and MOVE requests.

@cyberduck
Copy link
Collaborator Author

a08a14e commented

A little bit too quick to close this ticket. Quite disappointed. The only thing that has changed in the setup is Cyberduck 4.4. Essentially Cyberduck has introduced "a feature" or some other change that breaks backwards compatibility with other applications.

Here's the log drawers:

This is the output from 4.4 (not working):

GET /admindav.php/Public_Templates/Master%20Style%20%280%29/webdav_test13.html HTTP/1.1
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.4 (Mac OS X/10.9) (x86_64)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
HTTP/1.1 200 Ok
Date: Fri, 25 Oct 2013 19:40:56 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
ETag: c51ce410c124a10e0db5e4b97fc2af39
Content-Length: 2
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: text/html
PROPFIND /admindav.php/Public_Templates/Master%20Style%20%280%29/ HTTP/1.1
Depth: 1
Content-Type: text/xml; charset=utf-8
Content-Length: 99
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.4 (Mac OS X/10.9) (x86_64)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
HTTP/1.1 207 Multi-Status
Date: Fri, 25 Oct 2013 19:41:05 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Transfer-Encoding: chunked
Content-Type: application/xml; charset=utf-8
PROPFIND /admindav.php/Public_Templates/Master%20Style%20%280%29/ HTTP/1.1
Depth: 1
Content-Type: text/xml; charset=utf-8
Content-Length: 99
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.4 (Mac OS X/10.9) (x86_64)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
HTTP/1.1 207 Multi-Status
Date: Fri, 25 Oct 2013 19:41:06 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
Keep-Alive: timeout=5, max=99
Connection: Keep-Alive
Transfer-Encoding: chunked
Content-Type: application/xml; charset=utf-8
PUT /admindav.php/Public_Templates/Master%20Style%20%280%29/webdav_test13.html-b421701a-f176-4284-a052-dc431d76876c HTTP/1.1
Expect: 100-continue
Content-Length: 11
Content-Type: application/octet-stream
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.4 (Mac OS X/10.9) (x86_64)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
MOVE /admindav.php/Public_Templates/Master%20Style%20%280%29/webdav_test13.html-b421701a-f176-4284-a052-dc431d76876c HTTP/1.1
Destination: http://localhost/admindav.php/Public_Templates/Master%20Style%20%280%29/webdav_test13.html
Overwrite: T
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.4 (Mac OS X/10.9) (x86_64)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
HTTP/1.1 403 Forbidden
Date: Fri, 25 Oct 2013 19:41:07 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
Content-Length: 471
Keep-Alive: timeout=5, max=98
Connection: Keep-Alive
Content-Type: application/xml; charset=utf-8
HTTP/1.1 204 No Content
Date: Fri, 25 Oct 2013 19:41:07 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
Content-Length: 0
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: text/html

And this is the output from 4.3.1 (working):

PROPFIND /admindav.php/Public_Templates/Master%20Style%20%280%29/webdav_test14.html HTTP/1.1
Depth: 1
Content-Type: text/xml; charset=utf-8
Content-Length: 99
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.3.1 (Mac OS X/10.9) (i386)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
HTTP/1.1 207 Multi-Status
Date: Fri, 25 Oct 2013 19:44:47 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
Content-Length: 504
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: application/xml; charset=utf-8
GET /admindav.php/Public_Templates/Master%20Style%20%280%29/webdav_test14.html HTTP/1.1
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.3.1 (Mac OS X/10.9) (i386)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
HTTP/1.1 200 Ok
Date: Fri, 25 Oct 2013 19:44:47 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
ETag: aab3238922bcc25a6f606eb525ffdc56
Content-Length: 2
Keep-Alive: timeout=5, max=99
Connection: Keep-Alive
Content-Type: text/html
PUT /admindav.php/Public_Templates/Master%20Style%20%280%29/webdav_test14.html HTTP/1.1
Expect: 100-continue
Content-Length: 10
Content-Type: text/html
Host: localhost
Connection: Keep-Alive
User-Agent: Cyberduck/4.3.1 (Mac OS X/10.9) (i386)
Authorization: Basic Q2hyaXMgRGVlbWluZzpCaWdmb290MTIz
HTTP/1.1 200 Ok
Date: Fri, 25 Oct 2013 19:44:56 GMT
Server: Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10
X-Powered-By: PHP/5.4.10
Content-Length: 0
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: text/html
X-Pad: avoid browser bug

@cyberduck
Copy link
Collaborator Author

a08a14e commented

And exactly what do you expect XenForo to do about it?

You must consider that XenForo is a database driven application and therefore it has constraints on various things for many reasons. Specifically in this case, they don't allow anything other than a-z, A-Z, 0-9 _ or . in their template names, so the hyphenated string you append on the end causes an error. Finally, they don't allow template names longer than 50 characters. The title field in the xf_template table is VARBINARY(50).

It's unreasonable to expect them to change these as those constraints were included for a reason.

And despite myself bypassing those constraints by skipping the title validation in their code and increasing the field to VARBINARY(500), I instead get a template name that's garbage and the original template name is no longer there.

@cyberduck
Copy link
Collaborator Author

@dkocher commented

Replying to [comment:6 chris deeming]:

A little bit too quick to close this ticket. Quite disappointed.

No offense. We try to make a triage quickly with tickets to avoid piles of unresolved issues.

@cyberduck
Copy link
Collaborator Author

@dkocher commented

Replying to [comment:6 chris deeming]:

A little bit too quick to close this ticket. Quite disappointed. The only thing that has changed in the setup is Cyberduck 4.4. Essentially Cyberduck has introduced "a feature" or some other change that breaks backwards compatibility with other applications.

That is true only if the feature introduced would not be standard (WebDAV RFC) compliant.

@cyberduck
Copy link
Collaborator Author

@dkocher commented

Replying to [7509 chris deeming]:

It transpires that the error being thrown by XenForo is that the filename contains invalid characters. After bypassing that check, the error being thrown is the length of the filename is too long (over 50 characters).

I can't see this in the transcript posted. Do you have a screenshot of this error response?

@cyberduck
Copy link
Collaborator Author

@dkocher commented

Replying to [comment:7 chris deeming]:

And exactly what do you expect XenForo to do about it?

You must consider that XenForo is a database driven application and therefore it has constraints on various things for many reasons. Specifically in this case, they don't allow anything other than a-z, A-Z, 0-9 _ or . in their template names, so the hyphenated string you append on the end causes an error. Finally, they don't allow template names longer than 50 characters. The title field in the xf_template table is VARBINARY(50).

It's unreasonable to expect them to change these as those constraints were included for a reason.

And despite myself bypassing those constraints by skipping the title validation in their code and increasing the field to VARBINARY(500), I instead get a template name that's garbage and the original template name is no longer there.

It is not feasible to impose such internal design limitations as the WebDAV protocol does not have a restriction of the URI length (RFC 2616).

@cyberduck
Copy link
Collaborator Author

@dkocher commented

As a temporary workaround, disable the use of the temporary filename option using the hidden setting

defaults write ch.sudo.cyberduck editor.upload.temporary false

@cyberduck
Copy link
Collaborator Author

a08a14e commented

Ok. When you put it like that, I guess the change is reasonable.

No offense taken at the closing of the ticket. It's just all too often people out there like to get tickets out of the door with the minimum amount of work possible, but I see that isn't happening here.

Thank you for the workaround. I will try it and report back.

Can I make a simple change request that perhaps solves the problem satisfactorily for both parties? Could the hidden setting above be made a configurable option in Preferences? I have to say, I've been using Cyberduck extensively and it works perfectly. Before the temporary filename feature, I have never once experienced any issues relating to saving files from an external editor.

I can understand the reasons for this feature, though, and would be happy for it to be on by default, as long as I can understand the risks and turn it off.

I have asked XenForo if they will consider any fixes, but this is unlikely. As much as their template system do not adhere to WebDAV standards, it isn't feasible, necessarily, to expect them to. Primarily templates are edited directly from the Admin CP. WebDAV is a functionality for power users, but it was by no means written solely for WebDAV compatibility.

But that helps neither of us :)

I will try that workaround now, thank you so much for taking the time to assist further with the issue.

@cyberduck
Copy link
Collaborator Author

a08a14e commented

The temporary workaround works great, thank you.

@cyberduck
Copy link
Collaborator Author

@dkocher commented

To summarize.

  • although we send a expectation request for the PUT with Expect: 100-continue header, the server replies with a 100 status code to continue instead of forbidding the upload because it does not meet its internal filename length requirement.
  • The MOVE command fails with a forbidden response but still the original file seems to get overridden as reported by the user. This is clearly another bug in the server implementation.

@iterate-ch iterate-ch locked as resolved and limited conversation to collaborators Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug thirdparty Issue caused by third party webdav WebDAV Protocol Implementation
Projects
None yet
Development

No branches or pull requests

2 participants