Skip to content

Trust proxy#1086

Merged
mcollina merged 14 commits intomasterfrom
trust-proxy
Aug 21, 2018
Merged

Trust proxy#1086
mcollina merged 14 commits intomasterfrom
trust-proxy

Conversation

@mcollina
Copy link
Copy Markdown
Member

Fixes #1055
Fixes #1044

Checklist

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

@mcollina mcollina requested review from allevo and delvedor August 21, 2018 18:45
@mcollina
Copy link
Copy Markdown
Member Author

This does not include any performance regression.

Comment thread fastify.js

req.id = genReqId(req)
handleTrustProxy(req)
req.hostname = req.hostname || req.headers['host']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should add ip and hostname directly to our request and reply object.
Maybe we can do that in the next branch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, definitely.

Copy link
Copy Markdown
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

LGTM

@delvedor delvedor added feature documentation Improvements or additions to documentation semver-minor Issue or PR that should land as semver minor labels Aug 21, 2018
@mcollina mcollina merged commit 8fbc27c into master Aug 21, 2018
@mcollina mcollina deleted the trust-proxy branch August 21, 2018 18:58
@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation Improvements or additions to documentation semver-minor Issue or PR that should land as semver minor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants