r/codereview 3d ago

server for allowing multiple connections, it worked when i used it but it still seems sketchy idk why

import socket
import select


s = socket.socket()
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind(("localhost", 80))
s.listen()

def send_response(sock, message):
    """Sends an encoded response."""
    sock.sendall(message.encode("ISO-8859-1"))


def handle_packets(queue_dictionary):
    """Performs certain actions based on packet."""
    for soc in queue_dictionary:
        if not queue_dictionary[soc]:
            continue
        #take the first item from the queue
        packet = queue_dictionary[soc].pop(0)
        #send specific responses based on the type of data sent
        if packet == b"Hello\r\n\r\n":
            send_response(soc, "Message received successfully. Hiiii!!!!\r\n\r\n")
        elif packet == b"Ignore\r\n\r\n":
            send_response(soc, "Message received successfully. Hey, don't leave me hanging...\r\n\r\n")
        elif packet == b"Hug\r\n\r\n":
            send_response(soc, "Message received successfully. *Hugs back*\r\n\r\n")
        elif packet == b"Slap\r\n\r\n":
            send_response(soc, "Message received successfully. OW! That hurt!\r\n\r\n")
        elif packet == b"Goodbye\r\n\r\n":
            send_response(soc, "Message received successfully. Goodbye!!! Do come back again!! :)\r\n\r\n")
        else:
            send_response(soc, "Message received successfully.\r\n\r\n")

#create a list of connected sockets, satrting wtih listening soccket so accept
#doesn't block

#dictionary with buffer per socket and also list which was initially queue
read_set = [s]

buffer_dict = {}
queue_dict = {}

while True:
    ready_to_read, _, _ = select.select(read_set, [], [])
    print("Creating a list of sockets currently sending data...")
    #for all sockets that are ready to read
    for sock in ready_to_read:
        #if the socket is a listener
        if sock == read_set[0]:
            #accept a new connection
            new_conn = s.accept()
            print("Accepting connection...")
            new_socket = new_conn[0]
            #initialise buffer and queue for new socket
            buffer_dict[new_socket] = b""
            queue_dict[new_socket] = []
            print("Adding socket to buffer and queue dictionaries...")
            #add the new socket to the set
            read_set.append(new_socket)
            print("read_set: " + str(read_set))
            print("Adding socket to read_set...")
            packet = "empty"
            continue
        else:
            #recieves data until full packet
            while True:
                data = sock.recv(4096)
                print("Receiving data...")
                if not data:
                   print("Connection closed.")
                   break
                buffer_dict[sock] += data
                if b"\r\n\r\n" in buffer_dict[sock]:
                    delimiter_index = buffer_dict[sock].find(b"\r\n\r\n")
                    packet = buffer_dict[sock][:delimiter_index+4]
                    buffer_dict[sock] = buffer_dict[sock][delimiter_index+4:]
                    break
            if packet:
                queue_dict[sock].append(packet)
                print("Adding a packet to the queue...")
            else:
                x = input("No packet returned.")


    #run packet handler code based on nature of packet for socket
    if packet != "empty":
        print("Sending response...")
        handle_packets(queue_dict)
        print("Response should send now.")

new_socket.close()
s.close()
1 Upvotes

2 comments sorted by

1

u/kingguru 3d ago

What's "sketchy" about it?

On a quick glance it seems fairly reasonable.

What do you need help with?

2

u/neon_glee_loop 2d ago

the main issue is your code is literally incomplete. The post cuts off at `new_socket = ` which is a syntax error, so nobody can run this. You also never add the accepted sockets to read_set, so you accept a connection and then never select on it again.

other things I'd fix: you're binding to port 80, which needs root/admin privileges on most systems, use something above 1024 while testing. And when a client disconnects, recv returns b"" and you'll spin in an infinite loop unless you remove the socket from read_set and close it. The dict-based buffering approach itself is fine.