Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposed user happiness]A little consideration for add some useful apis in SocketStream #544

Closed
WindSoilder opened this issue Jun 9, 2018 · 2 comments

Comments

@WindSoilder
Copy link
Contributor

WindSoilder commented Jun 9, 2018

Hi,
I'm trying to build up a TCP server project, and after a little exploring on trio, I think maybe we can add some apis in the SocketStream class.

Consider for the basic echo server :)

async def echo_server(server_stream: SocketStream):
    # if we need to know who is connection to the server
    # may be we have to do something like this
    client_addr = server_stream.socket.getpeername()

async def main():
    await trio.serve_tcp(echo_server, PORT)

trio.run(main)

If we get a connection, I wonder how can we get client address(I think it's a common case for server to get client address)?
After a little hack into trio, I don't find out a way to get client address directly (except using the internal socket to get address like server_stream.socket.getpeeraddress(), maybe I'm wrong..)

I think exposing the api like getpeeraddress (of cause we can rename it like get_remote_addr), and recv_line(for some tcp based protocol which is using text, and they are separated by \r\n) can make trio more friendly.

The implementation for get client address can be implement like this:

def get_remote_addr(self):
    """
    Get remote address of connected connection

    See :meth:`socket.socket.getpeername` for details
    """
    return self.socket.getpeername()

provide the api like recv_line maybe harmful...I need more investigation on it :(

I'm sorry if I have make something wrong, please correct me :) What do you think about this ?

@WindSoilder WindSoilder changed the title A little consideration for add some useful apis in SocketStream [proposed user happiness]A little consideration for add some useful apis in SocketStream Jun 9, 2018
@njsmith
Copy link
Member

njsmith commented Jun 10, 2018

Thanks for trying out Trio, and for sharing feedback!

We definitely should have a high-level API for getpeername and getsockname – I just put it off initially because there are a few issues I want to think through. In particular, how to represent addresses in different namespaces, and how to handle cases like the PROXY protocol where the public address doesn't match the socket address. There's some small notes on this in #280. In the mean time, stream.socket.getpeername() is the simplest workaround, and totally fine to use – in general anything in Trio that doesn't involve a leading underscore somewhere is an intentional public API. (Though note that if you use serve_ssl_over_tcp, then it becomes: ssl_stream.transport_stream.socket.getpeername().)

For recv_line, yeah, it's bad that we don't provide any simple way to do this right now, we definitely need to make this nicer for users :-/. But it's also tricky (like you say), because there are so many different protocols (what if I want \n-separated lines? what if I have a protocol that mostly uses lines but then sometimes switches to some other framing, like HTTP does?), and it creates complex issues around buffering (if you read past the \r\n, you need to stash the extra data somewhere). I think the ideal solution is to teach people about the Sans-IO pattern for implementing protocols, create tools to make this much easier for simple cases like line-based protocols, and then provide some nice simple accessible examples of how it all fits together in Trio's documentation. But this requires finishing implementing that sansio_toolbelt library, and then writing a nice tutorial, and I haven't gotten around to either of those yet :-/.

In the mean time, here's a simple way of parsing lines out of a Trio stream:

# The two magic values here are pretty arbitrary
async def lines_in_stream(stream, receive_chunk_size=16384, max_line_length=4096, separator=b"\r\n"):
    buf = bytearray()
    while True:
        idx = buf.find(separator)
        if idx == -1:
            # Having a maximum line length is important to avoid DoS attacks
            # And also reduces the impact of our naive O(n**2) line-ending finding algorithm
            # (by putting a maximum limit on 'n')
            if len(buf) >= max_line_length:
                raise ProtocolError("line too long")
            chunk = await stream.receive_some(read_chunk_size)
            if not chunk:
                # No more data
                if buf:
                    raise ProtocolError("connection closed in the middle of a line")
                else:
                    return
        else:
            yield buf[:idx]
            del buf[:idx + len(separator)]

This is an async generator, so you use it like:

async for line in lines_in_stream(stream):
    print("got line!", line)

This uses the native async generator syntax that was introduced in Python 3.6. If you care about 3.5 support, then the async_generator library should let you backport it to 3.5.

Also, I haven't actually tested this, I just typed it straight into the github comment box, but I think the basic idea is correct at least.

Sorry for the bother – we're still working on sorting a lot of this stuff out :-). But hopefully that will let you get started for now, and it's great to hear which things are causing trouble for you; it helps us prioritize new features.

@njsmith
Copy link
Member

njsmith commented Mar 29, 2019

It looks like everything here is covered more directly in #280 and #796, so I'm closing this one.

@njsmith njsmith closed this as completed Mar 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants