Fix EZP-25479: better support for nginx in front of varnish#93
Fix EZP-25479: better support for nginx in front of varnish#93gggeek wants to merge 3 commits intoezsystems:masterfrom
Conversation
gggeek
commented
Feb 17, 2016
gggeek
commented
|
If accepted, please port this to ezpublish-kernel as well :-) |
| // only accept the x-forwarded-for header if the remote-proxy is trusted | ||
| // also we add our ip to the list of forwarders, as it is logged by apache by default | ||
| if (req.http.x-forwarded-for && client.ip ~ proxies) { | ||
| // funnily enough, it seems that this bit is executed twice, so we do a bit of dirty coding |
There was a problem hiding this comment.
Any idea why this happens ?
There was a problem hiding this comment.
the 'adding happens twice' ? nopes :-(
but I tested it, and the behaviour was consistent - without the workaround either no ip added or added twice
There was a problem hiding this comment.
not sure - we do have ESI, but this was in the request for the main page.
Userhash at play?
There was a problem hiding this comment.
Userhash at play?
maybe, some way to log the url for debug?
There was a problem hiding this comment.
After reading the vcl a bit more, I think that this might happen because of the way the user-hash request is sent. If this is the case, we might improve the code by testing for "req.restarts" instead...
Further digging ongoing
you meant ezpublish-community ? |
| } | ||
| } else { | ||
| set req.http.X-Forwarded-For = client.ip; | ||
| set req.http.X-Forwarded-For = "" + client.ip + ", " + server.ip; |
There was a problem hiding this comment.
When you have varnish in front of eZ, if you don't do that you will receive only Varnish IP. That could be a shame if you track IP address (apache access log for example)
There was a problem hiding this comment.
The original code put only the client ip in the x-forwarder-for header.
I have noticed that, unlike this, Nginx by default (in its stock reverse-proxy config) adds itself as well to the ips in x-forwarded-for.
And, in the Apache logs, 'common' format, the 1st field will contain both ips.
Which is a bit weird at 1st, but makes it actually easy to see which requests has come through the proxy and which ones not.
With this change, the apache log will contain the whole list of ips: client, nginx, varnish.
Eg:
192.168.127.1, 172.18.0.7, 172.18.0.3 - - [17/Feb/2016:10:55:08 +0000] "GET /setup/info/php HTTP/1.1" 200 ...
There was a problem hiding this comment.
Ok, so we should make sure it is in sync with what Symfony expects on this header then. But at least clear for me, and with a comment mentioning this it will be clear to others ;)
…presence of a reverse proxy when sending debug headers
|
Cleaned up, ready for further review and merge. |
4 participants
