Network protocol using TCP, sending images through sockets

Question

I would like to ask about your opinion about my code. The idea is simple: I designed my own protocol, where client asks the server about the image, and the server sends the image, following the below steps (this is the actual protocol I wanted to implement):

CLIENT SERVER
 GET\r\n 
 ----------------------------------->
 OK\r\n
 <----------------------------------- 
 GET_SIZE\r\n
 ----------------------------------->
 SIZE 1024\r\n
 <-----------------------------------
 GET_IMG\r\n
 ----------------------------------->
 IMG_DATA with when image sending is over EOF\r\n
 <-----------------------------------

Everything works (it seems so) but I would like to ask about what else I could possibly improve here.

1. Does the implementation reflects in 100% what I wanted to achieve in my protocol flow?
2. Are the steps implemented as I wanted them to be in my protocol?

client.py:

#!/usr/bin/env python
import socket
import sys
HOST = '127.0.0.1'
PORT = 6666
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
server_address = (HOST, PORT)
sock.connect(server_address)
fname = 'fromserver.png'
try:
 sock.sendall("GET\r\n")
 data = sock.recv(4096)
 if data:
 txt = data.strip()
 print '--%s--' % txt
 if txt == 'OK':
 sock.sendall("GET_SIZE\r\n")
 data = sock.recv(4096)
 if data:
 txt = data.strip()
 print '--%s--' % txt
 if txt.startswith('SIZE'):
 tmp = txt.split()
 size = int(tmp[1])
 print '--%s--' % size
 sock.sendall("GET_IMG\r\n")
 myfile = open(fname, 'wb')
 amount_received = 0
 while amount_received < size:
 data = sock.recv(4096)
 if not data :
 break
 amount_received += len(data)
 print amount_received
 txt = data.strip('\r\n')
 if 'EOF' in str(txt) :
 print 'Image received successfully'
 myfile.write(data)
 myfile.close()
 else :
 myfile.write(data)
finally:
 sock.close()

server.py:

 #!/usr/bin/env python
import random
import socket, select
from time import gmtime, strftime
image = 'tux.png'
HOST = '127.0.0.1'
PORT = 6666
connected_clients_sockets = []
server_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
server_socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
server_socket.bind((HOST, PORT))
server_socket.listen(10)
connected_clients_sockets.append(server_socket)
while True:
 read_sockets, write_sockets, error_sockets = select.select(connected_clients_sockets, [], [])
 for sock in read_sockets:
 if sock == server_socket:
 sockfd, client_address = server_socket.accept()
 connected_clients_sockets.append(sockfd)
 else:
 try:
 data = sock.recv(4096)
 if data :
 txt = data.strip()
 print '--%s--'%txt
 if txt == 'GET' :
 sock.sendall('OK\r\n')
 elif txt == 'GET_SIZE' :
 with open ('tux.png','rb') as f1:
 file_size = len(f1.read())
 f1.seek(0)
 print '--%s--'%file_size
 file_size = '%s' % file_size
 sock.sendall('SIZE %s\r\n' % file_size)
 elif txt == 'GET_IMG' :
 with open(image, 'rb') as fp:
 image_data = fp.read()
 msg = '%sEOF\r\r' % image_data
 sock.sendall(msg)
 print msg
 except:
 sock.close()
 connected_clients_sockets.remove(sock)
 continue
server_socket.close()

• Mast: I'm assuming you're using Python 2.7. If you're using Python 2.6 instead and are using features that are not available in 2.7 (I don't know all of them by heart), please replace the 2.7 tag by 2.6.
• yak: @Mast: I'm using Python 2.7.12
• Peilonrayz: Welcome to Code Review! Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .

Answer 1

One comment about the recv function: you should check that you indeed received the complete request/reply. For blocking sockets, recv will indeed block until some data is received, but it may return even after only 1 byte has been received. So for example in your client, you might receive something like "SIZE 1" as truncated version of "SIZE 1024\r\n". You'd have to keep recv'ing until you get to the "\r\n", for example by defining a "recvline" function that loops calling recv and appends the data into a result buffer, until "\r\n" is in the result (and perhaps stops after a maximum length is exceeded to avoid huge memory allocations if server has gone crazy :P).

Your commands are pretty short, so this is unlikely to happen unless there's some pretty bad connection issue, but still worth keeping in mind.

It is even possible that you might not detect that with the "EOF" mechanism that you implemented, because it is possible that by pure chance, the image data at the point of truncation contains the string "EOF". Then you'd have received "successfully" a corrupted image.

• yak: Thank you. I tried to follow your advices and modified my code. I also changed EOF to EOIMG. Is it ok now? The only thing that bothers me now, is that when I run the server, and the client (image is copied successfully), I have 100% of CPU used by python. Is it normal?