[BR-2165]:feat/add multipart download for shared files - #2040
Conversation
|
| (downloadFile as any).mockImplementation( | ||
| async (fileId, bucketId, mnemonic, network, cryptoLib, bufferFrom, downloadCallback, decryptCallback) => { | ||
| await downloadCallback([{ url: downloadExample }]); | ||
| await decryptCallback('aes256ctr', sdkDerivedKey, iv, 2048); |
There was a problem hiding this comment.
Shouldn't this be called with the overrideKey?
There was a problem hiding this comment.
I dont think so, because that's the purpose of the test: to check if the derived key is overridden by the override key. If we changed that line to the override key, it would be checking the same key — overrideKey === overrideKey.
There was a problem hiding this comment.
You added the test because you added the key override to decryptStream. Why was override added to decryptStream?
There was a problem hiding this comment.
P.S. It's working, and server changes were already merged, but QA was postponed due to Mail & Mobile
|
This pull request has been inactive for 30 days. Is it still in progress? If so, please leave a comment or make an update to keep it open. Otherwise, it will be closed automatically in 15 days. |
Deploying drive-web with
|
| Latest commit: |
2471c96
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://84cb1927.drive-web.pages.dev |
| Branch Preview URL: | https://feat-add-multipart-download.drive-web.pages.dev |
|
| * Creates a NetworkFacade for the bridge. Pass auth for downloads of the user's own files; | ||
| * omit it for shared-link downloads, which authenticate through the share token instead. | ||
| */ | ||
| const createNetworkFacade = (auth?: { username: string; password: string }): NetworkFacade => |
There was a problem hiding this comment.
Maybe it's easier to remove the shared download from the network than create an empty auth
There was a problem hiding this comment.
That empty auth was already there before this PR tbh, downloadSharedFile on master did the same thing, I just moved it into createNetworkFacade. And we can't really avoid it, the SDK client requires those fields and for shared files the backend only gives us the key + token, no network creds. The token is what authorizes the download so the auth is ignored anyway.
| fileStream = decryptStream(encryptedContentStreams, key as Buffer, iv as Buffer, chunkStart); | ||
| fileStream = decryptStream( | ||
| encryptedContentStreams, | ||
| (options?.key || key) as Buffer, |
There was a problem hiding this comment.
why override with options.key? It wasn't there before
There was a problem hiding this comment.
As this change adds multipart download for shared files, this wasn't needed before, multipart was only used for own files, where the SDK derives the key from the mnemonic. Shared files don't have a mnemonic; the share link provides the key directly as encryptionKey, which we pass as options.key, so downloadChunk needs to honor it. It's the same options?.key || key logic the single-part download() already had. Own-file downloads never set options.key, so nothing changes for them.
I uploaded a 1,2GB video into a business workspace (not workspace owner, invited user), and video streaming was failing
|
4 participants
Description
Adds multipart download support to public shared file links.
Changes:
download.tsbut wasn't exposed)., otherwise keep the regular single-requestdownloadFile`.Related Issues
Related Pull Requests
Checklist
Testing Process
Manually verified in the browser: shared files above the threshold trigger multiple parallel
206 Partial Contentrange requests instead of one single request, and the resulting file downloads and decrypts correctly.Additional Notes