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

android chunked upload: missing OC-Total-Length header #900

Closed
moscicki opened this issue Feb 26, 2015 · 12 comments
Closed

android chunked upload: missing OC-Total-Length header #900

moscicki opened this issue Feb 26, 2015 · 12 comments
Labels

Comments

@moscicki
Copy link

Android client does not respect the same protocol as the desktop client for chunked uploads.

For chunked PUT the client should send the OC-Total-Length header specifying the total file size (like the desktop sync client does). This gives a server a possibility, for example, to check the final file size as an integrity check. You may check with @dragotin and @DeepDiver1975 for details.

BWT, we started drafting the spec. It is not yet finalized: https://github.com/cernbox/smashbox/blob/master/protocol/protocol.md

You may also check if issues with failing 1MB+ uploads (e.g. #402) are not related.

@davivel
Copy link
Contributor

davivel commented Mar 16, 2015

Seen, Thanks a lot, seems promising.

@cdamken
Copy link

cdamken commented Mar 24, 2015

@MorrisJobke

00002767

@MorrisJobke
Copy link

@rperezb ;)

@rperezb
Copy link

rperezb commented Apr 8, 2015

Thanks for reporting this, we will take this into account

@davivel
Copy link
Contributor

davivel commented Apr 10, 2015

@DeepDiver1975 , @MorrisJobke , @dragotin : should this header be in all the chunks of a file or just in the first one?

Thanks.

@davivel
Copy link
Contributor

davivel commented Apr 10, 2015

Seems that desktop client sends it in every chunk, and also in non-chunked uploads; is it needed in non-chunked uploads too?

@moscicki
Copy link
Author

@davivel, @DeepDiver1975, @MorrisJobke, @dragotin:

I am attempting to make you all guys a favour and document your own protocol ;-)

Have a look here: https://github.com/cernbox/smashbox/blob/master/protocol/protocol.py

chunk_file_upload()
file_upload()

This is my best description of the protocol between the desktop sync client and the server, including the emulation of Android 900 issue (this one as we speak).

@davivel
Copy link
Contributor

davivel commented Apr 10, 2015

@moscicki , you are my hero XD

According to a comment in your own code, server OC 7 seems to ignore the header; that's a pity, maybe this fix does not have a so big impact on the tricky bugs in uploads that we expected.

@moscicki
Copy link
Author

Note that in this issue we are fixing the chunk_file_upload() for which this header is clearly missing. As for the plain upload I think you should include it. There should be no harm in doing this (this is my "sensible" guess).

Glad to be the hero! On the other hand, it would be appreciated to get some contribution to an effort, that someone like me who is not paid to do the work for your company does, in order to help you out with the basics such as documenting your own protocol and coding according to clearly documented protocol specification. I don't mean to be nasty, I am just sad about this.

The text description of this part is missing: https://github.com/cernbox/smashbox/blob/master/protocol/protocol.md

@masensio
Copy link

We've added the OC-Total-Length header in the PR owncloud/android-library#59

@masensio
Copy link

This change is also in our develop branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants