pushrax / node-rcon

A generic RCON protocol client for node.js
MIT License
134 stars 31 forks source link

Only send a response when authenticated. #12

Closed httpNick closed 3 years ago

httpNick commented 9 years ago

I was having an issue using this where it would send a response in between connecting and authenticated. Adding this check fixed the problem for me.

pushrax commented 9 years ago

Hmm, I guess this is the packets getting reordered, or the server doing something weird. Simply ignoring responses while not authed seems incorrect, maybe the expected behaviour would be to queue them. What do you think?

httpNick commented 9 years ago

True some may want a response when connected, but not authed. It seemed to be an empty response so I am not even sure what that response was for. Yeah queueing might solve the problem. What do you think a good way would be to queue them?

pushrax commented 9 years ago

Something like a _pendingResponses array on the instance that gets pushed to if a response is received before auth, and flushed when auth is complete.

Now that I think about it more, maybe it would be more useful if all response events are fired immediately, but having the auth status available. Still seems like the server is misbehaving here given that it is an empty response. Do you have a packet dump available?

pushrax commented 3 years ago

I think this is possibly fixed properly by 5155353f3c359af80ddf2ed55edebebb6137b0b1. It would be possible in some rare cases for TCP segmentation to cause the code to parse an incomplete packet.