My http server in c++ is not sending all files back correctly

Question

I'm working on an HTTP server in c++, and right now it works for requests of text files, but when trying to get a jpeg or something, only part of the file gets sent. The problem seems to be that when I use fgets(buffer, 2000, returned_file) it seems to increment the file position indicator much more than it actually ends up putting into the buffer. Why would this happen? I put all my code below. The problem occurs in while(true) loop that occurs when the response code is 200. Thank you to anyone who replies.

// Interpret the command line arguments
unsigned short port = 8080;

if ( (argc != 1) && (argc != 3) && (argc != 5) ) {
  cerr << "Usage: " << argv[0];
  cerr << " -p <port number> -d <base directory>" << endl;
  return 1;
}
else {
  for (int i = 1; i < argc; ++i) {
    if (strcmp(argv[i], "-p") == 0)
      port = (unsigned short) atoi(argv[++i]);
    else if (strcmp(argv[i], "-d") == 0)
      base_directory = argv[++i];
  }
}
// if base_directory was not given, set it to current working directory
if ( !base_directory ) {
  base_directory = getcwd(base_directory, 100);
}

// Create TCP socket
int tcp_sock = socket(AF_INET, SOCK_STREAM, 0);
if (tcp_sock < 0) {
  cerr << "Unable to create TCP socket." << endl;
  return 2;
}

// Create server socket
sockaddr_in server;
server.sin_family = AF_INET;
server.sin_port = htons( port );
server.sin_addr.s_addr = INADDR_ANY;

// Bind the socket
if (bind(tcp_sock, (sockaddr*)&server, sizeof(server)) < 0) {
  cerr << "Unable to bind TCP socket." << endl;
  return 3;
}

// Listen for a connection request on TCP port
listen(tcp_sock, 5);

// Create HTTP_Request object and start a while loop of accepting connections
char buffer[2000];
int bytes_recv = 0;
int recv_len = 0;
string error_reply;

HTTP_Response* response;

while (true) {
  int acc_tcp_sock = accept(tcp_sock, NULL, NULL);
  if (acc_tcp_sock == -1) {
    cerr << "Unable to open TCP connection with client." << endl;
  }
  do {
    // may want to do just one recv
    recv_len = recv( acc_tcp_sock, buffer + bytes_recv,
      2000 - bytes_recv, 0 );
    bytes_recv += recv_len;
  } while (false);
  bytes_recv = 0;
  // may want to see if this causes a memory leak
  HTTP_Request* request = HTTP_Request::Parse(buffer, 2000);

  response = handle_request(request); // function to handle the request

  // Put response header into buffer
  response->Print( buffer, 2000 );

  // if 200 and GET then send header with file
  if ( response->Get_code() == 200 ) {
    // send response header
    if ( send( acc_tcp_sock, buffer, strlen(buffer), 0 ) < 0 ) {
      cerr << "Unable to send response header to client." << endl;
    }
    if ( method == "GET" ) {
      // send file
      while ( true ) {
        fgets( buffer, 2000, returned_file );
        if ( feof( returned_file ) ) break;
        if ( send( acc_tcp_sock, buffer, strlen(buffer), 0 ) < 0 ) {
          cerr << "Unable to send file in response to client." << endl;
        }
      }
    }
    fclose( returned_file ); // close file
  }
  else {
    if ( method == "GET" ) {
      error_reply = buffer + error_page;
      if ( send( acc_tcp_sock, error_reply.c_str(), error_reply.length(), 0 ) < 0 ) {
        cerr << "Unable to send response to client." << endl;
      }
    }
    else {
      if ( send( acc_tcp_sock, buffer, strlen(buffer), 0 ) < 0 ) {
        cerr << "Unable to send respone header to client." << endl;
      }
    }
  }

  close( acc_tcp_sock ); // close the connection
}

return 0;

Answer 1

Wouldn't using strlen break on binary files?

send( acc_tcp_sock, buffer, strlen(buffer), 0 )

Answer 2

Don't use fgets() to read binary data that needs to survive bit-for-bit. You don't want record-separator translation, and some systems may assume it's text if you read it that way. For that matter, newlines and record-separators are completely meaningless so the fgets()` function of scanning for them is at best a confusing inefficiency and at worst simply not-binary-capable at all.

Use fread(3), or better yet, use the raw system call (Posix API anyway, on non-Unix) read(2). This will read a certain amount of bit-for-bit data and tell you how much it read. (Regarding which binary-capable API to use: normally, we are advised to buffer data because we typically process it in small units like lines. However, when moving an entire file from one place to another buffering just slows you down. In this case it is simpler and faster to just use read().)

You also can't strlen() binary data. You have to just use the byte count from the API call.

Answer 3

It is very probable that your binary files contain NULL bytes ('\0'). When you read data with fgets, it may be placed into buffer, but when you transmit it everything after \0 gets lost (your strlen call ensures this).

So, you need to use fread to read data. It returns a number of bytes that were actually read, so you don't need to use strlen at all. And don't forget to open file in binary mode!

Answer 4

Better, read about mapping files to memory, that way you don't have to manage buffers for reading file content, and you can pass that buffer to send and you get file size in one way.

If you really want to read bytes from file, then you have to distinguish reading binary files (with correct mime type or application/octet-stream, storing bytes read count with buffer) and opening files as text (text/* mime-types, and you can use strlen).