TCP client / server messaging problem

[bleedi]

Hi,

I have a little bit of a problem with my client / server apps. I'm trying to build a simple program that resembles a small instant messaging app. So far I've been working on the "user login" and "status change" parts, along with a query to get the status of all the other users.

The problem is, the status query message - or any other message after the initial login message - doesn't go through to the server from the client. The client sends the message and it reports the correct amount of data sent, but the message just won't appear at the server side. I would guess this could be a serverside problem, but then again, there's another problem in the client app. When I try to send the same message again, the client program crashes.

In a nutshell, it goes like this:
1. Client sends "LOGIN" to server - OK
2. Client sends "LIST" to server - Nothing happens
3. Client sends "LIST" again to server - Nothing happens at serverside, client crashes

The "LIST" command works if I replace the "LOGIN" message with it, so in theory, the connection works only for the first message sent and received.

Here's my not-so-clean and definitely not foolproof code so far. (PS, serverside uses also C++)

client.c

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

char* username;
char* status;
int socket_fd, portno, msg;
struct sockaddr_in server_addr;
struct hostent* server;

void printWelcome() {
	printf("Hello, %s.\n\n", username);
}

void sendLogin(char* usr) {
	
    char* msg_p1 = "POST /LOGIN";
    char* msg_p2 = " HTTP/1.1\r\n\r\n";
    char* msg_p3 = "username=";
    char* msg_end = "\r\n\r\n";
    
    int msg_length = sizeof(msg_p1) + sizeof(msg_p2) + sizeof(msg_p3) + sizeof(usr) + sizeof(msg_end);
    char* cmsg = malloc(1024);
    strcat(cmsg, msg_p1);
    strcat(cmsg, msg_p2);
    strcat(cmsg, msg_p3);
    strcat(cmsg, usr);
    strcat(cmsg, msg_end);

    int sent_bytes = send(socket_fd, cmsg, strlen(cmsg), 0);
    if(sent_bytes != strlen(cmsg)) {
		printf("Wrong amount of data was sent.\n");
	}
    free(cmsg);
}

void listUsers() {
	char* msg_p1 = "POST /LIST";
	char* msg_p2 = " HTTP/1.1\r\n\r\n";
	
	int msg_length = sizeof(msg_p1) + sizeof(msg_p2);
	char* cmsg = malloc(1024);
	strcat(cmsg, msg_p1);
	strcat(cmsg, msg_p2);
	
	int sent_bytes = send(socket_fd, cmsg, strlen(cmsg), 0);
	printf("Sent %d bytes.\n", sent_bytes);
	if(sent_bytes != strlen(cmsg)) {
		printf("Wrong amount of data was sent.\n");
	}
	free(cmsg);
}

void printMenu() {
    printf("\nChoose a command:\n");
    printf("1 - Set status to online\n");
    printf("2 - Set status to away\n");
    printf("3 - Show other users' statuses\n");
    printf("0 - Exit\n");
    printf(">> ");
}

int main(int argc, char *argv[]) {
    
    username = argv[3];
    status = argv[4];
  
    if(argc < 4) {
        fprintf(stderr, "Usage: %s <hostname> <port> <username>\n", argv[0]);
        exit(0);
    }
    
    portno = atoi(argv[2]);
    socket_fd = socket(AF_INET, SOCK_STREAM, 0);
    if(socket_fd < 0) {
        printf("Error opening socket\n");
        exit(0);
    }
    
    server = gethostbyname(argv[1]);
    if(server == NULL) {
        fprintf(stderr, "Cannot find the server\n");
        exit(0);
    }
    
    bzero((char *) &server_addr, sizeof(server_addr));
    
    bcopy((char *)server->h_addr, (char *)&server_addr.sin_addr.s_addr, server->h_length);
    server_addr.sin_family = AF_INET;
    server_addr.sin_port = htons(portno);
    
    if(connect(socket_fd, (struct sockaddr *)&server_addr, sizeof(server_addr)) < 0) {
        perror("Error connecting\n");
        exit(0);
    }
    
    sendLogin(username);
    
    int running = 1;
    int choice = 0;
    printWelcome();
    do{
    	printMenu();
    	scanf("%d", &choice);
    	switch(choice) {
			case 1:
				break;
			case 2:
				//sendAwayStatus();
				break;
			case 3:
				listUsers();
				break;
			case 0:
				printf("Bye bye!\n\n");
				shutdown(socket_fd, 2);
				close(socket_fd);
				exit(0);
				break;
		}
		
		
		
    }while(running == 1);

    return EXIT_SUCCESS;
}

server.cpp

#include <iostream>
#include <vector>
#include <cstdlib>
#include <cstdio>
#include <string>
#include <sstream>

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

using namespace std;

