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

refactor: use res.json or sendStatus for objects or empty body #424

Merged
merged 4 commits into from
Oct 7, 2020

Conversation

mantariksh
Copy link
Contributor

Problem

Usage of res.send creates the risk of HTML injection attacks if not used with care. Using res.json is better practice.

Solution

This PR addresses the cases where changing from res.status to res.json or res.sendStatus should be risk-free.

When res.send is called with an object, it behaves the same way as if res.json is called, since Express just calls res.json internally. Moreover, when res.send is called with no argument, it is (almost) equivalent to calling res.sendStatus, since the client should not be doing anything with the empty response body. This PR addresses these two cases throughout the codebase by changing res.send to res.json or res.sendStatus as necessary.

@mantariksh mantariksh requested a review from tshuli October 7, 2020 09:22
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

lgtm!

@liangyuanruo liangyuanruo merged commit f4a771f into develop Oct 7, 2020
@liangyuanruo liangyuanruo deleted the refactor/res-json branch October 7, 2020 09:31
@karrui karrui mentioned this pull request Oct 8, 2020
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.

None yet

2 participants