2
\$\begingroup\$

People say that the code in my implementation is generally obscure. Can you point me what looks bad?


import time
import string
from queue import Queue
from channel import Channel
from client import Client


__author__ = 'mark'
import socket
import connection
from threading import Thread

SPECIAL = "[]\`_^{|}"
ALLOWED_NICKNAME = string.digits + string.ascii_letters + SPECIAL + "-"
# noinspection PyUnboundLocalVariable
ALLOWED_CHANNEL = "".join([chr(x) for x in range(128)]).replace("\0", "").replace("\7", "").replace("\13", "").replace("\10", "")\
    .replace(" ", "").replace(",", "").replace(":", "")


class IRCServer(object):
    _message_queue = Queue()
    _sock = None
    _connections = None
    _bind_address = None
    running = None
    _clients = None
    _channels = None
    _nick_change_failed = []

    '''
        This prevents memory leak when the client who had error was disconnected
    '''
    def _dead_check_thread(self):
        while self.running:
            for conn in self._nick_change_failed:
                for client_conn in self._clients:
                    if client_conn == conn:
                        break
                else:
                    self._nick_change_failed.remove(conn)

    def _ping_check_thread(self):
        while self.running:
            time.sleep(1.0)
            for conn in self._clients.keys():
                if self._clients[conn].last_pinged >= 250:
                    print("[DBG] %s disconnected because ping has timed-out" %
                          str(conn.address))
                    self.disconnect(conn, "Ping timeout: 250 seconds")
                else:
                    self._clients[conn].last_pinged += 1

    def _ping_thread(self):
        while self.running:
            time.sleep(100.0)
            for client_conn in self._clients.keys():
                try:
                    self._clients[client_conn].send("PING")
                except IOError:
                    self.disconnect(client_conn, "Remote host closed the connection")

    def _message_thread(self):
        while self.running:
            for conn in self._connections:
                try:
                    for msg in conn.get_messages():
                        self._message_queue.put(msg)
                except IOError:  # We couldn't read from socket, thus the connection is dead.
                    print("[DBG] can't read from connection %s" % str(conn))
                    self._connections.remove(conn)

    def _message_handler_thread(self):
        self._nick_change_failed = []
        while self.running:
            msg = self._message_queue.get(True)
            text = msg.get_data()
            conn = msg.get_connection()
            args = text.replace("\r", "").replace("\n", "").split(" ")
            command = args[0].upper()
            command_args = args[1:]
            if command == "NICK":
                if len(command_args) = 2:
                    self._clients[conn].send("PONG %s" % command_args[1])
                else:
                    self._clients[conn].send("PONG")
            elif command == "QUIT":
                for client in self._clients.values():
                    for channel in self._clients[conn].channels:
                        if channel in client.channels:
                            client.connection.send("%s QUIT %s" % (self._clients[conn].get_identifier(),
                                                                   " ".join(command_args)))
                            break
                del self._clients[conn]  # It's dead
            elif command == "TOPIC":
                if len(command_args)  users and 
               services on  servers"
       252    RPL_LUSEROP
              " :operator(s) online"
       253    RPL_LUSERUNKNOWN
              " :unknown connection(s)"
       254    RPL_LUSERCHANNELS
              " :channels formed"
       255    RPL_LUSERME
              ":I have  clients and 
                servers"
        """
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 251 %s :There are %d users and 0 services on 1 servers" % (self.name, nick, len(self._clients)))
        self._clients[conn].send(":%s 252 %s 0 :operator(s) online" % (self.name, nick))
        self._clients[conn].send(":%s 253 %s 0 :unknown connection(s)" % (self.name, nick))
        self._clients[conn].send(":%s 254 %s %d :channels formed" % (self.name, nick, len(self._channels)))
        self._clients[conn].send(":%s 255 %s :I have %d clients and 1 servers" % (self.name, nick, len(self._clients)))

    def _send_no_channel(self, conn, chan_name):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 403 %s %s :No such channel" % (self.name, nick, chan_name))

    def _send_no_user(self, conn, target):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 401 %s %s :No such nick/channel" % (self.name, nick, target))

    def _send_not_enough_parameters(self, conn, command):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 461 %s %s :Not enough parameters" % (self.name, nick, command))

    def _send_unknown_command(self, conn, command):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 421 %s %s :Unknown command" % (self.name, nick, command))

    def _send_nickname_in_use(self, conn, nick):
        conn.send(":%s 433 %s :Nickname already in use" % (self.name, nick))

    def _send_erroneous_nickname(self, conn, nick):
        conn.send(":%s 432 %s :Erroneous nickname" % (self.name, nick))

    def disconnect(self, conn, message):
        client = self._clients[conn]
        identifier = client.identifier if client.identifier else client.nick
        self._send_to_related(conn, ":%s QUIT :%s" % (identifier, message))
        try:
            self._clients[conn].send("ERROR :Closing link [%s]: Disconnected" % conn.address)
        except IOError:
            pass
        del self._clients[conn]

    def _send_to_related(self, conn, msg, ignore_self=False):
        clnt = self._clients[conn]
        for client in self._clients.values():
            if ignore_self and client != conn:
                continue

            for channel in client.channels:
                if channel in clnt.channels:
                    client.connection.send(msg)
                    break

    def _channel_broadcast(self, conn, chan, msg):
        for client in self._clients.values():
            if client.nick == self._clients[conn].nick:
                continue
            if chan in client.channels:
                client.send(msg)


    def _nick_in_use(self, nick):
        """
        :param nick: Nickname of client
        :type nick: str
        :return: True if a client with that name exists, else False
        """
        for client in self._clients.values():
            if client.nick == nick:
                return True
        else:
            return False


    def _valid_nick(self, nick):
        if not all(c in ALLOWED_NICKNAME for c in nick) or len(nick) > 9 or\
                not len(nick):
            return False
        return True

    def _set_nick(self, conn, nick, ident=None):
        if self._nick_in_use(nick):
            self._send_nickname_in_use(conn, nick)
            return False
        elif not self._valid_nick(nick):
            self._send_erroneous_nickname(conn, nick)
            return False
        else:
            if not conn in self._clients:
                self._clients[conn] = Client(connection=conn, nick=nick)
            else:
                old_nick = self._clients[conn].nick
                self._clients[conn].nick = nick
                self._send_to_related(conn, "%s NICK %s" % (old_nick, nick))
                if ident:
                    self._clients[conn].ident = self._clients[conn].get_nick() + "!" + \
                                                                   ident + "@" + self.name

            return True


    def __del__(self):
        self._sock.close()
        del self._sock
        self.stop()
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

In the method IRCServer._ping_check_thread, you seem to be checking if a client's connection with the server has timed out.

However, you seem to be re-inventing the wheel there. The python socket library already has two functions that handle with timeouts called socket.setdefaulttimeout and socket.settimeout.

Both of these functions take a float parameter that specifies how long before a the socket.timeout error is raised and the user is disconnected. However, the method socket.setdefaulttimeout sets the timeout for all sockets.

I recommend that you use this rather than re-invent the wheel.

Note: I believe by using one of the settimeouts, the client will automatically disconnect. However, I don't know for sure.


I may be missing something, but it doesn't look like anywhere in your code are you using the socket library.

In that case, I don't see why you imported it.


I will look more into this post later; right now, I do not have the time.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.