Spring-boot: Expose preferProxiedForAddress of Jetty NCSARequestLog in ServerProperties

[serac]

For applications behind an HTTP proxy, it is more correct to report the IP address of the X-Forwarded-For header in HTTP access logs instead of the socket remote address. Fortunately this behavior is easily enabled in Jetty by setting NCSARequestLog#setPreferProxiedForAddress(true). It would be helpful to expose this setter as a Spring property.

[philwebb]

Interesting, do you happen to know if the same kind of feature is available with Tomcat and Undertow?

[bclozel]

If we create a new property for that, we might run into a situation where developers would like the property server.jetty.accesslog.prefer-proxied-for-address to be deduced from server.use-forward-headers by default.

Maybe we don't really need a new property for that and instead apply it automatically when it makes sense.

[serac]

It's been a while since I used Tomcat, but it looks like the present strategy is unchanged from older versions: configure RemoteIpValve, which sets request attributes that will be picked up by AccessLogValve when requestAttributesEnabled=true. I'm unfamiliar with Undertow, but there is a promising component, https://github.com/undertow-io/undertow/blob/master/core/src/main/java/io/undertow/server/handlers/ProxyPeerAddressHandler.java.

That's to say it looks like it ought to be possible to use a more general property as @bclozel suggested to drive the configuration logic in the container-specific WebServerFactoryCustomizer classes. I missed server.use-forward-headers in my review; does it "do the right thing" for Tomcat and Undertow today?

[wilkinsona]

For Tomcat, I don't think there's any need to enable the use of request attributes. The Remote IP valve also sets the remote host and remote address on the request based on the X-Forwarded-For header. In other words, with the use of forward headers enabled, Tomcat's access logging will already do the right thing.

[wilkinsona]

For Undertow, enabling the use of forward headers will add ProxyPeerAddressHandler. It sets the source address on the HttpServerExchange based on the value of the X-Forwarded-For header. The exchange's source address is what's used for %h by the access log so Undertow will already do the right thing too.

[wilkinsona]

For Jetty, enabling the use of forward headers will add ForwardedRequestCustomizer. It sets the remote address on the request based on the value of the X-Forwarded-For header. The request's remote address is what's used by NCSARequestLog so Jetty will already do the right thing too.

[wilkinsona]

In summary, I don't think we need to make a change here. With server.use-forward-headers=true all three containers with access logging support will already do the right thing. If I've misinterpreted the behaviour of one of the containers please let us know, ideally providing a sample that shows things not working as I have described above.

[serac]

Just closing the loop here: the property you suggested, server.use-forward-headers=true, already provides for the capability I requested on this issue. I appreciate your taking the time to consider my request and kindly correct my ignorance.