creationix / node-web

A small and fast web/http library for nodejs. (replaces the built-in http module)
33 stars 4 forks source link

How to detect client's network address? #10

Open mk-pmb opened 7 years ago

mk-pmb commented 7 years ago

How can my app detect the connection mechanism (address family) used for the request and the remote address details (e.g. IP, hostname, port)?

creationix commented 7 years ago

At the libuv level there is uv_tcp_getpeername for this. I forget the name, but node net streams expose this as a method.

creationix commented 7 years ago

It looks like it's split to a few functions on node. See https://nodejs.org/dist/latest-v7.x/docs/api/net.html#net_socket_remoteaddress

mk-pmb commented 7 years ago

node net streams expose this as a method.

Maybe I wasn't clear, I was looking for an "official" way to get address information via the interfaces that "web" exposes to my app. Since you even consider libuv level, I assume your answer was meant to work besides the "web" interface?

I just searched the readme again for "socket" but couldn't find any documented property on either the request object nor the respond function that could have a socket. One of the major advantages I see in the Readme is

Second, it's not bound to the net module anymore.

so even if there were some undocumented additional properties on the request object, my app probably shouldn't rely on them, should it?

creationix commented 7 years ago

Good points.

So looking at the code (haven't looked at it in years). It looks like it expects client to be an instance of a node net stream (https://github.com/creationix/node-web/blob/master/web.js#L74). It doesn't have to be a real net stream, but it at least needs to match the API points the code here uses.

I don't remember if this interface leaks to apps written in this framework. If it does, then we need a documented interface of properties that are known to exist and required for anyone implementing a replacement that doesn't use the net module.

The other option is simply to add a new method on request that asks the internal client instance the address. Feel free to suggest a new API for this.

mk-pmb commented 7 years ago

My current preference is req.lastPeerAddr() which should return the last known peer address as an object that has at least the family property. For IPv4/6 it shall also have address and port so it's compatible with net.Socket. If no address is available, the family key should be set to "unknown". The "last known" part is because net.Socket seems unable to remember the address in some cases: http://stackoverflow.com/questions/12444598/why-is-socket-remoteaddress-undefined-on-end-event http://stackoverflow.com/questions/25257709/get-remoteaddress-from-https-serverclienterror-in-node-js

creationix commented 7 years ago

If you want to query the address after a connection is closed like is mentioned in the other thread, I'll need to automatically query it upon connection and store the result.

If we're unconditionally doing this query, see no reason to use a function as this data is static for a given TCP connection. We should simply expose it as properties.

I don't know the cost of this libuv query, but I expect it's pretty cheap and doing it unconditionally shouldn't be a problem. (famous last words)

mk-pmb commented 7 years ago

as this data is static for a given TCP connection.

A function could expand to other protocols or transports where the addresses might change, e.g. in a peer-to-peer application-layer VPN. I think a protocol framework should impose as few as possible assumptions about transport layers below it. I might even have a location-aware transport layer whose address can represent which room of an exhibition I'm in, and have a long-running chunked stream of announcements that changes by room. The web app wants to update me every minute and thus has to check where I am.

Having a function might save performance in transports where the address is expensive to convert from internal representation to something user-friendly, but doesn't get lost early. A server for that transport wouldn't need to cache the address on connect and can thus defer calculation until an app requests it.

I don't yet have an idea about how to easily deal with transports that need to calculate the address async, but from what I can imagine today that seems so far out that a sync function might be good enough for now.

Transports could, of course, store a custom function on their address object, even one that expects a callback, and indicate this via family, or could use getters for some properties of the address object. Providing the function interface on the framework level makes it easier to write logging middleware, though, and getters are evil because they snatch control flow.

creationix commented 7 years ago

Premature abstraction is the root of all complexity.

Seriously though, what you're proposing sounds neat, but is far beyond the scope of this project and far more scope creep than I can afford personally to maintain. You're welcome to fork this project and take it where you want to go.

mk-pmb commented 7 years ago

Fair enough. :-)

When you do the address lookup shortly after connect and save the property, you'll only ever have one address, so I suggest to use just peerAddr, and make sure it's always an object, with { family: "unknown" } if detection didn't yield anything better. This way I can use just truthiness checks to detect whether a request object might implement the "web" interface (has truthy .peerAddr) or my current "fawa" draft (has truthy .lastPeerAddr).

Another advantage of guaranteeing it will be an object is that you can reliably access properties on it. That way you could decorate peerAddr with extra properties .ip4 and .ip6 in case of IPv4/6 family, set them to peerAddr.address (IP or hostname), and people could write stuff like

let peer = req.peerAddr;
let logAddr = (peer.ip4 || peer.ip6 || peer.family + '.something.strange.');