Not sending file correctly. Sockets. C

Question

I am writing a program that a client can ask for files to a server. Then the server will send them in chunks of 512 bytes. The problem is that when the client read the file:

*Sometimes the first 512 bytes are different from the original file. The total read file also has a different size (and obviously it also ends different from the original file) and therefore the client loop that writes to the new file does never end.

*Sometimes it works perfectly and i don't know why.

Server:

            /* Check if file exists */
            if(access(retrFileName, F_OK) == 0){

                /* Open file */
                fd = open(retrFileName, O_RDONLY); 
                lseek(fd, 0, SEEK_SET);
                if (fd == -1){
                        fprintf(stderr, "Error opening file --> %s", strerror(errno));

                        exit(EXIT_FAILURE);
                }

                /* Get file stats */
                if (fstat(fd, &fileStat) < 0){
                        fprintf(stderr, "Error fstat --> %s", strerror(errno));
                        exit(EXIT_FAILURE);
                }

                sprintf(fileSize, "%li", fileStat.st_size);

                /* Sending file data */
                offset = 0;
                remainData = fileStat.st_size;
                while (((sentBytes = sendfile(clientSock, fd, &offset, 512)) == 512) && (remainData > 0)) {
                        remainData -= sentBytes;
                        fprintf(stdout, "Server envio %d bytes del file, offset ahora vale: %li y quedan = %d bytes\n", sentBytes, offset, remainData);
                }
                remainData -= sentBytes;
                fprintf(stdout, "Server envio %d bytes del file, offset ahora vale: %li y quedan = %d bytes\n", sentBytes, offset, remainData);//do while
                close(fd);////////////////////////
                send(clientSock, NICETRANSFER, sizeof(NICETRANSFER), 0); //LO METE AL ARCHIVO
                printf("send\n");
                //close(clientSock);///////////

            }
            else{
                send(clientSock, FILEERROR, sizeof(FILEERROR), 0);
                printf("send\n");
            }


        }

Client:

/* Open file */
            receivedFile = fopen("r.txt", "wb");
            if (receivedFile == NULL){
                fprintf(stderr, "Failed to open file --> %s\n", strerror(errno));

                exit(EXIT_FAILURE);
            }

            /* Write to the file */
            int contador = 0;
            int remainData = fileSize;
            do{
                if(remainData < 512)
                    bytesLeidos = recv(clientSock, readingBuffer, remainData, 0);
                else
                    bytesLeidos = recv(clientSock, readingBuffer, 512, 0);

                fwrite(readingBuffer, bytesLeidos, 1, receivedFile);

                remainData -= 512;
                contador += 512;
                printf("bytesleidos: %li, contador: %d:\n%s\n\n", bytesLeidos, contador, readingBuffer);

            }while(contador < fileSize);
            fclose(receivedFile);

`/* Check if file exists */ if(access(retrFileName, F_OK)` is completely useless. Even if the `access()` call succeeds, the later `open()` call can fail. Doing X to try to make sure Y will work later is almost always a bad idea - for one, it's [a TOCTOU bug](https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use). Just call `open()` and handle any failure - you have to do that anyway. — Andrew Henle, Apr 25 '18 at 16:26
Why are you looping while `sendfile()` returns 512? It can return *any* positive number <= the stated buffer size. — user207421, Apr 26 '18 at 02:16
we will have a difficult time recreating your bug, because all that is posted is code snippets. Please post a [mcve] for the client and for the server — user3629249, Apr 26 '18 at 18:59

selbie · Answer 1 · 2018-04-26T01:53:47.433

4

Golden rule of socket programming: Always check the return value from recv. It's not always what you think it will be.

Even though you "send" 512 bytes at a time, you are in no way guaranteed that TCP will deliver the same number of bytes at a time to the receiver. TCP segmentation, IP fragmentation, and general Internet weirdness will cause the recv side to get an arbitrary number of bytes at a time.

Hence, your hardcoded assumption that recv will always return 512 is incorrect:

remainData -= 512;
contador += 512;

Instead, you should be saying:

remainData -= bytesLeidos;
contador += bytesLeidos;

An you need to check for errors and socket closing too.

This is an improved main loop for your client code:

while (remainData > 0)
{
    size_t recvSize = (remainData >= 512) ? 512 : remainData;
    bytesLeidos = recv(clientSock, readingBuffer, recvSize, 0);
    if (bytesLeidos > 0)
    {
        fwrite(readingBuffer, bytesLeidos, 1, receivedFile);
        remainData -= bytesLeidos;
        contador += bytesLeidos;

        /* null terminate readingBuffer so garbage isn't printed.*/
        /* Make sure readingBuffer is allocated to be at least */
        /*  N+1 bytes (513) to account for this character being appended. */

        readingBuffer[bytesLeidos] = '\0'; 
        printf("bytesleidos: %li, contador: %d:\n%s\n\n", bytesLeidos, contador, readingBuffer);
    }
    else if (bytesLeidos == 0)
    {
        /* remote side closed connection */
        printf("Remote side exited connection\n");
        break;   
    }
    else if (bytesLeidos < 0)
    {
         /* connection error */
        printf("Connection error\n");
        break;   
    }
}

edited Apr 26 '18 at 01:53

answered Apr 25 '18 at 16:20

selbie

100,020
15
103
173

i am having the same problem with your solution. When i go `printf("bytesleidos: %li, contador: %d:\n%s\n\n", bytesLeidos, contador, readingBuffer);` for the first time, it prints something different from the beginning of the original file. Also, the loop don't end; the client output ends with __bytesleidos: 103, contador: 2151:__ (and my original file size is 3062) Also, the server (or the client idk) write into the new file the message sent after sending(recieving) the file – boludo kid Apr 25 '18 at 18:13
My psychic powers suggest that are you forgetting to null terminate `readingBuffer` with a zero. Literally add this line `readBuffer[bytesLeidos] = '\0';` before the `printf` statement. – selbie Apr 26 '18 at 01:48
No. Change the `printf()` to `printf("bytesleidos: %li, contador: %d:\n%.*s\n\n", bytesLeidos, contador, bytesLeidos, readingBuffer)`. You should not mess with the read buffer. – user207421 Apr 26 '18 at 02:16
The printf statement change is a good idea. But null terminating the read buffer also makes debugging easier - especially for text protocols. We're already programming in "C", so the safety belts are already off... – selbie Apr 26 '18 at 06:07
Still having the problem. When i boot my computer the program works perfectly. The problem is after the first try, the client (i guess) don't recieve the full file correctly – boludo kid Apr 27 '18 at 20:06
What have you done to debug to understand the issue? Do you think the issue is on server or client? – selbie Apr 27 '18 at 20:30

score 0 · Accepted Answer · answered May 16 '18 at 16:41

I solved my problem!! I needed to sync both client and server. To do so, the server send the size of the file and waits for an answer for the client with recv. When the client recieve the file size, it send a "" message.

I don't know if this is the correct solution, but this way you can sync server and client.

After sync, the server send the respective file normally with sendfile

Not sending file correctly. Sockets. C

2 Answers2