# Network protocol using TCP, sending images through sockets

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)
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')

data = sock.recv(4096)
if not data :
break

txt = data.strip('\r\n')

if 'EOF' in str(txt) :
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.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, [], [])

if sock == server_socket:

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:
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:

msg = '%sEOF\r\r' % image_data
sock.sendall(msg)
print msg

except:
sock.close()
connected_clients_sockets.remove(sock)
continue

server_socket.close()

• 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. – Mast Mar 2 '17 at 12:34
• @Mast: I'm using Python 2.7.12 – yak Mar 2 '17 at 16:50
• 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. – Peilonrayz Mar 2 '17 at 16:55

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).
• 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? – yak Mar 2 '17 at 16:46