recv function doesn't block and recv some garbage value

Question

The situation: I am making a client-server chat.
Initially it works fine, then I introduced a log-in and register system.
The Problem: I have introduced two features in register:

1. If user able to register successfully, then it is promoted to log-in.
   Problem in this: In server code, After successfully registering the new user I am calling login_check() function. This function first block to receive username from the client. But here recv get some garbage data which I haven't send from the client. To get rid of this, I put one extra recv there which collects this garbage data and ignore it. Now My code for this part works fine but Why I need to put an extra recv there?

2. Case: during registering process, user enter a username which already exists. If this happens I am sending a message(kinda signal) to the client that username already exists. Receiving this the client asks user for again registering or to quit. Then from the client I am sending user's choice back to server. But there in server my recv got some garbage data not the choice I am sending and also it doesn't block(i.e server recv reading garbage data before user enter the choice at client). I have also tried to put some extra recv functions to get rid of garbage data but it doesn't work. So, why recv is not blocking here?

The Source Code: I am providing here the whole source code of server and client. But relevant function to see for my problem are for server: thread_function, rgstr, login_check and for client: registerYourself and login

SERVER CODE

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <error.h>
#include<fcntl.h>

#define PORT "1618" //port we are listening on is a divine port

struct clients{
char name[50];
char password[20];
}client;


//get sockaddr , IPv4 or IPv6:
void *get_in_addr(struct sockaddr *sa)
{
    if (sa->sa_family == AF_INET){
        return &(((struct sockaddr_in*)sa)->sin_addr);
    }
    return &(((struct sockaddr_in6*)sa)->sin6_addr);
}

//need to add one more string argument to show which part is calling this func
void recv_err(int nbytes)
{
    //got an error or connection is closed by client
    if(nbytes == 0) 
    {
        //connection closed
        printf("selectserver: socket hung up\n");
    } 
    else 
    {
        perror("recv");
    }
}

int login_check(int fd)
{
    char data[50],choice[2],*name, *pass;
    int nbytes,ret_status,limit = 50;
    ///////////////////////////////////////   Here Problem        /////////////////////////////////////
    recv(fd,client.name,sizeof(client.name),0);//extra recv as mentioned in 1st doubt
    if(( nbytes = recv(fd,client.name,sizeof(client.name),0)) <= 0) //receiving username
    {
        recv_err(nbytes);
        return 0;
    }
    if(( nbytes = recv(fd,client.password,sizeof(client.password),0)) <= 0) //receiving password
    {
        recv_err(nbytes);
        return 0;
    }
    //opening file containing login info
    FILE * record = fopen("records.txt","r+");
    if(record == NULL)
    {
        //need to write errno to log
        return -1;
    }
    while(fgets(data,100,record)!=NULL)
    {//stored in file in one login info per lineas : username,password
        name = strtok(data,",");
        pass = strtok(NULL,",");
        pass = strtok(pass,"\n");
        if((strcmp(client.name,data)==0) && (strcmp(client.password,pass)==0))
        {
            if(send(fd,"1",2,0) == -1)//signaling client that everythin is fine
            {
                recv_err(-1);
                return 0;
            }
            return 1;
        }
    }
            if(send(fd,"0",2,0) == -1)//if username already exists
            {
                recv_err(-1);
                return 0;
            }
            //waiting for user chooice to enter pass again or not
            if((nbytes = recv(fd,choice,sizeof(choice),0)) <= 0)
            {
                recv_err(-1);
                return 0;
            }
            if(!strcmp(choice,"1"))
            {
                ret_status = login_check(fd);


                if(ret_status == 1)
                    return 1;
                else if(ret_status == 0)
                    return 0;
                else if(ret_status == -1)
                    return -1;      
            }
            else if(!strcmp(choice,"0"))
            {
                return 0;       
            }

    return 0;   

}

