TAP5-2832: LocalhostOnly detect IPv6 correctly

[benweidig]

LocalhostOnly didn't detect a few of the IPv6 variants.
Furthermore, it might trigger a DNS lookup, which could impact performance.
Therefore, it now relies on HttpServletRequest.getRemoteAddr (via Request).

I've decided to extend the Request interfrace with getRemoteAddr instead of trying to provide LocalhostOnly with the HttpServletRequest somehow.

My reasoning is that Request is already a more high-level/convenient wrapper for HttpServletRequest, delegating multiple methods already to the underlying HttpServletRequest.
Adding another one makes sense, as anything relying on LocalhostOnly would need to provide the full/correct environment. This was an issue during initial testing with using RequestGlobalsinLocalhostOnly`, as it became an invisible dependency.

By extending Request and the related TestableRequest and DelegatingRequest, the Request-based code will behave correctly, regardless of how it's used.

[thiagohp]

For backward compatibility purposes, couldn't we make this a default method that returns null? Or maybe return type Optional and return Optional.empty() by default? While we cannot change the existing Tapestry methods that may return null to return Optional instead, I believe we should try to use them for new methods we introduce in Java interfaces.

[benweidig]

In general, I totally agree and get where you're coming from, but I would argue that Request is a "consume-only" interface with little reason to implement yourself.

For example, if MarkupRendererFilter would be changed, a lot of code would break.
But with Request, I don't see much risk.
If someone implements it, they are doing some special stuff, most likely in a library, and then needing to adapt to such a change seems reasonable to me, or not, as the change is binary-compatible.

However, if you want me to include a default method anyway, I would stay with String/null, as it's not an optional value according to spec, even though it's not guaranteed by all environments.

Lately, I try to avoid Optional, as it does not provide much utility if it only represents a singular value that is directly consumed after and not chained with multiple calls, and simple null-check is cheaper.

[thiagohp]

Yes, I'd like to have the new method to be default.