Skip to content

Conversation

@asger-semmle
Copy link
Contributor

res.end works like res.send since it actually forwards to NodeJS's response.end method.

An alternative way to implement this would be to treat Express response objects as a subtype of NodeJS's HTTP response objects, since those already recognize the end method. I'm not sure what the longterm benefits of that might be so I just took the easy way for now.

Running evaluation on express.slugs.

@asger-semmle asger-semmle added JS WIP This is a work-in-progress, do not merge yet! labels Feb 28, 2019
@asger-semmle asger-semmle requested a review from a team as a code owner February 28, 2019 12:42
ghost
ghost previously approved these changes Feb 28, 2019
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM.
(we could add a change note for this minor improvement)

I think we decided against using inheritance between NodeJS and Express way back.
The model of Connect explicitly use the inheritance, so I suspect we have a good reason not to use it for Express.

https://github.com/Semmle/ql/blob/edba24129d37ddd4fb490f416473bcb9a5bded39/javascript/ql/src/semmle/javascript/frameworks/Connect.qll#L89-L94

ghost
ghost previously approved these changes Feb 28, 2019
@asger-semmle
Copy link
Contributor Author

(we could add a change note for this minor improvement)

Added Express to the list of frameworks with improved support.

@asger-semmle
Copy link
Contributor Author

Evaluation looks good.

@asger-semmle asger-semmle removed the WIP This is a work-in-progress, do not merge yet! label Mar 1, 2019
@xiemaisi xiemaisi merged commit a6f3305 into github:master Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants