Sending image through sockets not receiving fine

Question

I'm trying to send a jpg image from a client process to a server via sockets. The image contains binary data so I want to do it on a low-level programming basis using reads and writes. I'm also sending the image data in iterations of 100 bytes.

This is the code I've done, which is not sending the image identically as I want:

CLIENT

void send_image(char *path, char *filename, int socket) {

    int fd = open(path, O_RDONLY); //I open the file of the image.jpg

    int n = 1;

    while (n > 0) {
        char img_data[100];
        n = read(fd, img_data, 100); //sending 100 bytes of image each iteration till n=0 (end of file)

        if (!n) break;

        int sending = 1;
        write(socket, &sending, sizeof(int)); //Tell the client the image still has data to send

        write(socket, img_data, strlen(img_data));

        usleep(250);
    }

    sending = 0; //Tell the server the image has been fully sent
    write(socket, &sending, sizeof(int));

    close(fd);
}

SERVER

void receiving_image(char *path) {

    int receiving = 0;
    int j=0;
    char *image_data = NULL; //Variable to store all the image data

    read(socket, &receiving, sizeof(int)); //Reads that the client is going to send an image

    while (receiving) {

        char data[100]; //Variable that stores partial data (100 bytes) of an image on each iteration 
        read(socket, data, 100);

        image_data = realloc(image_data, (j + strlen(data)) * sizeof(char)); //Readjust the size of the main image data.
      
        for (int i=0; i<(int) strlen(data); i++) {
           
            image_data[j] = data[i]; //copy the partial data of the image to the main variable of the image
            j++;
        }

        j = (int) strlen(image_data); 

        read(socket, &receiving, sizeof(int)); //Read if the image is still sending
    }

    image_to_directory(path, image_data); //Copy image to directory
}

This compiles and runs fine, but when I check the directory on the server side where the image has been stored, I can see it's not the same image as the client has sent (I confirmed via md5sum and hashes are not equal).

Is there something I am missing?

Answer 1

You shouldn't be using strlen for your binary data length calculations. It is intended only for terminated strings (thus the name). You also have highly-ill-advised naked calls to read/write, which is a recipe for disaster when sending data over sockets.

You never seem to be sending more than 100 bytes at a time, which is helpful in this case to develop a more solid protocol. Consider this:

1. First octet is a uint8_t byte count N, and will be in 0..100.
2. Following the byte count, N bytes are transferred.
3. Repeat 1-2 until no more bytes remain.
4. Notify the server of EOF by sending a single zero-octet

An example of this sender code is shown here.

void send_image(const char *path, int socket)
{
    int fd = open(path, O_RDONLY); //I open the file of the image.jpg
    if (fd == -1)
        return;

    ssize_t n = 0;
    do
    {
        // note the first octet will prefix the length
        uint8_t img_data[101];
        n = read(fd, img_data+1, 100);
        if (n > 0)
        {
            // you never know  just how many bytes are going to
            // be sent, so setup the frame, but then ensure even
            // piecewise deliver can succeed.
            img_data[0] = (uint8_t)n;
            ssize_t sent = 0;
            size_t pos = 0;
            do
            {
                sent = write(socket, img_data+pos, (n+1)-pos);
                if (sent < 0)
                    break;
                pos += sent;
            } while ( pos < (n+1) && sent > 0);
        }

    } while (n > 0);

    uint8_t done = 0;
    write(socket, &done, sizeof done); // not much we can do if this fails

    close(fd);
}

I make no claims the above code will even compile, but the concept should be fairly obvious. That's it, however. Obviously there is more that could/should be done (checksums, restart options, etc.), but that's the basic premise.

The server side can do something similar, which I leave as an exercise for you. The point of all of this is to utilize the return values from your read/write calls. They're there for a reason. If you find yourself coding a "naked" read or write (where you don't gather the result of function and utilize it in some way), chances are you've done something horribly wrong.