Merge pull request #14 from lil-skelly/http-refactor

[REFACTOR] HTTP library refactoring and bug fixes.

src/client/http.c

@@ -2,14 +2,15 @@
 #include "sock.h"
 #include <assert.h>
 #include <fcntl.h>
+#include <netdb.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/socket.h>
 #include <sys/file.h>
+#include <sys/socket.h>
+#include <time.h>
 
-
-const char *CONTENT_LENGTH = "Content-Length: ";
+const char *CONTENT_LENGTH_HEADER = "Content-Length: ";
 const char *GET_REQ_TEMPLATE =
     "GET %s HTTP/1.1\r\nConnection: keep-alive\r\n\r\n";
 
@@ -21,29 +22,67 @@ void print_http_res(const http_res_t res) {
   puts("--[ END ]--");
 }
 
+/* Parse HTTP status code */
+long parse_http_status_code(const char *buf) {
+  const char *status_code_start;
+  char *endptr;
+  long status_code;
+
+  status_code_start = strstr(buf, " ") + 1;
+  if (status_code_start == NULL) {
+    return -HTTP_INVALID_RESPONSE;
+  }
+  status_code = strtol(status_code_start, &endptr, 10);
+  if (endptr == status_code_start) {
+    return -HTTP_INVALID_RESPONSE;
+  }
+  return status_code;
+}
+
+/* Parse HTTP content length header */
+long parse_http_content_length(const char *buf) {
+  const char *content_length_start;
+  char *endptr;
+  long content_length;
+
+  content_length_start =
+      strstr(buf, CONTENT_LENGTH_HEADER) + strlen(CONTENT_LENGTH_HEADER);
+
+  content_length = strtol(content_length_start, &endptr, 10);
+  if (endptr == content_length_start) {
+    return -HTTP_INVALID_RESPONSE;
+  }
+  return content_length;
+}
+
 int http_get(int sfd, const char *path, http_res_t *res) {
   char request_buf[HTTP_BUFFER_SIZE]; // use separate buffer for the request
   char buf[HTTP_BUFFER_SIZE];
 
-  const char *status_code_start, *content_length_start, *body_start;
+  const char *body_start;
   int bytes_read;
-  long total_bytes, content_length, header_length, received_length, left_length;
+  long total_bytes, status_code, header_length, content_length, received_length,
+      left_length;
+  size_t req_buf_len;
+
+  /* send request */
+  snprintf(request_buf, HTTP_BUFFER_SIZE - 1, GET_REQ_TEMPLATE, path);
+  req_buf_len = strlen(request_buf);
 
-  // ensure buffers are null terminated
-  buf[HTTP_BUFFER_SIZE - 1] = 0;
-  request_buf[HTTP_BUFFER_SIZE -1] = 0;
+  if (send_request(sfd, request_buf) < 0) {
+    perror("Error: failed to send request\n");
+    return -HTTP_SOCKET_ERR;
+  }
+  if (HTTP_VERBOSE)
+    puts("Sent GET request");
 
-  // send request
-  snprintf(request_buf, HTTP_BUFFER_SIZE-1, GET_REQ_TEMPLATE, path);
-  send_request(sfd, request_buf);
-
-  res->request = request_buf;
-  if (HTTP_VERBOSE) puts("Sent GET request");
+  /* receive response from server */
+  if (HTTP_VERBOSE)
+    puts("Receiving data");
 
-  if (HTTP_VERBOSE) puts("Receiving data");
-  // receive response
   total_bytes = 0;
-  while ((bytes_read = recv(sfd, buf + total_bytes, HTTP_BUFFER_SIZE - 1 - total_bytes, 0)) > 0) {
+  while ((bytes_read = recv(sfd, buf + total_bytes,
+                            HTTP_BUFFER_SIZE - 1 - total_bytes, 0)) > 0) {
     total_bytes += bytes_read;
     // add temporary null terminator
     buf[total_bytes] = 0;
@@ -52,43 +91,73 @@ int http_get(int sfd, const char *path, http_res_t *res) {
       break;
     }
 
-    if (total_bytes >= HTTP_BUFFER_SIZE - 1) break;
+    if (total_bytes >= HTTP_BUFFER_SIZE - 1) {
+      buf[HTTP_BUFFER_SIZE - 1] = 0;
+      break;
+    }
   }
-  if (HTTP_VERBOSE) puts("Received data from server");
+  if (HTTP_VERBOSE)
+    puts("Received data from server");
 
+  /* Check if response starts with "HTTP" */
   if (memcmp(buf, "HTTP", 4)) {
-    return HTTP_INVALID_RESPONSE;
+    return -HTTP_INVALID_RESPONSE;
   }
 
-  status_code_start = strstr(buf, " ") + 1;
-  res->status_code = strtol(status_code_start, NULL, 10);
+  /* Parse status code */
+  status_code = parse_http_status_code(buf);
+  if (status_code < 0) {
+    return -HTTP_INVALID_RESPONSE;
+  }
+  res->status_code = (int)status_code;
 
-  content_length_start = strstr(buf, CONTENT_LENGTH) + strlen(CONTENT_LENGTH);
-  content_length = strtol(content_length_start, NULL, 10);
+  /* Parse content length */
+  content_length = parse_http_content_length(buf);
+  if (content_length < 0) {
+    return -HTTP_INVALID_RESPONSE;
+  }
+  res->size = (size_t)content_length;
 
-  res->size = content_length;
-  res->data = malloc(content_length);
-  if (NULL == res->data) {
-    return HTTP_OOM;
+  /* Parse the response body */
+  res->data = malloc(res->size);
+  if (res->data == NULL) {
+    return -HTTP_OOM;
   }
 
   body_start = strstr(buf, "\r\n\r\n") + 4;
   header_length = body_start - buf;
   received_length = MIN(total_bytes - header_length, content_length);
+
   memcpy(res->data, body_start, received_length);
 
   if (header_length + content_length > total_bytes) {
-    if (HTTP_VERBOSE) puts("Receiving left over data");
+    if (HTTP_VERBOSE)
+      puts("Receiving left over data");
     left_length = content_length - received_length;
-    recv_response(sfd, res->data + received_length, left_length);
+    if (recv_response(sfd, res->data + received_length, left_length) < 0) {
+      perror("Failed to receive left over data\n");
+      free(res->data);
+      return -HTTP_SOCKET_ERR;
+    }
   }
-  if (HTTP_VERBOSE) puts("Parsed response");
-  if (HTTP_VERBOSE > 1) print_http_res(*res);
+
+  res->request = malloc(req_buf_len);
+  if (res->request == NULL) {
+    free(res->data); // free previously allocated data
+    return -HTTP_OOM;
+  }
+  strncpy(res->request, request_buf, req_buf_len - 1);
+
+  if (HTTP_VERBOSE)
+    puts("Parsed response");
+  if (HTTP_VERBOSE > 1)
+    print_http_res(*res);
+
   return HTTP_SUCCESS;
 }
 
-
-/* Perform a GET request to path and write the body to the file specified in f_path */
+/* Perform a GET request to path and write the body to the file specified in
+ * f_path */
 int http_download_data_to_file(int sfd, const char *path, const char *f_path) {
   http_res_t res;
   FILE *file;
@@ -110,7 +179,7 @@ int http_download_data_to_file(int sfd, const char *path, const char *f_path) {
     perror("Error: Failed to write data to file");
     fclose(file);
     http_free(&res);
-    return -2;      
+    return -2;
   }
 
   if (fclose(file) != 0) {
@@ -123,9 +192,15 @@ int http_download_data_to_file(int sfd, const char *path, const char *f_path) {
   return 0;
 }
 
+/* Properly free a http_res_t structure */
 void http_free(http_res_t *res) {
-    free(res->data);
-    res->data = NULL;
-    res->size = 0;
-    res->status_code = 0;
+  free(res->data);
+  res->data = NULL;
+  free(res->request);
+  res->request = NULL;
+
+  res->size = 0;
+  res->status_code = 0;
+  return;
+
 }

src/client/http.h

@@ -13,7 +13,7 @@
 #define HTTP_INVALID_RESPONSE 2
 #define HTTP_OOM 3
 
-#define HTTP_VERBOSE 1
+#define HTTP_VERBOSE 2
 
 typedef struct {
     int status_code;
@@ -22,7 +22,11 @@ typedef struct {
     size_t size;
 } http_res_t;
 
-int   http_get(int sfd, const char *path, http_res_t *res);
 void  http_free(http_res_t *res);
+void  http_init(http_res_t *res);
+
+int   http_get(int sfd, const char *path, http_res_t *res);
 int   http_download_data_to_file(int sfd, const char *path, const char *f_path);
-#endif
+long  parse_http_status_code(const char *buf);
+long  parse_http_content_length(const char *buf);
+#endif // HTTP_H

src/client/main.c

@@ -1,9 +1,9 @@
-#include <sys/types.h>
 #include <stdlib.h>
+#include <sys/types.h>
 #include <unistd.h>
 
-#include "sock.h"
 #include "http.h"
+#include "sock.h"
 #include "utils.h"
 
 /* Networking constants */
@@ -31,9 +31,9 @@ int main() {
   if (sfd == -1) {
     return EXIT_FAILURE;
   }
-  
+
   /* Get the fraction links */
-  // Request index page of server
+  // http_init(&http_fraction_res);
   if (http_get(sfd, "/", &http_fraction_res) != HTTP_SUCCESS) {
     return EXIT_FAILURE;
   }
@@ -43,30 +43,34 @@ int main() {
   // Allocate memory for fraction links
   char **fraction_links = malloc(num_links * sizeof(char *));
   if (fraction_links == NULL) {
-      fprintf(stderr, "malloc failed to allocate memory for fraction links\n");
-      close(sfd);
-      return EXIT_FAILURE;
+    fprintf(stderr, "malloc failed to allocate memory for fraction links\n");
+    close(sfd);
+    http_free(&http_fraction_res);
+    return EXIT_FAILURE;
   }
 
-  // Split the response data into lines 
-  int lines_read = split_fraction_links(http_fraction_res.data, fraction_links, num_links);
+  // Split the response data into lines
+  int lines_read =
+      split_fraction_links(http_fraction_res.data, fraction_links, num_links);
   if (lines_read < 0) {
-      free(fraction_links);
-      close(sfd);
-      return EXIT_FAILURE;
+    http_free(&http_fraction_res);
+    free(fraction_links);
+    close(sfd);
+    return EXIT_FAILURE;
   }
 
   // Print the fraction links
   // TODO: Download each link to a file
   for (int i = 0; i < lines_read; i++) {
-      printf("%s\n", fraction_links[i]);
-      free(fraction_links[i]); // Free allocated memory for each line
+    printf("%s\n", fraction_links[i]);
+    free(fraction_links[i]); // Free allocated memory for each line
   }
 
-  // Free the array of pointers
+  /* Cleanup */
+  freeaddrinfo(ainfo);
+  close(sfd);
+  http_free(&http_fraction_res);
   free(fraction_links);
 
-  close(sfd);
-  freeaddrinfo(ainfo);
   return EXIT_SUCCESS;
 }

src/client/utils.h

@@ -8,7 +8,7 @@
 
 /* Helper to count new line characters in a string */
 static inline int count_lines(const char *str) {
-  int count;
+  int count = 0;
   while (*str) {
     if (*str == '\n') count++;
     str++;