fix #976 Support for "X-Forwarded-Proto" headers Multiple Values

[devsonic]

#976

changes:

• Added splitting for "x-forwarded-proto" multiple values.
• Added test codes.

[violetagg]

@devsonic Can you elaborate more why you decided to take into consideration the first entry from the comma separated header value?

[devsonic]

@violetagg

As far as i know, reactor-netty use 'ConnectorInfo.java' to write requested client connection infos.
In it, "x-forwarded-proto" is used to make 'scheme' and "x-forwded-host" is used to make 'host address'. and then finally these properties would create base url to call api. (spring reference)

In the reactor-netty code, "x-forwarded-host" was supposed to get the first element, so i decided to follow rule. (Similarly, 'proto' of forwarded specification also gets the first value.)

reactor-netty/src/main/java/reactor/netty/http/server/ConnectionInfo.java

Line 166 in fbd1c30

hostHeader.split(",", 2)[0].trim(), port);

Interestingly, other project(tornade) handled it differently.
they decided to use last entry of the 'x-forwarded-proto'. (refer to link)
they seem to think last entry would be more reliable.

I think it can be different for different server frameworks.
And also not all proxies set this header or support the multi value format.
the main reason is that it is de-factor.

if you have any opinion, please let me know. thank you.

[violetagg]

@devsonic What do you think if we expose a new API?

reactor.netty.http.server.HttpServer#forwarded(boolean, Function<?, ConnectionInfo>)

so if you just use reactor.netty.http.server.HttpServer#forwarded(boolean) it will work as it is now (or it will take the first in order to be consistent with the rest),
if you use the new API, you will receive the request, etc. and as a result you will return ConnectionInfo

[devsonic]

@violetagg
good idea!

but i have slight worries about that.
when using a new api, 'ConnectionInfo' requires too much information to create.
and i just need to focus on how to implement the forwarded information.
so, it might be better to just add a generating rule for forwarded connection, rather than create a whole thing.
otherwise i think it could be better to make ConnectionInfo into abstract class, and then you would override the methods that add forward rules.

reactor.netty.http.server.HttpServer#forwardedConnection(Function<?, ForwardedConnection>)
 // it will work if 'forwarded' options is true

and as you mentioned,
in current api, It's a good idea to use the first entry just like PR in order to be consistent with the rest.

[violetagg]

@devsonic I'll be happy to review your idea when it is ready
👍

[violetagg]

@devsonic should I/you create a feature request for this

reactor.netty.http.server.HttpServer#forwardedConnection(Function<?, ForwardedConnection>) // it will work if 'forwarded' options is true

or we will cover it with this PR?

[devsonic]

@violetagg
would you please create the feature request for this?
then i will follow up the issue you created.

[violetagg]

@devsonic thanks for the contribution
here is the new feature request #990