-
Notifications
You must be signed in to change notification settings - Fork 1
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
BREAKING CHANGE: Support trust proxy and frameworks other than Express #2
base: master
Are you sure you want to change the base?
Conversation
Hey @cchan, wanna review this before I merge it? |
True! Give me a min |
Two comments, otherwise looks good! |
They're not showing up. Are they still in draft? |
@cchan could you check if your comments are still in draft? |
|
||
function middleware(options) { | ||
return function (req, res, next) { | ||
if (isCloudflare(trimIp(req.ip))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No 127.0.0.1 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! Good catch.
next(); | ||
}else{ | ||
res.end(); | ||
function trimIp(ip) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe trim4Mapped6?
Future would be nice to have proper ipv6 support.
Ooops fixed! Exposed that I haven't used github reviews before xD |
This adds convenient support for trust proxy (without depending on a deprecated package) and adds an exported
isMiddleware
function that can be used with other frameworks.We could avoid the major version bump by exporting
middleware
although I think it's just as well to call this 2.0.0 since trust proxy is the recommended method.I've also added a code formatter which formats both the code and the readme examples.