CODEWITHSUNDEEP
c++windowssockets
Invictus
Invictus

Reputation: 4338

Garbage value received in client server communication c++

I was trying to write a simple client server program on windows. My Server side code looks like below .

#include <iostream>
#include <windows.h>
#include <winsock2.h>
#include <stdlib.h>
#include <stdio.h>

using namespace std;

int main()
{
    WSADATA WSAData;
    SOCKET serverSock, clientSock;
    SOCKADDR_IN serverAddr, clientAddr;

    WSAStartup(MAKEWORD(2,0),&WSAData);
    serverSock=socket(AF_INET,SOCK_STREAM,0);

    serverAddr.sin_addr.s_addr=INADDR_ANY;
    serverAddr.sin_family=AF_INET;
    serverAddr.sin_port=htons(5555);

    bind(serverSock,(SOCKADDR *)&serverAddr, sizeof(serverAddr));
    listen(serverSock,0);
    std::cout<<"Listening for incoming connections \n";

    char buffer[1024];
    //memset(buffer,0,sizeof(buffer));
    int clientAddrsize= sizeof(clientAddr);
    if(clientSock = ( accept(serverSock,(SOCKADDR *)&clientAddr, &clientAddrsize ) != INVALID_SOCKET) )
    {
        std::cout<<"Client connected \n";
        recv(clientSock,buffer,int(strlen(buffer)),0);
        std::cout<<"Client Says: " << buffer <<"\n" ;
        Sleep(10000);
        //memset(buffer,0,sizeof(buffer));
        closesocket(clientSock);
        closesocket(serverSock);
        WSACleanup();
    }
    return 0;
}

Client side code looks like:

#include <iostream>
#include <winsock2.h>
#include <stdlib.h>
#include <stdio.h>

using namespace std;

int main()
{
    WSADATA WSAData;
    SOCKET clientSock;
    SOCKADDR_IN addr;

    WSAStartup(MAKEWORD(2,0),&WSAData);
    clientSock=socket(AF_INET,SOCK_STREAM,0);

    addr.sin_addr.s_addr=inet_addr("127.0.0.1");
    addr.sin_family=AF_INET;
    addr.sin_port=htons(5555);

    connect(clientSock,(SOCKADDR *)&addr,sizeof(addr));
    std::cout <<"Connected to the server \n";
    //memset(buffer,0,sizeof(buffer));
    char buffer[1024]={'h','e','l','l','o','.','\0'};
    send(clientSock,buffer,int(strlen(buffer)),0);
    std::cout<<"Message sent \n" ;
    Sleep(10000);
    closesocket(clientSock);
    WSACleanup();
    std::cout<<"Socket closed \n";
    return 0;
}

I see below output on server side:

 Listening for incoming connections
 Client connected
 Client Says: R☺

What i was expecting was Client Says: Hello. . Not sure why i am getting the garbage value.

Thanks

Upvotes: 1

Views: 399

Answers (1)

Sam Varshavchik
Sam Varshavchik

Reputation: 118445

char buffer[1024];

This declares a local char array in automatic scope. The array is not initialized anything, and contains random garbage.

recv(clientSock,buffer,int(strlen(buffer)),0);

This passes buffer to strlen. However, buffer contains garbage, this is undefined behavior, with the eventual results could be, randomly, anything between an immediate crash, at this point, or some bizarre return value from strlen.

Additionally, the return value of recv is completely ignored and not checked at all.

All socket system calls can fail, and every socket system call, send, recv, and all others can fail. Additionally, even if it succeeds, recv gives you no guarantees whatsoever that everything that was send in one shot, will be recved, in one shot.

If, for example, send sent 10 bytes, the first call to recv might return 1 or 5, indicating that only the first one or five bytes were immediately recved. You are not guaranteed that recv will receive those 10 bytes all at once. Otherwise, recv will (obviously) need to be called again to receive the rest of the message.

You will need to fix all of these issues in order to have a reliable client/server application:

  1. Check the return value of all system calls. Everything that I said about recv also applies to send as well. An attempt to send 10 bytes might come back with, say, 7, indicating that only the first 7 bytes were sent, and you must then implement the appropriate logic to send the remaining bytes, too.

  2. Fix undefined behavior, of passing an uninitialized value to strlen.

Upvotes: 1

Related Questions