Sending Images over C++ Sockets (Linux)

Question

I am trying to send files over sockets. I created a program and it works for file types such as .cpp, .txt, and other text files. But binary files, images( .jpg, .png) and compressed files such as .zip and .rar are not being sent properly. I know it is not something to do with the size of the files because i tested with large .txt files. I do not know the problem, i am receiving all the bytes being sent but the file can not be opened. Most times the file is corrupt and cannot be viewed. I've searched through google for a solution and just found others with the same problem and no solution. So by helping me you are also helping anyone else in need of a solution.

Server Code:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <unistd.h>

int main ( int agrc, char *argv[] )
{
    /******** Program Variable Define & Initialize **********/
    int Main_Socket;    // Main Socket For Server
    int Communication_Socket; // Socket For Special Clients
    int Status; // Status Of Fucntion
    struct sockaddr_in Server_Address; // Address Of Server
    struct sockaddr_in Client_Address;// Address Of Client That Communicate with Server
    int Port;
    char Buff[100] = "";
    Port = atoi(argv[2]);
    printf ("Server Communicating By Using Port %d\n", Port);
    /******** Create A Socket To Communicate With Server **********/
    Main_Socket = socket ( AF_INET, SOCK_STREAM, 0 );
    if ( Main_Socket == -1 )
    {
            printf ("Sorry System Can Not Create Socket!\n");
    }
    /******** Create A Address For Server To Communicate **********/
    Server_Address.sin_family = AF_INET;
    Server_Address.sin_port = htons(Port);
    Server_Address.sin_addr.s_addr = inet_addr(argv[1]);
    /******** Bind Address To Socket **********/
    Status = bind ( Main_Socket, (struct sockaddr*)&Server_Address, sizeof(Server_Address) );
    if ( Status == -1 )
    {
            printf ("Sorry System Can Not Bind Address to The Socket!\n");
    }
    /******** Listen To The Port to Any Connection **********/        
    listen (Main_Socket,12);    
    socklen_t Lenght = sizeof (Client_Address);
    while (1)
    {
        Communication_Socket = accept ( Main_Socket, (struct sockaddr*)&Client_Address, &Lenght );

        if (!fork())
        {

            FILE *fp=fopen("recv.jpeg","w");
            while(1)
            {
                char Buffer[2]="";
                if (recv(Communication_Socket, Buffer, sizeof(Buffer), 0))
                {
                    if ( strcmp (Buffer,"Hi") == 0  )
                    {
                        break;
                    }
                    else
                    {
                        fwrite(Buffer,sizeof(Buffer),1, fp);
                    }
                }
            }
            fclose(fp);
            send(Communication_Socket, "ACK" ,3,0);
            printf("ACK Send");
            exit(0);
        }
    }
    return 0;
}

Client Code:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <unistd.h>

int main ( int agrc, char *argv[] )
{
    int Socket;

    struct sockaddr_in Server_Address;  
    Socket = socket ( AF_INET, SOCK_STREAM, 0 );
    if ( Socket == -1 )
    {   
        printf ("Can Not Create A Socket!");    
    }
    int Port ;
    Port = atoi(argv[2]);   
    Server_Address.sin_family = AF_INET;
    Server_Address.sin_port = htons ( Port );
    Server_Address.sin_addr.s_addr = inet_addr(argv[1]);
    if ( Server_Address.sin_addr.s_addr == INADDR_NONE )
    {
        printf ( "Bad Address!" );
    }   
    connect ( Socket, (struct sockaddr *)&Server_Address, sizeof (Server_Address) );


    FILE *in = fopen("background.jpeg","r");
    char Buffer[2] = "";
    int len;
    while ((len = fread(Buffer,sizeof(Buffer),1, in)) > 0)
    {            
        send(Socket,Buffer,sizeof(Buffer),0);            
    }
    send(Socket,"Hi",sizeof(Buffer),0);

    char Buf[BUFSIZ];
    recv(Socket, Buf, BUFSIZ, 0);
    if ( strcmp (Buf,"ACK") == 0  )
    {
         printf("Recive ACK\n");
    }        
    close (Socket);
    fclose(in);
    return 0;   
}

Answer 1

For reading ascii text files, a char buffer is acceptable.

For reading binary data, you will need to use unsigned char, otherwise your data is going to get hosed, as binary data is unsigned bytes.

Answer 2

Firstly, you should try increasing your buffer to something bigger. The bigger the buffer, the more efficient the transport is (just don't exaggerate and consume too much local memory).

Secondly, you should check and use the lengths returned by the read and write functions. In all your reads and writes you only check if something was read/written, but you should use this byte count on your next write/read operation. For example, if the client reports that it has read 1 byte, you should only write 1 byte to disk. Also, if you have 1024 bytes read from the server, you should try to write 1024 bytes to disk, which might not happen in that call and you might need another call to the write to complete the operation.

I know this sounds like a lot of work, but that's how it has to be done to guarantee I/O ops. All reads and write have basically to be done inside their own loops.

Answer 3

Do you know for certain that your binary image file has no byte sequence 0x48 0x69 ("Hi") in it? Your read loop will terminate as soon as it receives that sequence. Additionally, you call strcmp() with a character buffer that is two bytes long and almost certainly guarantees that it has no null terminating byte ('\0').

You might also investigate how the files differ? The cmp tool can give you a list of bytes that are different between the source and destination.

For correctness you definitely want to check the return results from read(), write(), send(), and so on. The possibility of short reads and writes with sockets is very high, so it is vitally important that your code be able to handle the cases where not all the data are transfered. Since you don't track how many bytes are returned by recv(), it is possible that you only received a single byte, but wrote two bytes in the write() call following it.

For performance a larger buffer will help reduce the overhead of the system calls for moving the data around, although it is acceptable to do smaller operations if bandwidth and latency are not primary concerns. And they shouldn't be until you have correct behaviour.

Lastly, for compatibility with less enlightened operating systems you should open the files in binary mode with "rb" and "wb" so that newlines are not mangled during the write.

Answer 4

The problem is that you're opening the files in text mode with fopen(..., "r") and fopen(..., "w"). You need to use binary mode ("rb" and "wb") for non-text files.

Answer 5

Others pointed out problems with your code, namely:

• not using return values of read(2) and write(2) calls,
• mixing binary data and character control data,
• using strcmp(3) on strings that might not be zero-terminated (let me note that using functions relying on terminating zero on data received from the network is generally not a good idea, and often leads to buffer overruns.)

You would be much better off defining simple protocol for file transfer (read the "The ultimate SO_LINGER page, or: why is my tcp not reliable" if you want to know why.) Let the server side know upfront how much data you are sending - precede the transfer with a fixed-size header containing the file length (that might also include the file name, but then you would need to transfer the length of that name too.) Pay attention to number endianness - always send numbers in network byte order.

Since you are on Linux, let me also point you to sendfile(2), which is a very efficient way of sending files since it avoids copying data to/from userland.