int rgstr(int fd)
{
    int nbytes,record_n,ret_status,limit = 50,passed =1;
    char data[50],choice[2], *name, *toBeWrit;

    if((nbytes = recv(fd,client.name,sizeof(client.name),0)) <= 0)//receiving username
    {
        recv_err(nbytes);
        return 0;
    }
    if((nbytes = recv(fd,client.password,sizeof(client.password),0)) <= 0) // receiving password
    {
        recv_err(nbytes);
        return 0;
    }
        //structing data to write in the file
    toBeWrit = (char *)malloc(strlen(client.name)+strlen(client.password)+3);
    strcpy(toBeWrit,client.name);
    strcat(toBeWrit,",");
    strcat(toBeWrit,client.password);
    strcat(toBeWrit,"\n");

    FILE * record = fopen("records.txt","r+");
    if(record == NULL)
    {
        //need to write errno to log
        return -1;
    }
    //checking if username already exists
    while(fgets(data,100,record)!=NULL)
    {
        name = strtok(data,",");
        if(strcmp(client.name,data)==0)
        {
            passed = 0;
            break;
        }
    }   
            //if everything work fine
    if(passed == 1)
    {
        if(send(fd,"1",2,0) == -1)//singaling client
        {
            recv_err(-1);
            return 0;
        }
        fclose(record);

        if((record_n = open("records.txt",O_WRONLY|O_APPEND)) == -1)
        {
            recv_err(-1);
            return 0;
        }
        if((nbytes =write(record_n,toBeWrit,strlen(toBeWrit))) <= 0)
        {
            recv_err(nbytes);
            return 0;
        }

        return 1;
    }
    else//if username already exists
    {
        if(send(fd,"0",2,0) == -1) //singaling client to again ask to user whether to rgstr agian or not
        {
            recv_err(-1);
            return 0;
        }
/////////////////////////////////////            Here the problem              ///////////////////////////////

        //waiting for user chooice to enter pass again or not
        if((nbytes = recv(fd,choice,sizeof(choice),0)) <= 0) // this recv getting garbage data before user select its choice
        {
            recv_err(nbytes);
            return 0;
        }printf("3 %s %d\n",choice,nbytes);

        printf("cc\n");
        if(!strcmp(choice,"1"))
        {
            ret_status = rgstr(fd);

            if(ret_status == 1)
                return 1;
            else if(ret_status == 0)
                return 0;
            else if(ret_status == -1)
                return -1;      
        }
        else if(!strcmp(choice,"0"))
        {
            return 0;       
        }
    }
    return 0;
}

void* thread_func(void* arg)// function that handles each connection
{
    int fd=(int)arg, login_status=0, rgstr_status=0, record_n;
    char choice[2];

    //opening record.txt- creating file
    record_n = open("records.txt",O_CREAT,0644);    
    close(record_n);

    printf("Set to receive the choice:\n");
    recv(fd,choice,sizeof(choice),0);
    printf("Received choice %s\n",choice);      
    if(!strcmp(choice,"1"))
    {
        rgstr_status = rgstr(fd);
        if(rgstr_status == 1)
        {
            printf("New User Registered\n");
            strcpy(choice,"2");
        }
        else if(rgstr_status == 0)
        {
            printf("rUsr close the connection\n");
            close(fd);
            return ((void*)0);
        }
        else if(rgstr_status == -1)
        {
            printf("There is some problem..\n");
            close(fd);
            return ((void*)0);
        }
    }
    if(!strcmp(choice,"2"))
    {
        login_status = login_check(fd);

        if(login_status == 1)
        {
            printf("HI %s\n",client.name);
            while(1);
        }
        else if(login_status == 0)
        {
            printf("lUsr close the connection\n");
            close(fd);
            return ((void*)0);
        }
        else if(login_status == -1)
        {
            printf("There is some problem..\n");
            close(fd);
            return ((void*)0);
        }
    }

    printf("Done with user\n");
    return ((void*)0);
}



