Add unsafe_remote_ip property

[betatim]

This property represents the IP closest to the user. This means it is
potentially unsafe because clients can fake it but might be more useful for geolocation purposes.

Closes #904

---

Hi 👋!

This is my first attempt at contributing to tornado (long time user via JupyterHub and BinderHub). I picked #904 because it looked interesting and not too hard. However it is a very old issue so maybe thinking has moved on since then about how useful it is?

I thought I'd open this PR already to get some early feedback. I think if things look about right a better/more doc string is needed and maybe some more edge case test. Also need to look at availability of the ipaddress module in terms of which versions tornado supports.

I think it is ok to always provide this, not only when xheaders=True, as it is always "unsafe" to look at this value/take it at face value.

What do you think of the name of the new property? Is the HTTPServerRequest class the right place for this or should it be in the connection class like the remote_ip handling?

(should this be a draft PR until it is ready for reviewing?)

[bdarnell]

However it is a very old issue so maybe thinking has moved on since then about how useful it is?

I don't think anyone has changed their minds about how useful this is, but it's always been something that's just a little bit useful, so no one has gotten around to implementing it. I'm happy to have a contribution for it.

Also need to look at availability of the ipaddress module in terms of which versions tornado supports.

Using the ipaddress module is fine (it was added in 3.3; our minimum is now 3.6).

I think it is ok to always provide this, not only when xheaders=True, as it is always "unsafe" to look at this value/take it at face value.

I agree. But it is strange that this uses different rules than xheaders, i.e. that it only looks at X-Forwarded-For and not X-Real-IP, and it only returns "global" IPs. Maybe that's OK (I don't think I'd want to change either of those things), but it should be documented.

What do you think of the name of the new property?

"Unsafe" is OK, but it sounds a little too strong and scary. Maybe "untrusted" or "unverified"?

Is the HTTPServerRequest class the right place for this or should it be in the connection class like the remote_ip handling?

I'm not sure. It feels like they both belong in the same place, but on the other hand I prefer to put things on HTTPServerRequest instead of in the HTTPServer as much as possible if they don't need state or direct access to the connection.

(should this be a draft PR until it is ready for reviewing?)

Nah, I never really use draft PRs.