rooey / chromeos-filesystem-onedrive

This project provides a OneDrive FileSystem mount for ChromeOS
http://onedrivefs.justmichael.uk/
BSD 3-Clause "New" or "Revised" License
36 stars 23 forks source link

onedrive_client.js fixes for upload, download & create folder #52

Closed iamacarpet closed 4 years ago

iamacarpet commented 4 years ago
iamacarpet commented 4 years ago

Link to the Microsoft thread about the CORS problem still being a known bug:

https://github.com/microsoftgraph/microsoft-graph-docs-contrib/issues/8394

iamacarpet commented 4 years ago

Ok, so actually, that isn't uploads working for larger files.

It works on anything smaller than about 500KB (as before no uploads were working for me), but anything larger than that is still broken.

I did find the problem with the multi-part uploads done with createUploadSession though:

{
    "error": {
        "code": "invalidRequest",
        "message": "The Content-Range header is missing or malformed."
    }
}

From Microsoft's documentation:

Important: Your app must ensure the total file size specified in the Content-Range header is the same for all requests. If a byte range declares a different file size, the request will fail.

It looks like we were adding the Content-Range header, but the final bit at the end that is supposed to be the total length of the upload Microsoft should be expecting, isn't known by us until the very end (Chrome OS seems to only give us the length of the current data and the offset, not the total size to expect in the end), so we're currently just setting it to the length of the current chunk.

This causes uploads of any subsequent chunks to fail, as OneDrive already thinks it has received the full length (as instructed by us), so shuts off the upload session in the background.

Mozilla's documentation suggests for an unknown total length in a Content-Range header, you can specify Content-Range: <start>-<end>/*, however Microsoft's Graph API just reports this as invalid on the first chunk.

I can't see a way around this, apart from buffering the whole upload in an internal array (during the writeFile calls), then flushing the array out to OneDrive on closeFile, although obviously this wouldn't scale very well to large files, as I'm sure this was origionally intended to address.

Any ideas?

droz36 commented 4 years ago

Couldn't you read the entire file, counting the data length of each call, then just discarding the data and adding the callback data length in a var until the file is completed, then you would be able to determine your actual file length? The main drawback to this is that you have to transfer the file twice.

If its a local file from chrome os, this wouldn't be too bad. If you're pulling from somewhere on the web it could be more difficult

iamacarpet commented 4 years ago

To my knowledge, we don’t have access to the original file, know anything about it or even a file handle.

The file system provider from us is just exposing an “fopen”, “fwrite”, “fclose” style interface where we give the OS a file handle, it writes out a few byes at a time via the fwrite style call, then closes when complete - everything else is abstracted above us and we have no knowledge of source (of any kind), as Chrome OS itself is handling the copy operation.

iamacarpet commented 4 years ago

Taking a further look this morning, it looks like building up the file contents and submitting them at the end is exactly how v2-legacy used to do it, except rather than trying to store the file contents in a memory array as I'd considered, it stores it in a temporary filesystem, with window.requestFileSystem(window.TEMPORARY.

It looks like we'll have to revert back to doing the same thing, although we can still keep chunked uploads when closing the file and performing the upload - I'm guessing it'll just mean that the file transfer bar will be inaccurate.

iamacarpet commented 4 years ago

Just to have covered all of our bases, I spent an hour or so researching Javascript Streams (from NodeJS) and how to use them to pipe into a request body using the NodeJS request class.

My assumption was that if this could be ported to the browser for this extension, it may allow, using Golang style channels, for us to keep writing out gradually to a single upload channel (go back to simple uploads) at the same time as Chrome OS thinks it is being written, to solve the progress bar problem.

However, it doesn't look like this is really supported at all in Javascript, as hidden in the source of the request package, even it says you can't pipe any more data via a stream once the outbound request is started - so I guess they don't support any kind of stream concept natively, the data has to be fixed at the point the request is initiated.

Looks like uploading on fileClose really is our only option.

rooey commented 4 years ago

Taking a further look this morning, it looks like building up the file contents and submitting them at the end is exactly how v2-legacy used to do it, except rather than trying to store the file contents in a memory array as I'd considered, it stores it in a temporary filesystem, with window.requestFileSystem(window.TEMPORARY.

I had tried and failed at this previously but if you can see a way to make it work, then by all means give it a try.

iamacarpet commented 4 years ago

I managed to get the upload working using the simpleFileUpload function and it does work fairly well, with the big caveat that the Graph API only accepts uploads up to 4MB in size.

That probably would be enough for our needs, but I feel the need to try and implement some kind of failure in the code if the OS tries to write any more than that, before even attempting the upload.

This is sucking up a lot of time and to be honest, the staff using Chrome OS don't really seem that bothered, they are happy using the web version and introducing this might just confuse them further.