unix tcp client/server weird behavior

Question

I've been trying to learn unix network programming so I tried to write a client/server program in which the client sends a message and the server returns the message converted to uppercase letters.

When I run the server and connect with a client to my own machine and send some test strings, it works fine until a point where the things I previously entered get written to the screen.

I suspect it has something to do with the buffer. Here's the sample run after I start the server:

can@ubuntu:~$ cd Desktop

can@ubuntu:~/Desktop$ ./tcpuppcli 127.0.0.1

Enter the string to echo: test

Echo response: TEST

Enter the string to echo: string2

Echo response: STRING2

Enter the string to echo: string3

Echo response: STRING3

Enter the string to echo: aaaaaaaaafsfagd

Echo response: AAAAAAAAAFSFAGD

Enter the string to echo: gdsgsg

Echo response: GDSGSG

AAFSFAGD ----------->!this is the weird line that has chars from the previous input!

Enter the string to echo: ^C

can@ubuntu:~/Desktop$

The code for the server and client is as follows:

// methods with prefix w_ are just error checking wrappers from UNP by Stevens    
// server
#include "socketwrap.h" // headers necessary and constants like MAXLINE
#include <ctype.h>

void sigchld_handler( int sig);
void str_upper( int connfd);
void toUpperCase( char buffer[], int length);

int main( int argc, char** argv)
{
  int listenfd;
  int connfd;
  pid_t childpid;
  struct sockaddr_in serverAddress;
  struct sockaddr_in clientAddress;
  socklen_t length;
  struct sigaction sa;

  // Create the socket
  listenfd = w_socket( AF_INET, SOCK_STREAM, 0);

  // Clear the serverAddress structure
  bzero( &serverAddress, sizeof( serverAddress));
  // Set up the serverAddress structure
  serverAddress.sin_family = AF_INET;
  serverAddress.sin_addr.s_addr = htonl( INADDR_ANY);
  serverAddress.sin_port = htons( 11979);

  // Bind the socket to a well-defined port
  w_bind( listenfd, (struct sockaddr*) &serverAddress, sizeof( serverAddress));

  // Start listening for connections
  w_listen( listenfd, BACKLOG);

  // Handle any zombie children by using a signal handler
  sa.sa_handler = sigchld_handler;
  sigemptyset( &sa.sa_mask);
  sa.sa_flags = SA_RESTART;
  if( sigaction( SIGCHLD, &sa, NULL) == -1)
  {
    perror( "signal error");
    exit( 1);
  }

  printf( "Waiting for connections...\n");

  while( 1)
  {
      length = sizeof( clientAddress);
      connfd = w_accept( listenfd, ( struct sockaddr*) &clientAddress, &length);
      if( connfd < 0)
      {
          if( errno == EINTR)
          {
            continue; // back to while
          }
          else
          {
            perror( "accept error");
            exit( 1);
          }
      }

      printf( "Obtained connection...\n");
      childpid = fork();
      if ( childpid == 0) /* child process */
      {
        w_close( listenfd); /* close listening socket */
        str_upper( connfd); // process the request
        exit( 0);
      }

    w_close( connfd); /* parent closes connected socket */
  }

}

void sigchld_handler( int sig)
{
  while( waitpid( -1, NULL, WNOHANG) > 0);
}

void str_upper( int connfd)
{
  char buffer[MAXLINE];
  while( read( connfd, buffer, MAXLINE - 1) > 0)
  {
    toUpperCase( buffer, strlen( buffer));
    write( connfd, buffer, strlen( buffer));
  }
}

void toUpperCase( char buffer[], int length)
{
  int i;
  for( i = 0; i < length - 1; i++)
  {
      buffer[i] = toupper( buffer[i]);
  }
}

// client
#include "socketwrap.h"

void str_cli( int connfd);

int main( int argc, char** argv)
{
  int sockfd;
  struct sockaddr_in serverAddress;

  if( argc != 2)
  {
      printf( "Invalid argument count\n");
      printf( "Correct usage: tcpcli4 <IPaddress>\n");
      exit( 1);
  }


  sockfd = w_socket( AF_INET, SOCK_STREAM, 0);
  bzero( &serverAddress, sizeof( serverAddress));
  serverAddress.sin_family = AF_INET;
  serverAddress.sin_port = htons( 11979);
  if( inet_pton( AF_INET, argv[1], &serverAddress.sin_addr) <= 0)
  {
      perror( "inet_pton error");
      exit( 1);
  }

  w_connect( sockfd, ( struct sockaddr*) &serverAddress, sizeof( serverAddress));

  str_cli( sockfd);
  exit( 0);
}

void str_cli( int connfd)
{
  char buffer[MAXLINE];

  printf( "Enter the string to echo: ");
  while( fgets( buffer, MAXLINE, stdin) > 0)
  {

    // Send string to echo server, and retrieve response
    write( connfd, buffer, strlen( buffer));
    read( connfd, buffer, MAXLINE - 1);

    // Output echoed string
    printf( "Echo response: %s\n", buffer);
    printf( "Enter the string to echo: ");
  }
}

If you need additional information or there's anything unclear please inform me

Thanks for all replies

Answer 1

As well as the \0 problem pointed out by Alan Curry, don't forget that TCP is not a message orientated transport, it is stream orientated.

There is absolutely no guarantee that a single write on one end will result in a single read at the other. In fact in some circumstances it's guaranteed that that won't happen.

You should put in some form of structure or demarcation in your packet to indicate how long the next chunk of data is going to be. A good one in this case would be a 32-bit integer containing the string's length, packed as four bytes. Network protocols usually send stuff like this in "big endian" format (aka "network order"), so you would use htonl() to convert the value on sending, and ntohl() on receipt.

On the sending side you can use writev() to combine writing the length value and the data itself into one system call. On the receiving side, just use read() to get the length, and then a second read() to read that much data.

Also, don't forget that any read() or write() call can send less data then was actually specified. Each such operation should be wrapped in a loop which calls the function repeatedly until the appropriate number of bytes have been processed.

Answer 2

read() doesn't add a '\0' terminator to the string, so the result your strlen() call is not well-defined. You need to use the return value of read() (store it in a variable, don't just test it for > 0 then throw it away) to know how long the string is.

It seemed to be working correctly for a while, but that's just dumb luck with '\0's that were in the buffer when it was uninitialized. Try it with a memset(buffer, 'X', sizeof buffer) before the read loop and you'll get an earlier failure.