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

Upgrade to Express 4 #1081

Closed
gvilarino opened this issue Oct 19, 2015 · 4 comments
Closed

Upgrade to Express 4 #1081

gvilarino opened this issue Oct 19, 2015 · 4 comments

Comments

@gvilarino
Copy link
Member

Or higher. More precisely higher than 4.13.3, as we can see in david eariler versions have security issues.

For doing this, I'd advice reading this article on how to move to Express 4 from earlier versions, or more specificaly: migrating to Express 4 from Express 3

Alternatively, we can go to the latest 3.y.z version, since there are security patches there as well, and upgrading to 4 will imply redoing part of our boot module, since many middlewares are no longer bundled with express itself (although it shouldn't be too much of a hassle).

@sachalifs
Copy link
Contributor

👍

we should test all https features as well with this change

@gvilarino
Copy link
Member Author

Agree. One thig to keep in mind is the following:

Our https-forcing mechanism relies on express-sslify which, in turn, some consirations to keep in mind.

  1. Express 4 will not work properly with redirection since express-sslify uses req.host instead of req.hostname to determine the URL for redirection and the url core module we use [uses on hostname as the property name]https://github.com/DemocracyOS/app/blob/1.0.0/lib/utils/index.js#L271). Double-check that before closing this issue.
  2. There is an open pull request that -if merged- will change express-sslify's API (i.e.: it will remove the boolean trap. Keep this in mind if bumping the dep; our lib/server-factory/ssl-redirect.js should be updated as well.

@gvilarino
Copy link
Member Author

The aforementioned pull request is now merged, so we should keep an eye out for a newer release of express-sslify that will break our current usage.

@mjlescano mjlescano assigned mjlescano and unassigned mjlescano Oct 26, 2015
@sachalifs
Copy link
Contributor

We'll upgrade on 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants