Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add root support in res.download() #4855

Merged
merged 1 commit into from
Mar 25, 2022

Conversation

mmito
Copy link
Contributor

@mmito mmito commented Mar 14, 2022

This pull request addresses the abnormal root behaviour in res.download() in #4834. It provides the non-breaking change to the code suggested in the linked issue (thus resolving the filename to an absolute path only if options.root is not set).

I have:

  • added a test covering the change (as res.download() relies on res.sendFile(), all of the various cases with root in options get dealt with in the sendFile tests).
  • run linter.

@dougwilson dougwilson added this to the 4.18 milestone Mar 25, 2022
@dougwilson dougwilson force-pushed the add-root-support-response-download branch from 3b13cae to 0f9c624 Compare March 25, 2022 02:32
@dougwilson dougwilson mentioned this pull request Mar 25, 2022
20 tasks
@dougwilson dougwilson changed the base branch from master to 4.18 March 25, 2022 04:07
@dougwilson dougwilson force-pushed the add-root-support-response-download branch from 0f9c624 to 0def9bb Compare March 25, 2022 04:09
Copy link
Contributor

@dougwilson dougwilson left a comment

Choose a reason for hiding this comment

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

Thanks so much!!

@dougwilson dougwilson merged commit 0def9bb into expressjs:4.18 Mar 25, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants