Python socket error - recv() function

Question

I've been trying to code a simple chat server in Python, my code is as follows:

import socket
import select

port = 11222
serverSocket = socket.socket(socket.AF_INET,socket.SOCK_STREAM)
serverSocket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1024)
serverSocket.bind(('',port))
serverSocket.listen(5)

sockets=[serverSocket]
print 'Server is started on port' , port,'\n'

def acceptConn():
    newsock, addr = serverSocket.accept()
    sockets.append(newsock)
    newsock.send('You are now connected to the chat server\n')
    msg = 'Client joined',addr.__str__(),
    broadcast(msg, newsock)

def broadcast(msg, sourceSocket):
    for s in sockets:
        if (s != serverSocket and s != sourceSocket):
            s.send(msg)
    print msg,


while True:
    (sread, swrite, sexec)=select.select(sockets,[],[])
    for s in sread:
        if s == serverSocket:
            acceptConn()
        else:
            msg=s.recv(100) 
            if msg.rstrip() == "quit":
                host,port=socket.getpeername()
                msg = 'Client left' , (host,port)
                broadcast(msg,s)
                s.close()
                sockets.remove(s)
                del s
            else:
                host,port=s.getpeername()
                msg = s.recv(1024)
                broadcast(msg,s)
                continue

After running the server and connecting via telnet, the server reads single character and skips the next one. Example if I type Hello in telnet, server reads H l o. Any help please ?! :)

Not your actual problem here, but you need to change socket.getpeername() to s.getpeername() here, and also handle clients that drop connection without sending "quit" first. — abarnert, Jul 09 '12 at 18:56
I actually used socket instead of s to get the list of socket members and forgot to rechange it. — Ahmed Mohamed, Jul 09 '12 at 19:09

abarnert · Accepted Answer · 2012-07-09T19:38:40.107

You call recv twice.

First:

msg=s.recv(100)

Then, if that's not "quit", you read and broadcast another message:

msg = s.recv(1024)
broadcast(msg,s)

So the original message is lost.

Because you're using telnet as the client, you get one character at a time, so you see every other character. If you used, say, nc instead, you'd get different results—but still the same basic problem of every other read being thrown away.

There are a few other problems here:

You're expecting clients to send "quit" before quitting—you should be handling EOF or error from recv and/or passing sockets in the x as well as the r.
You're assuming that "quit" will always appear in a single message, and an entire message all to itself. This is not a reasonable assumption with TCP. You may get four 1-byte reads of "q", "u", "i", and "t", or you may get a big read of "OK, bye everyone\nquit\n", neither of which will match.
The "Client left" and "Client joined" messages are tuples, not strings, and they're formed differently, so you're going to see ('Client joined', "('127.0.0.1', 56564)") ('Client left', ('127.0.0.1', 56564)).
You're relying on the clients to send newlines between their messages. First, as mentioned above, even if they did, there's no guarantee that you'll get complete/discrete messages. Second, your "system" messages don't have newlines.

Here's a modified version of your sample that fixes most of the problems, except for requiring "quit" to be in a single message alone and relying on the clients to send newlines:

#!/usr/bin/python

import socket
import select
import sys

port = 11222
serverSocket = socket.socket(socket.AF_INET,socket.SOCK_STREAM)
serverSocket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1024)
serverSocket.bind(('',port))
serverSocket.listen(5)

sockets=[serverSocket]
print 'Server is started on port' , port,'\n'

def acceptConn():
    newsock, addr = serverSocket.accept()
    sockets.append(newsock)
    newsock.send('You are now connected to the chat server\n')
    msg = 'Client joined: %s:%d\n' % addr
    broadcast(msg, newsock)

def broadcast(msg, sourceSocket):
    for s in sockets:
        if (s != serverSocket and s != sourceSocket):
            s.send(msg)
    sys.stdout.write(msg)
    sys.stdout.flush()


while True:
    (sread, swrite, sexec)=select.select(sockets,[],[])
    for s in sread:
        if s == serverSocket:
            acceptConn()
        else:
            msg=s.recv(100)
            if not msg or msg.rstrip() == "quit":
                host,port=s.getpeername()
                msg = 'Client left: %s:%d\n' % (host,port)
                broadcast(msg,s)
                s.close()
                sockets.remove(s)
                del s
            else:
                host,port=s.getpeername()
                broadcast(msg,s)
                continue

To fix the 'quit' problem, you're going to have to keep a buffer for each client, and do something like this:

buffers[s] += msg
if '\nquit\n' in buffers[s]:
   # do quit stuff
lines = buffers[s].split('\n')[-1]
buffers[s] = ('\n' if len(lines) > 1 else '') + lines[-1]

But you've still got the newline problem. Imagine that user1 logs in and types "abc\n" while user2 logs in and types "def\n"; you may get something like "abClient joined: 127.0.0.1:56881\ndec\nf\n".

If you want a line-based protocol, you have to rewrite your code to do the echoing on a line-by-line instead of read-by-read basis.

Thanks, I removed the second recv and it works, but still it reads as I type (without pressing enter) and there are whitespaces after each character. Any help? - Thanks just read your edit. If you could provide some code I would be grateful :) — Ahmed Mohamed, Jul 09 '12 at 19:09
Seems I will re-design the code again, keeping your Points in my head! Thanks for your elegant and comprehensive answer :) — Ahmed Mohamed, Jul 09 '12 at 19:27
It reads as you type because your telnet is sending one byte at a time, so each read is one byte. (Across a real network, the packets may or may not get merged, but with localhost, each one will invariably show up as a separate read.) The whitespace after each character is because the comma explicitly prints a space instead of a newline; if you want to print neither space nor newline, use sys.stdout.write instead of print. — abarnert, Jul 09 '12 at 19:31
One last thing: That "with localhost, each one will invariably show up as a separate read" isn't actually so invariable. It depends on how your platform implements localhost sockets; there's nothing in any standard that says they must do it that way. Hopefully that didn't mislead you. Anyway, the important point is that across a real network, it's completely unpredictable how the writes and reads correspond, so you can't rely on whatever you happen to see with small localhost writes. — abarnert, Jul 11 '12 at 00:10

Python socket error - recv() function

1 Answers1