-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Overwriting populated folders using web ui leads to errors and unexpected behaviour. #28844
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
Comments
I think this never worked properly... |
@SergioBertolinSG not reproducible on master 7c7e4f9 It is likely that some other of my related PRs fixed it. Can you verify ? |
Testing in current master, the step 2 simply doesn't happen. When uploading again the folder, it is not detected as an overwrite. |
In my test it worked. I dragged a folder from dolphin (file manager) into the current folder. Did you drag onto a subfolder maybe ? |
oh, so you too see "infinite years"... |
Works for me, see above. Note that the 412 errors are expected, it's the conflict detection that will trigger the conflict dialog. |
I get 405s |
In your gif some 405s appear as well. |
Yes, these are expected too: we try to create the folder and the server tells us that it already exists with 405, so we skip it. This saves a server call, because the alternative would be to do two requests: one PROPFIND and then if 404 comes, do a MKCOL, which might also fail if someone created the folder between the two calls. This is Webdav. |
@SergioBertolinSG can you clear the cache and try again ? |
Still happening. |
Ok, I was able to reproduce it with @SergioBertolinSG's |
There is something weird happening: the first upload goes through and the But the second upload, while it goes through Now to find out why this is happening... Maybe some cached stuff from the previous upload |
Ok, looks like there are zombie past uploads. I checked the state inside of Normally when creating a new upload, the upload object simply receives the new data. |
and the weirdest part, it always only happens with @SergioBertolinSG's folder structure:
but doesn't happen with mine:
|
Maybe the hidden files? |
It happens also in 10.0.2 so not a regression. |
I tried removing the |
|
|
I have the feeling there's a race condition somewhere in the upload code. Sometimes I receive different errors. The weird thing is that sometimes the "OC.Upload" object arrives uninitialized in the "finish" callback even though it was initialized before and even run the query. The fact that it's uninitialize/unpopulated causes JS errors or deviations from the expected code path. Since this is random, I'll try adding a lot of log statements to be able to trace the exact code paths and states. |
I can't seem to reproduce the issue after renaming ".DS_Store" to "DS_Store" (no dot). Weird... And the errors, when they happen, are random. It fails in different locations upon different trials. |
Okay, seems I found something: it seems that jquery.fileupload (in |
We get this fixed for 10.0.4 ? |
Yes, still on my list |
If I remove the copy logic and have it just a reference, it's even worse: the internal events are triggered in a cyclic manner |
Okay, after digging into the code I found the following: the upload id that is generated doesn't properly include the target upload folder but just the name. So if you're trying to upload multiple files with the same name but into different folders, it would cause an upload id conflict and mess up when all is done in parallel. |
Seems the upload id is not the problem here as there was an appended timestamp. I'll still provide a fix for this as high concurrency could cause upload id collisions if the timestamp is exactly the same. There is another problem remaining: at the time of the |
This is very random. I have the feeling that sometimes one piece of the upload batch finishes earlier and calls The weird thing is that when I add logging to try and reproduce this, it doesn't happen. |
Ok, I managed to observe this race condition with logging. Sometimes it happens that the first items of the transfer finish early and fast, so that there are no more "submitted" items in jquery.fileupload's queue, so it fires "fileuploadstop" which marks the end of the transfer. "fileuploadstop" would simply clear the list of known uploads. However we still have a list of unprocessed uploads, so I've adjusted the PR incoming soon. |
Ah, I also wanted to mention that I noticed this behavior when reuploading the same folder within the same page. It almost feels like Chrome is caching the files somewhere so the second upload is much faster, but also more prone to trigger this race condition. This happens even if you upload into a second separate folder. This likely matches somehow what @SergioBertolinSG observed, except that he reuploaded into the same folder while I uploaded to a second folder. |
@SergioBertolinSG please retest with #29393 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Steps to reproduce
Expected behaviour
The upload detects that it is an overwrite and asks if the folder should upload again the files.
Showing the conflicts.
Actual behaviour
The behaviour is erratic, sometimes the upload is just ignored, other it warns you with an upload cancelled warning. And others it simply fails this way:
Server configuration
Operating system:
Ubuntu 16.04
Web server:
Apache
Database:
MySQL
PHP version:
7.0
Cache
DB
ownCloud version: (see ownCloud admin page)
(master)
{"installed":"true","maintenance":"false","needsDbUpgrade":"false","version":"10.0.3.0","versionstring":"10.0.3 beta","edition":"Community","productname":"ownCloud"}
Patch 28692 was applied while testing this, no changes.
Updated from an older ownCloud or fresh install:
Fresh
The content of config/config.php:
Are you using external storage, if yes which one: local/smb/sftp/...
No.
Are you using encryption:
No.
Logs
Client configuration
Browser
Chrome, Firefox
The text was updated successfully, but these errors were encountered: