Use Webdav PUT for uploads #21237

PVince81 · 2015-12-16T16:40:20Z

Test cases

mention-bot · 2015-12-16T16:40:24Z

By analyzing the blame information on this pull request, we identified @butonic, @pdessauw and @schiesbn to be potential reviewers

MorrisJobke · 2015-12-16T21:59:43Z

apps/files/js/file-upload.js

@@ -518,7 +524,7 @@ OC.Upload = {
 				}
 			};

-			// initialize jquery fileupload (blueimp)
+			// initialize jquery fileupload (nlueimp)


really? 😉

PVince81 · 2015-12-17T17:20:34Z

apps/files/js/file-upload.js

+			}
+
+			parentPromise.then(function() {
+				console.log('createDirectory: ' + fullPath);


PVince81 · 2015-12-17T21:14:53Z

apps/dav/lib/connector/sabre/iframetransportplugin.php

+ * mostly used as a workaround for browsers that
+ * do not support PUT upload.
+ */
+class IFrameTransportPlugin extends \Sabre\DAV\ServerPlugin {


@DeepDiver1975 @evert is this acceptable or have a gone too far 😉

I wanted to replace the old ajax/upload.php and use Webdav PUT for uploads.
However old browsers like IE8/IE9 need an iframe transport and do a POST, so this plugin catches that and forwards the call to createFile / updateFile and checks preconditions.

Goal is to bring the code paths as close as possible.

What do you think ?

also @LukasReschke in case of security considerations. I added the CSRF check here, but we can probably move it to a generic plugin or the auth plugin.

Don't really see anything wrong with it. You could even take it one step further by creating a $subRequest / $subResponse and sending that to Server::invokeMethod as a more generic approach.

Ah! That's the kind of thing I was looking for 😄 Thanks!
Problem though is that I need to post-process the response to repackage it as JSON.
I'll have a look if that's possible.

PVince81 · 2015-12-18T12:28:12Z

Will do autorename using Webdav POST on the collection with suggested name, as per RFC 5995 http://greenbytes.de/tech/webdav/rfc5995.html (starting to like that one!)

PVince81 · 2015-12-18T13:06:03Z

And here we go, autorename implemented based on RFC 5995. So cool!

~~BUG: conflict dialog must appear at the end of all transfers~~ not a regression, something for another time (it behaved like this in OC7 too)

PVince81 · 2015-12-21T15:41:26Z

I'm still wondering whether it's ok to have all these extra PROPFIND after upload, or whether we should try and return all the needed info through headers...

PVince81 · 2015-12-21T15:44:22Z

Here are some ideas to remove the extra PROPFIND on after upload:

get fileid from headers
get etag from headers
mtime? if accepted => use sent one, else set to now
size? use one if browser knows it, else need to return as header
share/tags info => not really needed or can be computed
mimetype ? could be problematic, unless we return it too...
permissions: compute ?
mount type: compute ?

PVince81 · 2015-12-21T15:45:27Z

Would be even easier if we could just return a JSON blob (or the result of XML PROPFIND) as a result of the PUT request.
@evert would that violate any Webdav convention ?

evert · 2015-12-21T17:44:14Z

I don't think there's any problems with doing stuff with the response to a PUT request. It's a bit unusual

PVince81 · 2015-12-21T18:08:41Z

@evert so I guess it might be better to return information as headers instead ? That's what we party did so far: OC-FileId contains the file id and OC-Mtime: accepted tells whether the server accepted the mtime. We could add OC-Permissions, OC-Size.

Keeping the subsequent PROPFIND is so easy... but some people might complain about performance.

evert · 2015-12-21T20:47:38Z

Well, there's another idea.. you could add a specific header to PUT requests indicating that you want the server to return information about the resource... But yea headers would work as well.

PVince81 · 2015-12-22T09:38:14Z

BUG: IE8 requests basic auth when uploading to public page

PVince81 · 2015-12-23T11:38:48Z

IE8 will not be supported for 9.0, see #15567 (comment)

investigate whether iframe transport is needed for IE9+ and others like Safari => yes 😢

evert · 2015-12-23T14:18:09Z

🎉

PVince81 · 2016-01-11T11:01:02Z

Noooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo...

IE9 also requires the iframe transport.

PVince81 · 2016-08-31T08:20:12Z

Rebased. Some JS tests will fail, need to look into it.

PVince81 · 2016-08-31T08:22:33Z

The plan is as follows:

keep the chunking commit as it refactors stuff a bit further and makes it better, but chunking is disabled (one line can enable it)
fix those JS tests
retest all upload scenarios listed in the original post
motivate people to review this PR

- uses PUT method with jquery.fileupload for regular and public file lists - for IE and browsers that don't support it, use POST with iframe transport - implemented Sabre plugin to handle iframe transport and redirect the embedded PUT request to the proper handler - added RFC5995 POST to file collection with "add-member" property to make it possible to auto-rename conflicting file names - remove obsolete ajax/upload.php and obsolete ajax routes

Not needed any more in IE >= 11

Hacked around Blueimp's jquery.fileupload to make it work with our new chunking API.

PVince81 · 2016-09-03T16:33:43Z

Tested in Firefox, Chromium and IE11.

Ready for review @DeepDiver1975 @butonic @owncloud/qa

butonic · 2016-09-05T08:46:07Z

Really nice! 👍

That being said I cannot even login in IE8:

Webpage error details

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C)
Timestamp: Mon, 5 Sep 2016 08:42:47 UTC


Message: Object doesn't support this property or method
Line: 2
Char: 29759
Code: 0
URI: https://192.168.56.56/Repositories/oc/core/core/vendor/jquery/dist/jquery.min.js?v=14269cbd8477654a15c200a1df0a28b2


Message: 'jQuery' is undefined
Line: 2
Char: 1
Code: 0
URI: https://192.168.56.56/Repositories/oc/core/core/vendor/jquery-migrate/jquery-migrate.min.js?v=14269cbd8477654a15c200a1df0a28b2

I reverted to 8308348 to see if it was caused by the move to make, but I get the same errors there. I tried IE11 in IE8 emulation and the Win7 IE8 VM: https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/

@DeepDiver1975 @PVince81 @felixboehm de we still support IE8?

DeepDiver1975 · 2016-09-05T08:49:16Z

Ie8 support is long gone

DeepDiver1975 · 2016-09-05T08:50:11Z

That said - we should prevent users from logging in with unsupported browsers

butonic · 2016-09-05T09:51:26Z

Ok, IE9, uploading a 300mb video fails:

SCRIPT5007: Unable to get value of the property '0': object is null or undefined 
file-upload.js?v=14269cbd8477654a15c200a1df0a28b2, line 287 character 4

davitol · 2016-09-05T10:02:09Z

@butonic In oC 9.1 IE9 is no longer supported. Here are the supported web browsers for oC 9.1.

Web browser: IE11+ (except Compatibility Mode), Firefox 14+, Chrome 18+, Safari 5+

https://doc.owncloud.org/server/9.1/admin_manual/installation/system_requirements.html

butonic · 2016-09-05T10:50:42Z

I scanned the thread from the start and stumbled over IE8 and IE9 ... reading thoroughly would have helped: #21237 (comment)

sorry for the noise

so a 👍 from me

PVince81 · 2016-09-06T08:40:08Z

@butonic yeah, because when I started we still wanted to support IE8 and I had a workaround with an iframe transport plugin... then killed it again when we killed IE9.

Thanks for the reviews.

lock · 2019-08-04T20:02:08Z

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.

PVince81 added the 2 - Developing label Dec 16, 2015

PVince81 self-assigned this Dec 16, 2015

PVince81 added this to the 9.0-current milestone Dec 16, 2015

DeepDiver1975 added the in progress label Dec 16, 2015

PVince81 mentioned this pull request Dec 16, 2015

File upload in browser - use PUT if possible #4380

Closed

MorrisJobke reviewed Dec 16, 2015
View reviewed changes

PVince81 mentioned this pull request Dec 17, 2015

Fix files UI mtime parsing from webdav #21238

Merged

PVince81 force-pushed the files-ui-webdav-upload branch from 7054253 to a3ca184 Compare December 17, 2015 17:20

PVince81 reviewed Dec 17, 2015
View reviewed changes

apps/files/js/file-upload.js

}

parentPromise.then(function() {

console.log('createDirectory: ' + fullPath);

Copy link

Contributor Author

PVince81 Dec 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eeergh

PVince81 force-pushed the files-ui-webdav-upload branch from cb4467a to 9feacc4 Compare December 17, 2015 21:12

PVince81 reviewed Dec 17, 2015
View reviewed changes

PVince81 force-pushed the files-ui-webdav-upload branch from 96e9454 to ef903f4 Compare December 21, 2015 14:48

PVince81 force-pushed the files-ui-webdav-upload branch from ef903f4 to 5e88bef Compare December 21, 2015 18:07

PVince81 force-pushed the files-ui-webdav-upload branch 2 times, most recently from c78916c to b672770 Compare December 22, 2015 16:41

Vincent Petry added 6 commits September 3, 2016 18:24

JS utility function to compare paths

16268bd

Goodbye Iframe transport !

55e933b

Not needed any more in IE >= 11

Add support for chunked upload

36f49ca

Hacked around Blueimp's jquery.fileupload to make it work with our new chunking API.

Fix js unit tests for webdav put upload changes

1871f7b

More fixes to file upload

a89f4f0

PVince81 changed the title ~~[WIP] Use Webdav PUT for uploads~~ Use Webdav PUT for uploads Sep 3, 2016

PVince81 added 3 - To Review and removed 2 - Developing labels Sep 3, 2016

PVince81 force-pushed the files-ui-webdav-upload branch from 1d9bfac to a89f4f0 Compare September 3, 2016 16:34

PVince81 merged commit 3f99035 into master Sep 6, 2016

PVince81 deleted the files-ui-webdav-upload branch September 6, 2016 08:41

rullzer mentioned this pull request Sep 6, 2016

Use Webdav PUT for uploads nextcloud/server#1283

Merged

4 tasks

PVince81 mentioned this pull request Sep 8, 2016

Remove upload_limit in files app as it is not needed with PUT upload #26069

Merged

10 tasks

PVince81 mentioned this pull request Sep 20, 2016

Storage changes overview #12487

Closed

22 tasks

hodyroff mentioned this pull request Nov 25, 2016

File Modified Date is Changed When Using WebDav and File Manager #26718

Closed

korelstar mentioned this pull request Feb 11, 2017

[Work in Progress] 3rd-party API: ETag for faster synchronization nextcloud/notes#51

Closed

VicDeo mentioned this pull request May 10, 2017

Owncloud 10 - IE 11 destroys files during upload #27830

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Webdav PUT for uploads #21237

Use Webdav PUT for uploads #21237

PVince81 commented Dec 16, 2015 •

edited

Loading

mention-bot commented Dec 16, 2015

MorrisJobke Dec 16, 2015

PVince81 Dec 17, 2015

PVince81 Dec 17, 2015

PVince81 Dec 17, 2015

PVince81 Dec 17, 2015

evert Dec 17, 2015

PVince81 Dec 18, 2015

PVince81 commented Dec 18, 2015

PVince81 commented Dec 18, 2015

PVince81 commented Dec 21, 2015

PVince81 commented Dec 21, 2015

PVince81 commented Dec 21, 2015

evert commented Dec 21, 2015

PVince81 commented Dec 21, 2015

evert commented Dec 21, 2015

PVince81 commented Dec 22, 2015

PVince81 commented Dec 23, 2015

evert commented Dec 23, 2015

PVince81 commented Jan 11, 2016

PVince81 commented Aug 31, 2016

PVince81 commented Aug 31, 2016 •

edited

Loading

PVince81 commented Sep 3, 2016

butonic commented Sep 5, 2016 •

edited

Loading

DeepDiver1975 commented Sep 5, 2016

DeepDiver1975 commented Sep 5, 2016

butonic commented Sep 5, 2016

davitol commented Sep 5, 2016

butonic commented Sep 5, 2016

PVince81 commented Sep 6, 2016

lock bot commented Aug 4, 2019

Use Webdav PUT for uploads #21237

Use Webdav PUT for uploads #21237

Conversation

PVince81 commented Dec 16, 2015 • edited Loading

Test cases

mention-bot commented Dec 16, 2015

MorrisJobke Dec 16, 2015

Choose a reason for hiding this comment

PVince81 Dec 17, 2015

Choose a reason for hiding this comment

PVince81 Dec 17, 2015

Choose a reason for hiding this comment

PVince81 Dec 17, 2015

Choose a reason for hiding this comment

PVince81 Dec 17, 2015

Choose a reason for hiding this comment

evert Dec 17, 2015

Choose a reason for hiding this comment

PVince81 Dec 18, 2015

Choose a reason for hiding this comment

PVince81 commented Dec 18, 2015

PVince81 commented Dec 18, 2015

PVince81 commented Dec 21, 2015

PVince81 commented Dec 21, 2015

PVince81 commented Dec 21, 2015

evert commented Dec 21, 2015

PVince81 commented Dec 21, 2015

evert commented Dec 21, 2015

PVince81 commented Dec 22, 2015

PVince81 commented Dec 23, 2015

evert commented Dec 23, 2015

PVince81 commented Jan 11, 2016

PVince81 commented Aug 31, 2016

PVince81 commented Aug 31, 2016 • edited Loading

PVince81 commented Sep 3, 2016

butonic commented Sep 5, 2016 • edited Loading

DeepDiver1975 commented Sep 5, 2016

DeepDiver1975 commented Sep 5, 2016

butonic commented Sep 5, 2016

davitol commented Sep 5, 2016

butonic commented Sep 5, 2016

PVince81 commented Sep 6, 2016

lock bot commented Aug 4, 2019

PVince81 commented Dec 16, 2015 •

edited

Loading

PVince81 commented Aug 31, 2016 •

edited

Loading

butonic commented Sep 5, 2016 •

edited

Loading