Socket Programming -- recv() is not receiving data correctly

Question

I have looked at the similar threads but can't seem to find anything that could solve my problem.

I am programming a server that could send an image(jpg) file from the path sent to it from the client. I am using send/recv functions in C for that.

I read files one chunk of data at a time and send it to the client that receives the contents and writes them at some location to build the file.

Problem is 'recv' doesn't receive the number of bytes sent by the 'send'.

As part of debugging, I have tried different buffer sizes and '128' buffer size doesn't give me any problem, and the file is successfully transferred and built.

However, for '32' and '64' bit buffers, 'recv' receives '32' bit or '64' bit data at the last chunk, even though the data sent by the server is less than either '32' bit or '64' bit. And, for '256', '512', '1024' so on, 'recv' returns ONLY '128' bits at EXACTLY one of the responses, even though the server sends complete chunk, i.e. '256' or'512', depending on the buffer size.

I'll appreciate any advise for debugging. Following code is for the relevant parts only, but I can provide more, should someone requires it.

//Client Code

#define BUFFER_SIZE 4096

#define   HEADER_LEN 512

const char * const scheme = "GETFILE";
const char* const method = "GET";
const char * const end_marker = "\\r\\n\\r\\n";

struct gfcrequest_t
{
    int filelen;
    char cport[12];
    char servIP[50];
    gfstatus_t ret_status;
    char spath[1024];
    int tot_bytes;
    char filecontent[BUFFER_SIZE];

    void (*fl_handler)(void *, size_t, void *);
    void * fDesc;

    void (*head_handler)(void *, size_t, void *);
    void * headarg;

};

static pthread_mutex_t counter_mutex;

gfcrequest_t *gfc;

// 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);
}

static char *stringFromError(gfstatus_t stat)
{
    static const char *strings[] = {"GF_OK", "GF_FILE_NOT_FOUND", "GF_ERROR", "GF_INVALID"};

    return strings[stat];
}


int getFileRequestHeader(char * req_header)
{
    return snprintf(req_header,HEADER_LEN, "%s%s%s%s", scheme, method,gfc->spath, end_marker);
   // return snprintf(req_header,HEADER_LEN, "%s%s%s%s", scheme, method,"/courses/ud923/filecorpus/yellowstone.jpg", end_marker);
}


gfcrequest_t *gfc_create()
{

    gfc = (gfcrequest_t*)malloc(1* sizeof(gfcrequest_t));

    return gfc;
}

void gfc_set_server(gfcrequest_t *gfr, char* server)
{
    strcpy(gfr->servIP, server);
}

void gfc_set_path(gfcrequest_t *gfr, char* path)
{
    strcpy(gfr->spath, path);
}

void gfc_set_port(gfcrequest_t *gfr, unsigned short port)
{
    snprintf(gfr->cport,12, "%u",port);
}

void gfc_set_headerfunc(gfcrequest_t *gfr, void (*headerfunc)(void*, size_t, void *))
{

    gfr->head_handler = headerfunc;

}

void gfc_set_headerarg(gfcrequest_t *gfr, void *headerarg)
{
/*have to change this...*/
    gfr->headarg = headerarg;

}

int isEndMarker(char *iheader, int start)
{
    char *marker = "\\r\\n\\r\\n";
    int i = 0; int ind=0;


    while (ind <= 7)
    {
        if (iheader[start++] == marker[ind++])
        {
            continue;
        }

        return 0;
    }

    return 1;

}

int getFileLen(char *resp_header, gfcrequest_t *gfr)
{
    char scheme[8];
    char status[4];
    int istatus;
    char filelen[12];

    int contentlen = 0;

    int fileindex = 0;
    char end_marker[12];
    int fexit=0;
    int end=0;

    sscanf(resp_header, "%7s%3s", scheme, status);


    istatus = atoi(status);

    if (istatus == 200)
    {
        gfr->ret_status = GF_OK;
    }
    else if (istatus == 400)
    {
        gfr->ret_status = GF_FILE_NOT_FOUND;
    }
    else if (istatus == 500)
    {
        gfr->ret_status = GF_ERROR;

    }

    if (!strcmp(scheme, "GETFILE") && (istatus == 200 || istatus == 400 || istatus == 500))
    {
        int index = 10;
        while(1)
        {

            if (resp_header[index] == '\\')
            {
                end = isEndMarker(resp_header, index);
            }

            if (end)
                break;

            filelen[fileindex++] = resp_header[index++];

        }

        filelen[fileindex] = '\0';
    }

    int head_len = strlen(scheme) + strlen(status) + strlen(filelen) + 8;

    return atoi(filelen);

}

void gfc_set_writefunc(gfcrequest_t *gfr, void (*writefunc)(void*, size_t, void *))
{

    gfr->fl_handler = writefunc;

}

void gfc_set_writearg(gfcrequest_t *gfr, void *writearg)
{
    gfr->fDesc = writearg;
}

int gfc_perform(gfcrequest_t *gfr){

    struct addrinfo hints, *servinfo, *p;

    int sockfd, rv, totalBytesRcvd;
    char req_header[HEADER_LEN];

    int bytesRcvd;

    int header_len;

    char s[INET6_ADDRSTRLEN];

    memset(&hints, 0, sizeof(hints));
    memset(req_header,0,sizeof(req_header));

    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_flags = AI_PASSIVE; //use my IP

    if ((rv = getaddrinfo(NULL, gfr->cport, &hints, &servinfo)) != 0)
    {
        fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(rv));
        return 1;
    }

    // loop 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;
    }

    //printf("connected...\n");

    inet_ntop(p->ai_family, get_in_addr((struct sockaddr *)p->ai_addr), s, sizeof(s));

//Ahsan
   // printf("Before getFileRequestHeader...\n");
    header_len = getFileRequestHeader(req_header);

    //printf("Header Description:%s, Header Len: %u\n", req_header, header_len);

    if (send(sockfd, req_header, header_len, 0) != header_len)
        perror("send() sent a different number of bytes than expected");

    if ((bytesRcvd = recv(sockfd, gfr->filecontent, BUFFER_SIZE, 0)) <= 0)
        perror("recv() failed or connection closed prematurely");

    //printf("Header Received: %s\n", gfr->filecontent);

    gfr->filelen = getFileLen(gfr->filecontent, gfr);

    //printf("File Length: %d\n", gfr->filelen);

    /* Receive the same string back from the server */
    int req_no=1;
    gfr->tot_bytes = 0;

    while ( 1 )
    {
        printf("Request: %d ", req_no++);
        ssize_t nb = recv( sockfd, gfr->filecontent, BUFFER_SIZE, 0 );
        if ( nb == -1 ) err( "recv failed" );
        if ( nb == 0 ) {printf("zero bytes received...breaking");break;} /* got end-of-stream */

        gfr->fl_handler(gfr->filecontent, nb, gfr->fDesc);
        gfr->tot_bytes += nb;

        printf("Received Bytes: %zd Total Received Bytes: %d\n", nb, gfr->tot_bytes);
    }

    return 0;
}


/*
 * Returns the string associated with the input status
 */

char* gfc_strstatus(gfstatus_t status)
{
    return stringFromError(status);
}


gfstatus_t gfc_get_status(gfcrequest_t *gfr){
    return gfr->ret_status;
}

size_t gfc_get_filelen(gfcrequest_t *gfr)
{
    return gfr->filelen;
}

size_t gfc_get_bytesreceived(gfcrequest_t *gfr)
{

    return gfr->tot_bytes;

}

void gfc_cleanup(gfcrequest_t *gfr)
{
    free(gfr);
    gfr=NULL;
}

void gfc_global_init()
{
;
//  pthread_mutex_lock(&counter_mutex);
//
//  gfc = (gfcrequest_t*)malloc(1* sizeof(gfcrequest_t));
//
//  pthread_mutex_unlock(&counter_mutex);


}

void gfc_global_cleanup()
{
;
//    pthread_mutex_lock(&counter_mutex);
//
//    free(gfc);
//
//    pthread_mutex_unlock(&counter_mutex);

}

//Server Code

struct gfcontext_t
{
    int sockfd;
    int clntSock;
};

struct gfserver_t
{
    char port[12];
    unsigned short max_npending;
    char fpath[256];
    ssize_t (*fp_handler)(gfcontext_t *ctx, char *, void*);
    int *handler_arg;
};


/*Variable decalation*/


static gfserver_t *gfserv;

static gfcontext_t *gfcontext;

int isEndMarker(char *iheader, int start)
{
    char *marker = "\\r\\n\\r\\n";
    int i = 0; int ind=0;


    while (ind <= 7)
    {
        if (iheader[start++] == marker[ind++])
        {
            //printf("Header Char:%c Marker:%c\n", iheader[start], marker[ind]);
            //start++;
            continue;
        }

        return 0;
    }
    //printf("Its a marker!!!\n");
    return 1;

}

int parseHeader(char *iheader, gfserver_t *gfs, int hlen)
{
//"GETFILEGET/courses/ud923/filecorpus/road.jpg\r\n\r\n"

    char scheme[8];
    char method[4];
//    char path[256];
    int pathindex = 0;
    char end_marker[12];

    int end = 0;
    int fexit=0;

    sscanf(iheader, "%7s%3s", scheme, method);

    int i=10;
        if (iheader[i] == '/')
        {
//            printf("Path has started...\n");
            if (!strcmp(scheme, "GETFILE") && !strcmp(method, "GET"))
            {

                while(1)
                {

                    if (iheader[i] == '\\')
                    {
                        end = isEndMarker(iheader, i);
                    }

                    if (end)
                        break;

                    gfs->fpath[pathindex++] = iheader[i++];

                }

                gfs->fpath[pathindex] = '\0';
            }
        }


    printf("Scheme: %s Method:%s Path:%s\n", scheme, method, gfs->fpath);

    return 0;
}


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);
}



ssize_t gfs_sendheader(gfcontext_t *ctx, gfstatus_t stat, size_t file_len)
{

    char resp_header[MAX_REQUEST_LEN];
    char end_marker[12] = "\\r\\n\\r\\n";

    sprintf(resp_header, "GETFILE%d%zd%s",stat, file_len, end_marker);

    printf("Response: %s\n", resp_header);

    if (send(ctx->clntSock, resp_header, MAX_REQUEST_LEN, 0) != MAX_REQUEST_LEN)
        perror("send() failed");

    return 0;
}

ssize_t gfs_send(gfcontext_t *ctx, void *data, size_t len)
{
    size_t total = 0;
    size_t bytesLeft = len;
    size_t n;

    int debug_req=1;

    while (total < len)
    {
         n = send(ctx->clntSock, data+total, bytesLeft, 0);
         if (n == -1) { printf("Nothing to send...\n"); break; }

         fprintf(stderr, "Tries: %d Bytes Sent: %zu\n", debug_req++, n);

         total += n;
         bytesLeft -= n;
    }

   // if ( shutdown( ctx->clntSock, SHUT_WR ) == -1 ) err( "socket shutdown failed" );

    return total;
}

void gfs_abort(gfcontext_t *ctx){

    close(ctx->clntSock);
    close(ctx->sockfd);
    free(ctx);
    free(gfserv);

    perror("aborting...");

    exit(1);
}

gfserver_t* gfserver_create()
{

    gfserv = (gfserver_t*) malloc(1 * sizeof(gfserver_t));
    gfcontext = (gfcontext_t*) malloc(1*sizeof(gfcontext_t));

    return gfserv;
}

void gfserver_set_port(gfserver_t *gfs, unsigned short port)
{
    //set port number in gfs structure
    snprintf(gfs->port,12, "%u",port);
}

void gfserver_set_maxpending(gfserver_t *gfs, int max_npending)
{
    //set maxpending connections
    gfs->max_npending = max_npending;
}

