Tickets you have mentioned and
forwarded-for-5807 branch are mostly about parsing X-Forwarded-For in order to obtain correct client IP. While it is valuable task, it is not what strikes me right now.
I'm now more concerned with an absence of API for getting user-visible server's name, not client's ip.
Look, I'm currently porting my app from Django to Klein and noticed strange behavior of Klein. For example:
@app.route('/alias', alias=True)
@app.route('/path')
def path(request): return b'42'
When /alias is requested werkzeug generates a redirect to /path. But Klein is passing Request.getHost() to Werkzeug, so redirect gets internal hostname and exposes backend's internal hostname and port to the user. Seems like Klein is passing incorrect hostname to Werkzeug. But how can we fix that?
There are two methods in Request:
• Request.getHost() — "Get my originally requesting transport's host" as doc says. Ok, seems like this method intentionally returns server's internal address.
• Request.getRequestHostname() —doc says:
>> "Get the hostname that the user passed in to the request. This will either use the Host: header (if it is available) or the host we are listening on if the header is unavailable."
Cool, but why does this method only returns a hostname without a port? It intentionally strips out the port number from Host header. What is the point of such implementation? This method is used only a couple of times inside Twisted itself, and in both places Twisted gets what getRequestHostname() returned and mixes it with request.getHost().port which is *definitely* incorrect, because the former is user-visible while latter is internal. So if my backend server is using different port than a fronend, it is impossible to use getRequestHostname() to build user-visible URL. I think current getRequestHostname() implementation is broken.
So I have two proposals:
Proposal #1 (fixing current behavior):
• Variant #1: Change Request.getRequestHostname() to return b"hostname:port". I think this is the correct thing to do, but this is a backward-incompatible change.
- or -
• Variant #2: Change Klein to use Request.getHeader(b'Host') with fallback to Request.getHost()
Proposal #2 (adding new feature if Variant #1 is choosed):
• Add useXForwardedHost=False argument to Request.getRequestHostname() and useXForwardedProto=False to Request.isSecure(). If True is passed, these methods will obey corresponding request headers that are de-facto standard for reverse proxies. Also add corresponding options to Klein app. This can simplify reverse proxy configuration a bit.
-- ilya