Re: internal vs. external hostname in Request.getHost9)
Ilya Skriblovsky <[email protected]> Tue, 14 Mar 2017 22:00:00 +0000
| Newsgroups | gmane.comp.python.twisted.web |
|---|---|
| Message-ID | <CAOG7vkwe5EXOirbaniA6_bPeNhGQGb8MRKU=ApjaoOMrk9bqzA@mail.gmail.com> |
--===============0826628897807731371==
Content-Type: multipart/alternative; boundary=001a1141a8aaa8e5a7054ab7f33d
--001a1141a8aaa8e5a7054ab7f33d
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
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=3DTrue)
@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:
=E2=80=A2 Request.getHost() =E2=80=94 "Get my originally requesting transpo=
rt's host" as
doc says. Ok, seems like this method intentionally returns server's
internal address.
=E2=80=A2 Request.getRequestHostname() =E2=80=94doc 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):
=E2=80=A2 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 -
=E2=80=A2 Variant #2: Change Klein to use Request.getHeader(b'Host') with f=
allback
to Request.getHost()
Proposal #2 (adding new feature if Variant #1 is choosed):
=E2=80=A2 Add useXForwardedHost=3DFalse argument to Request.getRequestHostn=
ame() and
useXForwardedProto=3DFalse 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
=D0=B2=D1=82, 14 =D0=BC=D0=B0=D1=80. 2017 =D0=B3. =D0=B2 10:33, Ilya Skribl=
ovsky <[email protected]>:
> Thanks, I will study tickets you mentioned and hopefully fix it.
> Quick-n-dirty fix gave me only two failed tests and in both cases it seem=
s
> to be a wrong assumption in tests. So I hope this change won't break the
> world.
>
> -- ilya
>
> =D0=B2=D1=82, 14 =D0=BC=D0=B0=D1=80. 2017 =D0=B3. =D0=B2 10:12, Glyph Lef=
kowitz <[email protected]>:
>
>
> On Mar 13, 2017, at 11:01 PM, Ilya Skriblovsky <[email protected]=
>
> wrote:
>
> Hi,
>
> I'm using Twisted Web server behind Nginx reverse-proxy and I'm getting
> backend's internal host:port from Request.getHost().
>
> Seems like Request.host is explicitly set to socket's address (i.e.
> internal address) here: =E2=80=8B
> https://github.com/twisted/twisted/blob/trunk/src/twisted/web/http.py#L83=
8
> But comment at =E2=80=8B
> https://github.com/twisted/twisted/blob/trunk/src/twisted/web/http.py#L12=
97
> and what this method does points that Request.host meant to reflect Host
> header of the request, i.e. user-visible hostname and port.
>
> This creates problems for me when using Klein because it correctly uses
> Request.getHost() to create host part of URLs for redirects.
>
> It seems like inconsistency in Twisted code. I'd expect Request.host
> should be only set from the Host request header to reflect user-visible
> hostname, not the internal backend server's address. Or may be I'm missin=
g
> something?
>
>
> You're absolutely correct! I even filed a ticket for this functionality,
> 5 years ago: https://twistedmatrix.com/trac/ticket/5807 There's even a
> branch for it. Oddly enough we *do* have a *private*
> _XForwardedForRequest, but... it's only used for logging, for some reason=
.
>
> If you want accurate access logging and request information,
> https://twistedmatrix.com/trac/ticket/7704 will probably also be of
> interest to you.
>
> I'm so sorry you've hit this glaring deficiency in Twisted.
>
> On the other hand: I'm so glad that you've hit this glaring deficiency in
> Twisted! I hope you will be motivated to fix it :-). It's bothered me f=
or
> quite some time that we don't play nicely with proxying setups, when such
> setups are so *incredibly* common. If you can write pull requests to fix
> these issues and put them into review, I'm pretty sure you will find an
> enthusiastic reviewer quickly.
>
> -glyph
> _______________________________________________
> Twisted-web mailing list
> [email protected]
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-web
>
>
--001a1141a8aaa8e5a7054ab7f33d
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr">Tickets you have mentioned and=C2=A0<span style=3D"font-fa=
mily:"helvetica neue","bitstream vera sans",helvetica,a=
rial,sans-serif">forwarded-for-5807 branch</span>=C2=A0are mostly about par=
sing X-Forwarded-For in order to obtain correct client IP. While it is valu=
able task, it is not what strikes me right now.<div><br></div><div>I'm =
now more concerned with an absence of API for getting user-visible server&#=
39;s name, not client's ip.</div><div><div><br class=3D"inbox-inbox-App=
le-interchange-newline">Look, I'm currently porting my app from Django =
to Klein and noticed strange behavior of Klein. For example:</div><div>@app=
.route('/alias', alias=3DTrue)</div><div>@app.route('/path'=
)</div><div>def path(request): return b'42'</div><div><br></div><di=
v>When /alias is requested werkzeug generates a redirect to /path. But Klei=
n is passing Request.getHost() to Werkzeug, so redirect gets internal hostn=
ame 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 t=
hat?</div></div><div><br></div><div>There are two methods in Request:</div>=
<div>=E2=80=A2 Request.getHost() =E2=80=94 "Get my originally requesti=
ng transport's host" as doc says. Ok, seems like this method inten=
tionally returns server's internal address.</div><div>=E2=80=A2 Request=
.getRequestHostname() =E2=80=94doc says:</div><div>>> "Get the h=
ostname that the user passed in to the request. This will either use the Ho=
st: header (if it is available) or the host we are listening on if the head=
er is unavailable."</div><div>Cool, but why does this method only retu=
rns 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 Twist=
ed gets what getRequestHostname() returned and mixes it with request.getHos=
t().port which is *definitely* incorrect, because the former is user-visibl=
e 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=
.<br></div><div><br></div><div>So I have two proposals:</div><div><br></div=
><div>Proposal #1 (fixing current behavior):</div><div>=E2=80=A2 Variant #1=
: Change Request.getRequestHostname() to return b"hostname:port".=
I think this is the correct thing to do, but this is a backward-incompatib=
le change.</div><div>- or -</div><div>=E2=80=A2 Variant #2: Change Klein to=
use Request.getHeader(b'Host') with fallback to Request.getHost()<=
/div><div><br></div><div>Proposal #2 (adding new feature if Variant #1 is c=
hoosed):</div><div>=E2=80=A2 Add useXForwardedHost=3DFalse argument to Requ=
est.getRequestHostname() and useXForwardedProto=3DFalse to Request.isSecure=
(). If True is passed, these methods will obey corresponding request header=
s that are de-facto standard for reverse proxies. Also add corresponding op=
tions to Klein app. This can simplify reverse proxy configuration a bit.</d=
iv><div><br></div><div>-- ilya<br></div></div><br><div class=3D"gmail_quote=
"><div dir=3D"ltr">=D0=B2=D1=82, 14 =D0=BC=D0=B0=D1=80. 2017 =D0=B3. =D0=B2=
10:33, Ilya Skriblovsky <<a href=3D"mailto:[email protected]">i=
[email protected]</a>>:<br></div><blockquote class=3D"gmail_quote=
" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><=
div dir=3D"ltr" class=3D"gmail_msg">Thanks, I will study tickets you mentio=
ned and hopefully fix it.<div class=3D"gmail_msg">Quick-n-dirty fix gave me=
only two failed tests and in both cases it seems to be a wrong assumption =
in tests. So I hope this change won't break the world.</div><div class=
=3D"gmail_msg"><br class=3D"gmail_msg"></div><div class=3D"gmail_msg">-- il=
ya<br class=3D"gmail_msg"><br class=3D"gmail_msg"><div class=3D"gmail_quote=
gmail_msg"><div dir=3D"ltr" class=3D"gmail_msg">=D0=B2=D1=82, 14 =D0=BC=D0=
=B0=D1=80. 2017 =D0=B3. =D0=B2 10:12, Glyph Lefkowitz <<a href=3D"mailto=
:[email protected]" class=3D"gmail_msg" target=3D"_blank">glyph@twist=
edmatrix.com</a>>:<br class=3D"gmail_msg"></div></div></div></div><div d=
ir=3D"ltr" class=3D"gmail_msg"><div class=3D"gmail_msg"><div class=3D"gmail=
_quote gmail_msg"><blockquote class=3D"gmail_quote gmail_msg" style=3D"marg=
in:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style=3D"wo=
rd-wrap:break-word" class=3D"gmail_msg"><br class=3D"gmail_msg"><div class=
=3D"gmail_msg"><blockquote type=3D"cite" class=3D"gmail_msg"><div class=3D"=
gmail_msg">On Mar 13, 2017, at 11:01 PM, Ilya Skriblovsky <<a href=3D"ma=
ilto:[email protected]" class=3D"gmail_msg" target=3D"_blank">ilyas=
[email protected]</a>> wrote:</div><br class=3D"m_-869863249413997738=
6m_-3306553942595082654Apple-interchange-newline gmail_msg"><div class=3D"g=
mail_msg"><div dir=3D"ltr" class=3D"gmail_msg"><div class=3D"gmail_msg">Hi,=
</div><div class=3D"gmail_msg"><br class=3D"gmail_msg"></div><div class=3D"=
gmail_msg">I'm using Twisted Web server behind Nginx reverse-proxy and =
I'm getting backend's internal host:port from Request.getHost().</d=
iv><div class=3D"gmail_msg"><br class=3D"gmail_msg"></div><div class=3D"gma=
il_msg">Seems like Request.host is explicitly set to socket's address (=
i.e. internal address) here: =E2=80=8B<a href=3D"https://github.com/twisted=
/twisted/blob/trunk/src/twisted/web/http.py#L838" class=3D"gmail_msg" targe=
t=3D"_blank">https://github.com/twisted/twisted/blob/trunk/src/twisted/web/=
http.py#L838</a> But comment at =E2=80=8B<a href=3D"https://github.com/twis=
ted/twisted/blob/trunk/src/twisted/web/http.py#L1297" class=3D"gmail_msg" t=
arget=3D"_blank">https://github.com/twisted/twisted/blob/trunk/src/twisted/=
web/http.py#L1297</a> and what this method does points that Request.host me=
ant to reflect Host header of the request, i.e. user-visible hostname and p=
ort.</div><div class=3D"gmail_msg"><br class=3D"gmail_msg"></div><div class=
=3D"gmail_msg">This creates problems for me when using Klein because it cor=
rectly uses Request.getHost() to create host part of URLs for redirects.</d=
iv><div class=3D"gmail_msg"><br class=3D"gmail_msg"></div><div class=3D"gma=
il_msg">It seems like inconsistency in Twisted code. I'd expect Request=
.host should be only set from the Host request header to reflect user-visib=
le hostname, not the internal backend server's address. Or may be I'=
;m missing something?</div></div></div></blockquote></div><div class=3D"gma=
il_msg"><br class=3D"gmail_msg"></div></div><div style=3D"word-wrap:break-w=
ord" class=3D"gmail_msg"><div class=3D"gmail_msg">You're absolutely cor=
rect!=C2=A0 I even filed a ticket for this functionality, 5 years ago:=C2=
=A0<a href=3D"https://twistedmatrix.com/trac/ticket/5807" class=3D"gmail_ms=
g" target=3D"_blank">https://twistedmatrix.com/trac/ticket/5807</a> =C2=A0T=
here's even a branch for it.=C2=A0 Oddly enough we <i class=3D"gmail_ms=
g">do</i> have a <i class=3D"gmail_msg">private</i> _XForwardedForRequest, =
but... it's only used for logging, for some reason.</div><div class=3D"=
gmail_msg"><br class=3D"gmail_msg"></div><div class=3D"gmail_msg"><div clas=
s=3D"gmail_msg"><div class=3D"gmail_msg">If you want accurate access loggin=
g and request information,=C2=A0<a href=3D"https://twistedmatrix.com/trac/t=
icket/7704" class=3D"gmail_msg" target=3D"_blank">https://twistedmatrix.com=
/trac/ticket/7704</a> will probably also be of interest to you.</div><div c=
lass=3D"gmail_msg"><br class=3D"gmail_msg"></div></div></div><div class=3D"=
gmail_msg"><div class=3D"gmail_msg">I'm so sorry you've hit this gl=
aring deficiency in Twisted.</div><div class=3D"gmail_msg"><div class=3D"gm=
ail_msg"><br class=3D"gmail_msg"></div></div><div class=3D"gmail_msg">On th=
e other hand: I'm so glad that you've hit this glaring deficiency i=
n Twisted!=C2=A0 I hope you will be motivated to fix it :-).=C2=A0 It's=
bothered me for quite some time that we don't play nicely with proxyin=
g setups, when such setups are so <i class=3D"gmail_msg">incredibly</i>=C2=
=A0common.=C2=A0 If you can write pull requests to fix these issues and put=
them into review, I'm pretty sure you will find an enthusiastic review=
er quickly.</div></div><div class=3D"gmail_msg"><br class=3D"gmail_msg"></d=
iv><div class=3D"gmail_msg">-glyph</div></div>_____________________________=
__________________<br class=3D"gmail_msg">
Twisted-web mailing list<br class=3D"gmail_msg">
<a href=3D"mailto:[email protected]" class=3D"gmail_msg" target=
=3D"_blank">[email protected]</a><br class=3D"gmail_msg">
<a href=3D"http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-web" r=
el=3D"noreferrer" class=3D"gmail_msg" target=3D"_blank">http://twistedmatri=
x.com/cgi-bin/mailman/listinfo/twisted-web</a><br class=3D"gmail_msg">
</blockquote></div></div></div></blockquote></div>
--001a1141a8aaa8e5a7054ab7f33d--
--===============0826628897807731371==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Twisted-web mailing list
[email protected]
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-web
--===============0826628897807731371==--