Receiving strange characters/symbols in winsock

Im learning about Winsock and Im having a strange issue when sending and receiving a simple string. Here's my code (pure C):

Client:



//...
//Declarations and stuff

//----------- SEND SOME DATA -------------------------------------------------

    char string1[] = "string-1";
    int bytes_sent = 0;

    bytes_sent = send(client_socket, string1, strlen(string1), 0);  

    printf("BYTES SENT: %i\n", bytes_sent);
    printf("\n-----------------------------------------------\n\n");

    system("pause");

//...

Server:



//...
//Declarations and stuff

//----------- START LISTENING FOR REQUESTS ------------------------------------

    SOCKET ClientSocket;

    #define BUFFER_SIZE 256

    int size;
    struct sockaddr_in client_info;
    char client_ip[16];
    char data_received[BUFFER_SIZE];    
    int bytes_received = 0; 

    listen(ListenSocket, SOMAXCONN);

    while(1){           

        ClientSocket = accept(ListenSocket, (struct sockaddr *)&client_info, &size);        
        strcpy(client_ip, inet_ntoa(client_info.sin_addr));     

        do{

            bytes_received = recv(ClientSocket, data_received, BUFFER_SIZE, 0);

            if(bytes_received > 0){
                printf("DATA RECEIVED FROM %s: %s (%i bytes)\n", client_ip, data_received, bytes_received);
            }


        }while(bytes_received > 0);

        printf("\n-----------------------------------------------\n\n");


    }

//...

The problem is that the server prints my string + some strange symbols (see the pic).

Strange symbols

Im using a stream socket. The example is very simple so I dont know what could be wrong. The problem disappears (server prints OK the string) if I randomly modify the string, or the server's buffer size, or both. The problem fixes if in the send() call i use sizeof() instead of strlen(). Im a little bit lost here. Please be kind if i have missed something, this is my very first post here. I can provide the whole code (its basically the winsock start and the socket definition) .

Solution

The data you send does not contain a terminating null character:

bytes_sent = send(client_socket, string1, strlen(string1), 0);

...because strlen does not count the terminating null. This isn't exactly the problem in itself, but rather is coupled with the fact that on the receiving side:

char data_received[BUFFER_SIZE];
// ...
bytes_received = recv(ClientSocket, data_received, BUFFER_SIZE, 0);

data_received is not initialized, and you can receive up to BUFFER_SIZE bytes. This means that since the data you send is not null-terminated:

If bytes_received < BUFFER_SIZE, the rest of data_received may not be initialized so accessing/printing would be undefined behavior. It's actually not 100% clear, as the documentation says:

[...] calling recv will return as much data as is currently available—up to the size of the buffer specified [...]

...so it may mean that the rest of the buffer is left untouched.
If bytes_received == BUFFER_SIZE, there is no null-terminator, so printf will invoke undefined behavior by trying to print it, since it doesn't know where the string stops and will overrun the array.

The easiest ways to fix these are either to send a null terminator:

bytes_sent = send(client_socket, string1, strlen(string1)+1, 0); // +1 here
bytes_sent = send(client_socket, string1, sizeof(string1), 0);   // same as above

...or receive a byte less and put the null terminator on the receiving size:

bytes_received = recv(ClientSocket, data_received, BUFFER_SIZE-1, 0); // -1 here
data_received[bytes_received] = 0;

I'd personally go with the first.