[fix] Flash Security event

[yannsucc]

• fix a case where if you call TcpFlashClient::Connect() twice (by making an auto reconnection for exemple) the SecurityErrorEvent was not correctly listen on the second call. add

*fix loseConnection call on Server.hx, we call the function before remove all socket in case of user need to remove some reference.

[yannsucc]

Don't forget this too ! haha

joke :)

[5Mixer]

Hi. Another issue was found, not sure if it was ever on github, where calling .broadcast() inside the 'loseConnection' handler caused the server to crash. It was because by the time that eof error is called, that socket is entirely invalid, so using it in .broadcast or .send crashed the server. I'd like to check that this doesn't happen before merging. Thanks!

[yannsucc]

seems strange. on our project, we use a broadcast inside loseConnection without any issue. :s

[yannsucc]

any news about testing crash using broadcast/send inside the "loseConnection" ?

[5Mixer]

Hey there! I'm super sorry about being so late again. I'm lazy and forgetful!

I'm still not sure about your code though. All of this code is firing when the socket hits an eof error. By that stage, the client is gone. There isn't a reason why, the TCP connection was just cut. This could obviously be for any reason, even the client losing internet connection. My point is that by the time this eof error is triggered, the client isn't there any more.

This calls the .onConnectionClose event, where mphx users can do whatever they want, including broadcasting. Given that .broadcast sends to all connections that should be live, I don't see why onConnectionClose should be ran before the socket is removed from the array. It doesn't make sense to try to send to a connection that we know has dropped.

However, broadcasting to all other players that a player has dropped (except for the player that dropped) makes sense, you often want to know a player disconnected. I changed the basic/simple test to show an example for how this might look, and in my tests (keep in mind this is all on the Neko target) it worked as expected. (4b809fadd108d19d3)

If you want the server to be able to handle a client disconnection more, I suppose, gracefully, then your best bet is to have that client send a quit event to the server, along with a reason etc.

If this example behaves unexpectedly (perhaps on flash), feel free to reply. Hopefully I am quicker!

[yannsucc]

Sorry for my late reply. I'm busy

+                           protocol.loseConnection("close connection");
                            readSockets.remove(socket);
                            clients.remove(socket);
                            error = true;
-                           protocol.loseConnection("close connection");

I only call protocol.loseConnection before remove the socket reference on container(readsockets/clients) if the user need to remove the socket reference on his own custom container (like map/array/etc) because protocal loseConnection close the socket, and set it to null (in my memory).

In my current project for exemple (server side), i have a map<Socket/GameUser> and i can't correctly remove a GameUser because of this.

Don't know if i correctly explain my point of view, but feel free to accept or not this fix.

The second commit (https://github.com/5Mixer/mphx/pull/48/commits/44d5b7ac634a6f579fb5d14316919b49b878b002) is very important because it's fix the server sending the crossdomain.xml on port 843 to a flash client, when a Neko/C++ server is on a distant server (like OVH). If the flash client don't receive this crossdomain, it can't connect to a classic server.

I use these fix without any issue for 5 month 👍