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

BREAKING CHANGE: Support trust proxy and frameworks other than Express #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paulmelnikow
Copy link
Collaborator

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.

@paulmelnikow paulmelnikow changed the title BREAKING CHANGE: Support trust proxy and other frameworks BREAKING CHANGE: Support trust proxy and frameworks other than Express Oct 15, 2020
@paulmelnikow
Copy link
Collaborator Author

Hey @cchan, wanna review this before I merge it?

@cchan
Copy link
Member

cchan commented Oct 23, 2020

True! Give me a min

@cchan
Copy link
Member

cchan commented Oct 23, 2020

Two comments, otherwise looks good!

@paulmelnikow
Copy link
Collaborator Author

They're not showing up. Are they still in draft?

@paulmelnikow
Copy link
Collaborator Author

@cchan could you check if your comments are still in draft?


function middleware(options) {
return function (req, res, next) {
if (isCloudflare(trimIp(req.ip))) {
Copy link
Member

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?

Copy link
Collaborator Author

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) {
Copy link
Member

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.

@cchan
Copy link
Member

cchan commented Oct 25, 2020

Ooops fixed! Exposed that I haven't used github reviews before xD

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.

2 participants