Unix socket programming in C++, recv returning 0, but still receiving data, but sometimes receives more than what is sent

Question

I am new to C++ and socket programming. I studied with Beej's guide so my codes are almost same as the guide, but I am struggling really strange bugs.

First, my server's recv() returns 0. According to document, the client should gracefully close the connection for recv() to return 0. Not really in my case. It returns 0, at the same time, I still receive the data from the client. So, the way Beej's do to receive, does not work for me. Can someone explain how this can be possible?

char buf[MAXDATASIZE];
numbytes = recv(new_fd, buf, MAXDATASIZE-1, 0); 
buf[numbytes] = '\0';

the last line here, because numbytes is 0, it sweeps out all message I received. So I had to comment that out. Now, my code looks like this

char buf[MAXDATASIZE];
numbytes = recv(new_fd, buf, MAXDATASIZE-1, 0); 
//buf[numbytes] = '\0';
printf("received: %s\n", buf); 

It now works with receiving some messages sent by client. However, I did some string manipulation (appending) in the client side, and then sent the message. Now, I send string length of 29 in the client side, but the server receives 41 bytes with strange characters. What I sent: received: Login#1 Mary 123456 451912345 received: Login#1 Mary 123456 451912345ÿ>É„ÿy@ÿ>Ád

Here is how I receive in the server:

while(1) { // main accept() loop

    new_fd = accept(sockfd, (struct sockaddr *)&their_addr, &sin_size);
    if (new_fd == -1) {
        perror("accept");
        continue;
    }
    char buf[MAXDATASIZE];
    int numbytes;
    if (numbytes = recv(new_fd, buf, MAXDATASIZE-1, 0) == -1) 
    perror("recv");
    //buf[numbytes] = '\0'; // this had to be commented out
    printf("received: %s\n", buf); // prints out with weird characters

    string msgRcved = buf;

        close(new_fd); 
}

This is how I send from client:

// string loginCredential is loaded with "1 Mary 123456 451912345" at this point
loginCredentials.insert(0, "Login#");
const char* msgToSend = loginCredentials.c_str();
int numbytesSent;
if (numbytesSent = send(sockfd, msgToSend, strlen(msgToSend), 0) == -1)
    perror("send");

I'd like to know how my recv receives data while it returns 0 at the first place. And, I'd like to know what I am doing wrong to recv data from client/send data to server.

Answer 1

You have a precedence problem.

This:

if (numbytes = recv(new_fd, buf, MAXDATASIZE-1, 0) == -1) 

is equivalent to

if (numbytes = (recv(new_fd, buf, MAXDATASIZE-1, 0) == -1))

and

recv(new_fd, buf, MAXDATASIZE-1, 0) == -1

is 0 whenever recv succeeds.

The same problem is present on the sending end.

There's no reason to write such awkward and error-prone condition.
This is safer:

int numbytes = recv(new_fd, buf, MAXDATASIZE-1, 0);
if (numbytes == -1) 
    perror("recv");

Answer 2

You have to test 'numbytes' for zero, separately, and if you get it close the socket and exit the read loop, because the peer has closed the connection. Otherwise, and assuming you have also tested for -1, you have to only process 'numbytes' bytes of the buffer. Not all of them. Otherwise you're liable to reprocess bytes you already processed. In this case that might mean restoring the line that null-terminated the buffer, or it might mean this:

printf("%.*s", numbytes, buf);

Answer 3

You are printing whatever garbage was in that stack-allocated buffer, not what the client sent. When recv(2) returns zero, nothing has been placed into the supplied buffer, so this is probably from some previous iteration of the loop.

Notes:

1. Connected TCP socket is is a bi-directional stream of bytes. This means you might send several of your "messages" and receive them in one chunk on the other side, or the other way around. Read from the socket in a loop until you have enough data to process, i.e. use explicit message separators, or pre-pend a length of your message that follows. This is your application-level protocol.

2. Don't mix C and C++ string handing like this. std::string has a size() method, use it instead of doing strlen( msgToSend.c_str() ).

3. Allocating any sizable buffers on the stack, especially ones receiving input from the network is a bad idea.

4. Printing, or otherwise passing further, unverified network input is a gross security violation leading to all sorts of problems.

Edit 0:

@molbdnilo's answer is the right one. I did not spot the precedence problem in the conditionals. My notes still apply though.