void gfserver_set_handler(gfserver_t *gfs, ssize_t (*handler)(gfcontext_t *, char *, void*))
{
    gfs->fp_handler = handler;
}

void gfserver_set_handlerarg(gfserver_t *gfs, void* arg)
{
    gfs->handler_arg = (int *)arg;
}

void gfserver_serve(gfserver_t *gfs)
{

    struct addrinfo hints, *servinfo, *p;
    struct sockaddr_storage clntAddr; //connectors address information
    socklen_t clntSize;
    int yes = 1;
    char s[INET6_ADDRSTRLEN];
    int rv;

    int rcvMsg = 0;
    char recvBuff[MAX_REQUEST_LEN];
    char *req_path;

    memset(&hints, 0, sizeof(hints));

    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM; //using stream socket instead of datagrams
    hints.ai_flags = AI_PASSIVE; //use my IP

    if ((rv = getaddrinfo(NULL, gfs->port, &hints, &servinfo)) != 0)
    {
        fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(rv));
        return 1;
    }

    // loop through all the results and bind to the first we can
    for(p = servinfo; p != NULL; p = p->ai_next)
    {

        if ((gfcontext->sockfd = socket(p->ai_family, p->ai_socktype, p->ai_protocol)) == -1)
        {
            perror("server: socket");
            continue;
        }

        //get rid of 'address already in use' error.
        if (setsockopt(gfcontext->sockfd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(int)) == -1)
        {
            perror("setsockopt");
            exit(1);
        }

        if (bind(gfcontext->sockfd, p->ai_addr, p->ai_addrlen) == -1)
        {
            close(gfcontext->sockfd);
            perror("server: bind");
            continue;
        }

        break;
    }

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

    freeaddrinfo(servinfo); // no need of servinfo structure anymore

    if (listen(gfcontext->sockfd, gfs->max_npending) == -1)
    {
        perror("listen");
        exit(1);
    }


    //printf("server: waiting for connetions...\n");

    while(1)
    {
        clntSize = sizeof(clntAddr);
        gfcontext->clntSock = accept(gfcontext->sockfd, (struct sockaddr *)&clntAddr, &clntSize);

        if (gfcontext->clntSock == -1)
        {
            perror("accept");
            continue;
        }

        inet_ntop(clntAddr.ss_family, get_in_addr((struct sockaddr *)&clntAddr), s, sizeof(s));

        //printf("server: got connection from %s\n", s);

        if (!fork())
        { // this is the child process
            if ((rcvMsg = recv(gfcontext->clntSock, recvBuff, MAX_REQUEST_LEN, 0)) < 0)
            {
                perror("recv() failed");
                exit(1);
            }

            /*Still to parse received request...*/
            //printf("Recd Header: %s, Recd %d bytes\n",recvBuff, rcvMsg);

            /*Parse the received header...*/

            int len = parseHeader(recvBuff, gfs, rcvMsg);

            //printf("Requested Path: %s\n", gfs->fpath);

            if (gfs->fp_handler(gfcontext, gfs->fpath, NULL) < 0)
            {
                printf("some problem...\n");

            }

            if ( shutdown( gfcontext->clntSock, SHUT_WR ) == -1 ) err( "socket shutdown failed" );
            //close(gfcontext->clntSock);
        }

    }

}

//Server gf_send is being called from following function handler function:

Handler:
ssize_t handler_get(gfcontext_t *ctx, char *path, void* arg){
    int fildes;
    size_t file_len, bytes_transferred;
    ssize_t read_len, write_len;
    char buffer[BUFFER_SIZE];

    printf("Path: %s\n", path);

    if( 0 > (fildes = content_get(path)))
        return gfs_sendheader(ctx, GF_FILE_NOT_FOUND, 0);

    /* Calculating the file size */
    file_len = lseek(fildes, 0, SEEK_END);

    gfs_sendheader(ctx, GF_OK, file_len);

    /* Sending the file contents chunk by chunk. */
    int req=1;
    bytes_transferred = 0;
    while(bytes_transferred < file_len){
        read_len = pread(fildes, buffer, BUFFER_SIZE, bytes_transferred);
        if (read_len <= 0){
            fprintf(stderr, "handle_with_file read error, %zd, %zu, %zu", read_len, bytes_transferred, file_len );
            gfs_abort(ctx);
            return -1;
        }

        printf("Request No: %d ", req++);
        write_len = gfs_send(ctx, buffer, read_len);
        if (write_len != read_len){
            fprintf(stderr, "handle_with_file write error");
            gfs_abort(ctx);
            return -1;
        }

        bytes_transferred += write_len;
    }

    printf("Total Bytes sent to client: %zu\n", bytes_transferred);

    return bytes_transferred;
}