int parseMessageCode(string inc_msg) {
	cout << "Parsing the message..." << endl;
	size_t pos = inc_msg.find("/LOGIN", 0);
	if(pos!=string::npos) {
		return 1;
	}
	pos = inc_msg.find("/PRESENT", 0);
	if(pos!=string::npos) {
		return 2;
	}
	pos = inc_msg.find("/AWAY", 0);
	if(pos!=string::npos) {
		return 3;
	}
	pos = inc_msg.find("/LIST", 0);
	if(pos!=string::npos) {
		return 4;
	}
	
	return 0;
}

string parseSender(string inc_msg) {
	string usr;
	size_t pos = inc_msg.find("username=", 0);
	if(pos!=string::npos) {
		usr = inc_msg.substr(pos+9);
		stringstream ss(usr);
	
		ss >> usr;
		
	}
	return usr;
}

int main(int argc, char** argv) {
	
	if(argc < 2) {
		printf("Usage: %s <port>\n", argv[0]);
		return 0;
	}
	
	vector <vector <string> > users;
	
	int inc_socket = socket(AF_INET, SOCK_STREAM, 0);
	int portno = atoi(argv[1]);
	
	struct sockaddr_in server_addr;
	
	bzero((char *) &server_addr, sizeof(server_addr));
	
	server_addr.sin_family = AF_INET;
	server_addr.sin_addr.s_addr = INADDR_ANY;
	server_addr.sin_port = htons(portno);
	int err = bind(inc_socket, (struct sockaddr *) &server_addr, sizeof(server_addr));
	if(err < 0) {
		printf("Error while binding (%d): %s\n", err, gai_strerror(err));
		return -1;
	}
	
	listen(inc_socket, 5);
	
	while(true) {
		struct sockaddr_in client_addr;
		bzero((char *) &client_addr, sizeof(client_addr));
		client_addr.sin_family = AF_INET;
		client_addr.sin_addr.s_addr = INADDR_ANY;
		

		cout << "Waiting for messages..." << endl;
		int client_len = sizeof(client_addr);
		int new_socket = accept(inc_socket, (struct sockaddr *) &client_addr, (socklen_t *)&client_len);
		cout << "Message received, processing the message..." << endl;
		
		char *buffer;
		buffer = (char*)malloc(1024);
		
		int msg = read(new_socket, buffer, 1023);
		string inc_msg = string(buffer, strlen(buffer));
		
		int inc_type = parseMessageCode(inc_msg);
		string inc_usr;
		if(inc_type == 1) inc_usr = parseSender(inc_msg);
		
		cout << "Type: " << inc_type << endl;
		
		switch(inc_type) {
			case 1: { // Login announcement
				cout << "User " << inc_usr << " logged in. - "<< inet_ntoa(client_addr.sin_addr) << "\n" << endl;
				vector<string> v;
				v.push_back(inet_ntoa(client_addr.sin_addr));
				v.push_back(inc_usr);
				v.push_back("Online");
				users.push_back(v);
				break;
			}
			case 2: { // status change "online"
			
				break;
			}
			case 3: { // status change "away"
			
				break;
			}
			case 4: { // Status query
				vector <vector <string> >::iterator iter; 
				for(iter = users.begin(); iter < users.end(); iter++) {
					//cout << iter->at.(1) << " (" << iter->at.(0) << ") - " << iter->.at(2) << endl;
					cout << (*iter).at(0) << endl;
				}
				break;
			}
		}
		
		free(buffer);
		
		shutdown(new_socket, 2);
		close(new_socket);
		
	}
	
	return 0;
}

[Banfa]

Line 28 and 47 of the client you strcat into an allocated buffer that contains random data (malloc does not clear the contents of the memory it allocates).

Make the first call in each of those sets strcpy.

[bleedi]

Line 28 and 47 of the client you strcat into an allocated buffer that contains random data (malloc does not clear the contents of the memory it allocates).

Make the first call in each of those sets strcpy.

That didn't help. I also tried to clear the cmsg buffers with memset.

I've been playing with memory allocations the whole day now, and I found out that I didn't handle the recv() calls the way I should. Well, now I do, but the problem persists.

EDIT:
I tried changing the client's messages to stub messages, clearing out all the excess material. The problem still exists, and seems to be caused by the send() command.

[Banfa]

If you have altered the code then re-post it, particularly the functions sendLogin and listUsers which sound like they have received the most modification.

After calling send you try the following

if(sent_bytes != strlen(cmsg)) {
    printf("Wrong amount of data was sent.\n");
}

This is not the correct way to handle send. Send may not send all the data you provide it rather depends on the underlying link and how busy the network is and a number of other things. If send has not sent all the data you wanted it is normal operation rather than an error, you should call send again with the rest of the data.

This is all rather well explained in "Beej's Guide to Network Programming" which you can find at http://beej.us/guide/bgnet/ to either download or buy as a book.

[nezachem]

Your server closes new_socket after each message it receives. Your client, on the other hand, sends all its messages over the same socket. That is, the second message is sent through the socket which is already closed. Therefore, the crash.

Rethink your design. Either let the client reestablish connection for each message, or make the connection persistent at the server side.