int main(void)
{
//  fd_set updated_fds; //main file descriptor list which is updated continously on any change
//  fd_set read_fds; //temp file descriptor list for select() which always asiigned to updated_fds before call to select
//  int fdmax; //The maximum filedescriptor for select

    int listener_fd; //listening the socket descriptor which is listening for new calls
    int new_fd;    //newly accept()ed file descriptor , for each connection

    struct sockaddr_storage clientaddr; // client address is stored in this with a typecasting dependent on family of connection (IPv4 or IPv6)
    socklen_t addrlen;  //length of clientaddr for inet_ntoi

    char buf[256];  //buffer for client - reading buffer
    int nbytes;//return value of recv function

    char remoteIP[INET6_ADDRSTRLEN]; //store the ip of the remote client. Its size is defined as INET6_ADDRSTRLEN because now it can store both IP - IPv4 and IPv6

    int yes=1; // integer passed to setsockopt
    int i, j, rv;

    struct addrinfo hints, *link_struc, *struc_iter;// to use with getaddrinfo

    int err; // pthread_create error
    pthread_t ntid;
    void *a;

//  FD_ZERO(&updated_fds); //clear the updated_fds and temp sets
//  FD_ZERO(&read_fds);

    //get us a socket and bind it
    memset(&hints, 0, sizeof hints);

    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_flags = AI_PASSIVE;

    if((rv = getaddrinfo(NULL, PORT, &hints, &link_struc)) != 0) {
        fprintf(stderr, "selectserver : %s\n",gai_strerror(rv));
        exit(1);
    }

    for(struc_iter = link_struc;struc_iter != NULL ; struc_iter = struc_iter->ai_next) {
        listener_fd = socket(struc_iter->ai_family, struc_iter->ai_socktype, struc_iter->ai_protocol);
        if(listener_fd < 0){
            continue;
        }

        //lose the pesky "address already in use " error message
        setsockopt(listener_fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(int));

        if(bind(listener_fd, struc_iter->ai_addr, struc_iter->ai_addrlen) < 0) {
            close(listener_fd);
            continue;
        }

        break;
    }

    //if we got here, it means we didnot get bound
    if(struc_iter == NULL){
        fprintf(stderr, "selectserver: failed to bind\n");
        exit(2);
    }

    freeaddrinfo(link_struc); // all  done with this

    //listen
    if(listen(listener_fd, 10) == -1){ //make that socket active to listen with max connection queue - 10
        perror("listen");
        exit(3);
    }

    //add the listener_fd to the updated_fds set
//  FD_SET(listener_fd, &updated_fds);

    //keep track of the bigge file descriptor
//  fdmax = listener_fd; // so far it is this one

    //main loop
    for(;;){
        addrlen = sizeof clientaddr;
        new_fd = accept(listener_fd, (struct sockaddr *)&clientaddr, &addrlen);

        if(new_fd == -1){
            perror("accept");
            continue;
        }

        inet_ntop(clientaddr.ss_family, get_in_addr((struct sockaddr *)&clientaddr), remoteIP, INET6_ADDRSTRLEN);
        printf("server : got connection from %s\n", remoteIP);

        a=(void *)new_fd;

        err = pthread_create(&ntid, NULL, thread_func,a );
        if(err != 0)
        {
            printf("can't create thread: %s\n", strerror(err));
        }

        err = pthread_detach(ntid);
        if(err != 0)
        {
            printf("can't detaching thread: %s\n", strerror(err));
        }
    }//END for(;;)-- an you thought it would never end!

    return 0;
}  

Client Code

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <string.h>
#include <netdb.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <sys/socket.h>
#include <arpa/inet.h>

#define PORT "1618" //the port client will be connecting to
#define MAXDATASIZE 100 // max number of bytes we can get at once

static const int BUFFER_SIZE = 16*1024;
//get sockaddr ,IPv4 or IPv6:




void *get_in_addr(struct sockaddr *sa)
{
    if(sa->sa_family ==AF_INET) {
        return &(((struct sockaddr_in*)sa)->sin_addr);
    }

    return &(((struct sockaddr_in6*)sa)->sin6_addr);
}
void registerYourself(int sockfd)
{
    char username[50], password[50];
    printf("You have chosen to register yourself\n");
    printf("Please enter a username and password\n");
    printf("Username:");
    scanf("%s",username);
    //printf("You entered %s\n",username);
    send(sockfd,username,sizeof(username),0); // sending username
    printf("\nPassword:");
    scanf("%s",password);
    //printf("-- %s\n",password);
    send(sockfd,password,sizeof(password),0);       //sending password
}

void login(int sockfd)
{
    char username[50],password[50];
    printf("You have chosen to login yourself\n");
    printf("Please enter a username and password\n");
    printf("Username:");
    scanf("%s",username);
//  printf("-- %s\n",username);
    send(sockfd,username,sizeof(username),0);//send username
    printf("\nPassword:");
    scanf("%s",password);
    //printf("-- %s\n",password);
    send(sockfd,password,sizeof(password),0);//send password
}


int main(int argc, char *argv[])
{
    char fname[50], s[INET6_ADDRSTRLEN], buf[MAXDATASIZE], username[50], password[50], registerStatus[2], login_status[2];
    int sockfd, numbytes,fp,i,rv, choice;

    struct addrinfo hints, *servinfo, *p;

    if(argc != 2) 
        {
            fprintf(stderr,"usage: client hostname\n");
            exit(1);
        }

        memset(&hints, 0, sizeof hints);
        hints.ai_family = AF_UNSPEC;
        hints.ai_socktype = SOCK_STREAM;

        if((rv = getaddrinfo(argv[1], PORT, &hints, &servinfo)) != 0) 
        {
            fprintf(stderr,"getaddrinfo: %s\n",gai_strerror(rv));
            return 1;
        }

        //lopp through all the results and connect to the first we can
        for(p = servinfo; p != NULL; p = p->ai_next) 
        {
            if((sockfd = socket(p->ai_family, p->ai_socktype, p->ai_protocol)) == -1){
                perror("client: socket");
                continue;
            }

            if(connect(sockfd, p->ai_addr, p->ai_addrlen) == -1)
            {
                close(sockfd);
                perror("client: connect");
                continue;
            }
            break;
        }

        if(p ==NULL) 
        {
            fprintf(stderr,"client: failed to connect\n");
            return 2;
        }

        inet_ntop(p->ai_family, get_in_addr((struct sockaddr *)p->ai_addr), s, sizeof s);
        printf("client : connecting to %s\n", s);

        freeaddrinfo(servinfo); // all done with this structure

        printf("\n----------------Menu-------------------------\n1.Register\n2.Log in\n");
        scanf("%d",&i);

        switch(i)
        {
            case 1: 
                send(sockfd,"1", 2, 0);//sending server that user want to register
                registerYourself(sockfd);

                recv(sockfd,registerStatus,sizeof(registerStatus),0);//receiving from server that register is successful or not
                if(strcmp(registerStatus,"1")==0) //If register successful then promoting for log-in
                {
                    login(sockfd);
                    break;
                }
                else if(strcmp(registerStatus,"0")==0)// Asking user what to do
                {
                    printf("Username already taken...Please try again with other username...\nPress:\t 1 to try again\n\t2 to quit");
                    scanf("%d",&choice);
                    switch(choice)
                    {
                        case 1:
                            send(sockfd,"1", 2, 0);
                            login(sockfd);
                            break;
                        case 2:
                            send(sockfd,"0",2,0);
                            return;
                    }           
                }

            case 2:
                send(sockfd,"2", 2, 0); //sending server that user want to login
                login(sockfd);
                recv(sockfd,login_status,sizeof(login_status),0); // //receiving from server that login is successful or not
                if(strcmp(login_status,"1")==0)
                {
                    printf("login successfully\n");
                    while(1);// doing other job here
                }
                else if(strcmp(login_status,"0")==0)
                {
                    printf("Wrong Username And/Or Password\nPress:\n__1)__To Enter Again  |  __2)__To Quit");
                    scanf("%d",&choice);
                    switch(choice)
                    {
                        case 1:
                            send(sockfd,"1", 2, 0);// sending that user want to login again
                            login(sockfd);
                            break;
                        case 2:
                            send(sockfd,"0",2,0);// sending that user want to stop
                            return;
                    }
                }
                break;//case 2 ends
        }//switch ends

        while(1); //doing other job
        close(sockfd);

        return 0;
}

Answer 1

TCP is a "streaming" protocol and has no concept of message boundaries, so when you have code like this:

void login(int sockfd)
{
    char username[50],password[50];
    ...
    scanf("%s",username);
    send(sockfd,username,sizeof(username),0);//send username
    ...
    scanf("%s",password);
    send(sockfd,password,sizeof(password),0);//send password
}

You are trying to send 50 bytes twice, with no indication where the username ends and the password begins and causing problems with recv later.

Read the documentation of send carefully. send returns the number of bytes successfully sent, which can be < 50. In simple testing this may work, but during high network bandwidth send can and will send less than requested. It is your responsibility to check the return value and send the rest of the data if it wasn't transmitted successfully.

Another problem is you are sending the entire username and password buffer, so if the user types "Mark" and "123" as the username and password, you'll still send 100 total bytes.

On the server side your buffers aren't the same size:

struct clients{
char name[50];
char password[20];
}client;

So when the data is read:

recv(fd,client.name,sizeof(client.name),0);//extra recv as mentioned in 1st doubt
...
if(( nbytes = recv(fd,client.password,sizeof(client.password),0)) <= 0) //receiving password

50 bytes, then 20 bytes are attempted to be read, leaving 30 bytes unread...the source of your extra data when calling recv.

Note that recv also has no concept of message boundaries and can return less than the amount of data requested, and it can contain the last part of one send and the first part of another. It is your responsibility to define a protocol that buffers data until you have a complete message.

Since you seem to be always sending nul-terminated strings, one algorithm would be:

1. Maintain a recv buffer and current length count.
2. When data is needed, check recv buffer for a nul-termination byte.
3. If no nul byte, issue a large recv request, say 4K bytes, else goto #5.
4. Add data received to recv buffer and go back to #3.
5. Remove data from buffer up to nul, leaving any remaining in buffer to satisfy next request.

Also make sure to send only the string and the nul-termination byte, not the entire buffer, and check the return value of send. If you send 100 bytes and send returns 50, call send again with a pointer to the remaining 50 bytes...repeat until all the data is sent.