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

Close connection #59

Closed
Uriopass opened this issue Mar 17, 2021 · 8 comments
Closed

Close connection #59

Uriopass opened this issue Mar 17, 2021 · 8 comments

Comments

@Uriopass
Copy link
Contributor

Let's say a client sends a bogus packet, or some error occurs, or the server wants to ban or rate-limit a client, would it be possible to add a way to close an endpoint? That is, cut the connection. The API could look like Network::close(Endpoint).
The NetEvent::Disconnected event would be sent to the server, and if a new connection attempt is done, the NetEvent::Connected would be sent again.

@lemunozm
Copy link
Owner

Hi @Uriopass,

I think that what you are searching is
Network::remove(ResourceId). In the case you have exposed, you could do something like network.remove(endpoint.resource_id()). Any endpoint can be used: endpoints that you have connected manually or endpoints comming from an accepted connection by a listener.

@Uriopass
Copy link
Contributor Author

Uriopass commented Mar 18, 2021

Isn't a resource for a server the thing returned by Network::listen?
If I just want to disconnect one client and not all clients connected to this resource, will this work?
So resourceId is actually the pair (socket, client)? Then what is endpoint for?

EDIT: After looking through the source code, it looks like ResourceID is actually what is unique "per connection". My question is, then, when a message is received, how can I know from which listener it is associated?

For example

        let (network, events) = Network::split();
        network
            .listen(Transport::Udp, format!("localhost:{}", UNRELIABLE_PORT))?;

        network
            .listen(Transport::FramedTcp, format!("localhost:{}", RELIABLE_PORT))?;

        let ev = events.receive();
        if let NetEvent::Message(endpoint, _) = ev {
              // How can I know if it was from UDP or FramedTCP listener?
        }

@lemunozm
Copy link
Owner

Suppose the following example (explained inline in the comments):

let (tcp_listener_id, addr) = network.listen(Transport::FramedTcp, "127.0.0.1:3000").unwrap();
let (udp_listener_id, addr) = network.listen(Transport::Udp, "127.0.0.1:3000").unwrap();

loop {
    match events.receive() {
        NetEvent::Connected(endpoint, listener_resource_id) => {
            // The listener idenfified with listener_resource_id has accepted a new connection.
            // Udp is not connection oriented so the Connected event is not generated. We can assume here:
            assert_eq!(tcp_listener_id, listener_resource_id); 
            // You can remove the new conection using the resource id of the endpoint that represent it:
            network.remove(endpoint.resource_id());
            // You can remove the listener:
            network.remove(tcp_listener_id);
        }
        NetEvent::Message(endpoint, data) => {
             //Because UDP is not connection-oriented, the resource used internally is the same as the listener, so:
            if endpoint.resource_id() == upd_listener_id() {
                // This message is Udp
                // Note that if you remove like the tcp case: network.remove(endpoint.resource_id()), 
                // the udp listener is removed.
            }
            // Another way to archieve this is checking the adapter of the resource:
            if endpoint.resource_id().adapter_id() == Transport::Udp.id() {
                // This message is Udp
            }
        }
    }
}

@lemunozm
Copy link
Owner

lemunozm commented Mar 18, 2021

Answering your questions:

Isn't a resource for a server the thing returned by Network::listen?

yes, but only the id of that resource, along with the listening address.

If I just want to disconnect one client and not all clients connected to this resource, will this work?

No, you need to disconnect each client based on the resource_id that comes with its endpoint.

So resourceId is actually the pair (socket, client)?

ResourceId = socket internal id + type(remote/local) + adapter id(or transport). The content is codified in 64bits, to easily be copied, hashed, etc..).

Then what is endpoint for?

Endpoint = Remote address + ResourceId. You need also the address because in some cases, like UDP, the listener resource is shared among several remotes. Also, in most cases, you would want to identify the remote by the address.

@Uriopass
Copy link
Contributor Author

I get it know! Thank you. I think my main mistake was thinking UDP was somehow connection oriented, but all the clients actually share the same resource.

@lemunozm
Copy link
Owner

Hi, @tree786 do not worry about open/reopen issues. Your problem can help other users that have the same as you :)

When you listen from UDP. message-io do not save any information of any of the incoming messages (in contrast to TCP, which save that information). The endpoint you manage is created just when the message is received creating the illusion of having something "connected". Since there is no information inside message-io about the remote UDP endpoint, you can not filter/ban the connections.

This feature (ban a user port) needs to be added by yourself, which can be easily managed with a table, avoiding processing messages coming from the blacklist. Nevertheless, this kind of ban should be implemented better at a session-level over the transport and independently of it. This is because if you block the 30000 port, the banned app could be closed and a new app (that should not be banned) can reuse that port and will not be able to connect to your server.

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

3 participants
@Uriopass @lemunozm and others