Eldridgeeldritch

(1/2) Exactly what type of socket? (That is, show how you are calling socket, bind, and connect.) – Silber

(2/2) Your last-chunk-too-large problem might just be that you erroneously use BUFFER_SIZE instead of bytesRecvd in the call to fl_handler. – Silber

(3/2) Nitpick unrelated to your actual problem: the second argument to memset should be 0, not NULL. NULL is a pointer. The second argument to memset is a character. (You may have become confused because the name of ASCII character 0 is "NUL" with one L.) – Silber

fl_handler is just writing into the file, but I'm printing the returned bytes, which is printing the size of the buffer instead of the actual data sent from the server. And, yes understood about NULL -- I'll have to modify that. I'm using TCP sockets. – Eldridgeeldritch

Have a read here. His guides are super practical. beej.us/guide/bgnet – Decontaminate

Answer 1

You didn't specify, so I am assuming you are using TCP here (send/receive semantics is different with UDP),

You are suffering from a very common misconception that one send on one end of a TCP socket corresponds to one receive of sent number of bytes on the other end. This is wrong.

In fact, TCP socket is a bi-directional stream of bytes with no notion of messages. One write can correspond to many reads on the other end, and vise versa. Treat it as a stream.

You need to keep number of bytes sent and received as returned from sending and receiving system calls.

It is also important to let the other side know how much data you are sending, so it will know when, say, an image is fully transferred. This is the job of an application-level protocol that you have to either come up with or use an existing one.

Edit 0:

Here is what looks needed even before setting up any meaningful protocol between client and the server. First the sending code:

size_t total = 0;

while ( total != len ) {
    ssize_t nb = send( s, data + total, len - total, 0 );
    if ( nb == -1 ) err( "send failed" );
    total += nb;
}
if ( shutdown( s, SHUT_WR ) == -1 ) err( "socket shutdown failed" );
/* also need to close client socket, see below */

Then the receiving code:

char buffer[BUFFER_SIZE]; /* somewhere, might be static */
size_t total = 0; /* everything received */
while ( 1 ) {
    ssize_t nb = recv( s, buffer, BUFFER_SIZE, 0 );
    if ( nb == -1 ) err( "recv failed" );
    if ( nb == 0 ) break; /* got end-of-stream */
    if ( write( file_fd, buffer, nb ) == -1 ) err( "file write failed" );
    total += nb;
}
/* send an ack here */
if ( close( s ) == -1 ) err( "socket close failed" );
if ( close( file_fd )) err( "file close failed" );
printf( "received and saved total of %zu bytes\n", total );

Then your application-level protocol might be as simple as server sending, say, 64-bit file length immediately after accepting new client connection (you need to decide what endianness to use for that), then after sending that many bytes to the client and shutting down writing on the socket, waiting for the client to acknowledge successful receipt of data. That might be that same number back, or just a single byte - up to you, then finally closing the socket. That way the client knows upfront how many bytes to expect, and the server knows that transfer was successful.

After getting this simple version working you can extend it to allow multiple file transfers per connection, and/or dive into IO multiplexing with select(2)/poll(2).

Hope this helps.

Sharma

1up - Fantastic answer. – Synovia

Yes, I'm using TCP. I have read across the problem you have mentioned and tried to solve it at 'sender' end. Can you please provide some insight into how I can solve it at the receiver end? I was under the assumption that while loop is working till all the data reaches, so if I receive less bytes, recv will be called again in next iteration. – Eldridgeeldritch

