Skip to content

[IMPROVE] Add support to range downloads on file system storage - #21463

Merged
sampaiodiego merged 4 commits into
developfrom
fix-file-system-uploads-ranges
Apr 7, 2021
Merged

sampaiodiego merged 4 commits into
developfrom
fix-file-system-uploads-ranges

Conversation

@sampaiodiego

Copy link
Copy Markdown
Member

Proposed changes (including videos or screenshots)

Issue(s)

Closes #20203
Related to RocketChat/Rocket.Chat.ReactNative#2759

Steps to test or reproduce

Further comments

@sampaiodiego sampaiodiego added this to the 3.14.0 milestone Apr 6, 2021
const range = getFileRange(file, req);
setRangeHeaders(range, file, res);

if (range) {

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.

The setRangeHeaders takes into consideration if range is out of bounds, but this condition does not. Maybe changing to range && !range.outOfRange?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

right.. I'll change to look more how GridFS does.. 👍

res.setHeader('Content-Length', file.size);

this.store.getReadStream(file._id, file).pipe(res);
if (!options.hasOwnProperty('start')) {

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.

This may fail if { start: undefined } (cause the property is there, but with an invalid value).

Comment thread app/file-upload/server/lib/ranges.js
@RocketChat RocketChat deleted a comment from lgtm-com Bot Apr 6, 2021
Comment thread app/file-upload/server/config/FileSystem.js
Comment thread app/file-upload/server/lib/ranges.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants