Skip to content

[oc] Handle OC-Total-Length in new chunking - #7533

Merged
MorrisJobke merged 4 commits into
masterfrom
oc-28545-handle-oc-total-length-in-new-chunking
Jan 3, 2018
Merged

[oc] Handle OC-Total-Length in new chunking#7533
MorrisJobke merged 4 commits into
masterfrom
oc-28545-handle-oc-total-length-in-new-chunking

Conversation

@danxuliu

@danxuliudanxuliu commented Dec 15, 2017

Copy link
Copy Markdown
Member

Imported from owncloud/core#28545.

This pull request ensures that the OC-Total-Length header provided in the final MOVE operation of a chunked upload matches the length of the chunks in the server, aborting the upload if they do not match; it is similar to the check performed for each chunk, but for the whole file instead. Obviously this is not an integrity check, but something to quickly reject an upload if some chunks are missing.

Differences with the original pull request:

It can be tested by modifying the uploader code of the Web UI to send less bytes than expected (for example, by adding options.blob = options.blob.slice(0, options.blob.size - 1); to the beginning of _initXHRData.

Finally, this pull request should not conflict with the desktop, Android or iOS clients; OC-Total-Length is taken into account when provided in the MOVE operation, but if it is not given the verification is simply skipped.

DeepDiver1975and others added 4 commits December 15, 2017 14:46
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu
danxuliuforce-pushed the oc-28545-handle-oc-total-length-in-new-chunking branch from d7974ad to ec8bf53CompareDecember 15, 2017 13:48
@codecov

codecovBot commented Dec 15, 2017

Copy link
Copy Markdown

Codecov Report

Merging #7533 into master will increase coverage by <.01%.
The diff coverage is 85.36%.

@@ Coverage Diff @@## master #7533 +/- ##
============================================
+ Coverage 51.16% 51.17% +<.01% - Complexity 24866 24871 +5 
============================================
Files 1601 1602 +1 Lines 94684 94701 +17 Branches 1368 1368 ============================================
+ Hits 48448 48459 +11 - Misses 46236 46242 +6
Impacted FilesCoverage ΔComplexity Δ
apps/dav/lib/Connector/Sabre/File.php73.86% <ø> (-0.49%)101 <0> (-4)
apps/dav/composer/composer/autoload_static.php0% <ø> (ø)1 <0> (ø)⬇️
apps/dav/bin/chunkperf.php0% <0%> (ø)0 <0> (ø)⬇️
apps/dav/composer/composer/autoload_classmap.php0% <0%> (ø)0 <0> (ø)⬇️
apps/dav/lib/Connector/Sabre/FilesPlugin.php79.88% <100%> (-1.65%)48 <0> (-3)
apps/dav/lib/Server.php44.66% <100%> (+0.37%)17 <0> (ø)⬇️
apps/dav/lib/Connector/Sabre/Node.php72.17% <83.33%> (+0.61%)47 <4> (+4)⬆️
apps/dav/lib/Upload/ChunkingPlugin.php96.29% <96.29%> (ø)8 <8> (?)
apps/files_trashbin/lib/Expiration.php90.32% <0%> (-1.62%)29% <0%> (ø)

@rullzerrullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets do this!

@MorrisJobke

Copy link
Copy Markdown
Member

For me the chunkingPlugin beforeMove is never called by uploading from the web UI. Also changing the size does not change anything. :/

@danxuliu

Copy link
Copy Markdown
MemberAuthor

@MorrisJobke Strange... Was the file larger than 10MiB (otherwise the chunking is not used)?

@MorrisJobke

Copy link
Copy Markdown
Member

@MorrisJobke Strange... Was the file larger than 10MiB (otherwise the chunking is not used)?

Yes - but I will have another try, could be 10,5 MB and and 10,5 MiB.

@MorrisJobkeMorrisJobke mentioned this pull request Jan 2, 2018
30 tasks

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke
MorrisJobke merged commit 876238c into masterJan 3, 2018
@MorrisJobke
MorrisJobke deleted the oc-28545-handle-oc-total-length-in-new-chunking branch January 3, 2018 15:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsfeature: dav

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@danxuliu@MorrisJobke@rullzer@DeepDiver1975