Looks like you need to increment your buffer offsets. Also check out @EJP answer. – Sharma

What you say about recv, actually applies to send as well. – Decontaminate

Can you please point me to some correct implementation of recv, the way its described above? – Eldridgeeldritch

@NikolaiNFetissov thanks for the detailed reply. I have made the changes suggested, but they don't fix the issue. Also, what's different between the 'receive' code you have written or the one I have? Server code is duly modified. I'm editing my post above to give output on 1024 buffer. Any buffer other than '128' is still not working correctly for receive. – Eldridgeeldritch

Try shutting down/closing the socket on the server side when you're done sending. – Sharma

So when I shutdown the socket, it misses a complete chunk of data, but if I close the socket I get same behavior. I was using fork but I have disabled it. It didn't change anything. – Eldridgeeldritch

@Ahsan, you're not showing us something in your code that messes this up. Simplify it so you get the socket communications correct, then build up from there, – Sharma

One more thing: I get the same number of bytes in both cases, in 'close socket' case client keeps in running mode, but in 'shutdown' case, client quits with missing bytes. – Eldridgeeldritch

I want to show everything... please let me know which part. Actually my code is in different files, so I can only post it in chunks. Please tell me which parts do you need to see. – Eldridgeeldritch

This is why an ack from the client is important :) Set SO_LINGER option on the connected socket on the server side (man7.org/linux/man-pages/man7/socket.7.html), this will prevent shutdown/close tearing down the connection. – Sharma

While I'm checking that, I'm sharing the complete code... server.c & client.c... some functions aren't listed in here, but I think they may not be that relevant as well. – Eldridgeeldritch

@NikolaiNFetissov I'm still stuck. Have you been able to look at the code? :( – Eldridgeeldritch

Looks like at least one problem is the client does not account for the number of bytes received on the first read while getting the "header" - again assuming it reads just the header-worth of data. Make that header fixed length and read only that many bytes on the first read. – Sharma

Or count the number of bytes parsed from that header, then the rest you got on the first read is the file data. – Sharma

Answer 2

First of all: recv() does not always receive the data in the chunks that it was sent by send(), as a matter of fact - it rarely does - because of buffering (for example, you send 256-bytes receive two buffers of 128-bytes each)

Now to your error: I think the problem is that you are not be calling select() with a FD_SET to reset your socket to a "ready to receive" state before calling recv() a second time.

I have a metric-ton of winsock/c-sockets code on my site if you want to dig through it.

Let me know if I can expand on this answer, I'd be happy to provide additional assistance!

Synovia

Thanks! I'd love to hear about how I can reset the socket before I call recv again. I think it would solve my error. – Eldridgeeldritch

The first paragraph of this answer is correct, but the second paragraph is completely wrong. It is not necessary to call select on a socket before calling recv a second time. If you only have one socket to receive from, there is no harm in blocking in recv instead of select. If you have more than one, then you of course have set all of them to nonblocking, and you can -- indeed, should -- call recv in a loop until it fails with errno set to EAGAIN. – Silber

I agree, but at the time of this answer - the question did not make this clear and there was much less source code to digest. – Synovia

Answer 3

gfr->fl_handler(gfr->filecontent, BUFFER_SIZE, gfr->fDesc);

Usual problem. You're assuming the read filled the buffer. It should be:

gfr->fl_handler(gfr->filecontent, bytesRcvd, gfr->fDesc);

Cornwall

Yes, it should be like this. I'll correct it. It's one less bug. :) – Eldridgeeldritch

Also, when bytesRcvd is <= 0, that loop iteration needs to not update tot_bytes (in case bytesRcvd is -1) or call fl_handler(), as there won't be any data available from that iteration. When bytesRcvd is 0, the socket was disconnected and there won't be any more data, so just break the loop. But if bytesRcvd is -1, you have to look at errno to know whether you can safely continue the loop to retry recv() or need to break it. – Gono