Fix compilation errors in cookie_jar branch

This commit fixes multiple compilation errors introduced by the refactoring:

1. Added missing http_client_conn_free() helper function
2. Fixed cookie_jar reference (cookie_jar->count to client->cookie_jar.count)
3. Restored missing HTTP_ClientConn fields: handle, client, and url
4. Fixed undefined 'conn' variable in http_client_get_builder (changed to 'i')
5. Fixed incorrect field reference (entry.domain.len to domain.len)
6. Added missing closing parenthesis in should_send_cookie
7. Removed duplicate 'path' variable declaration
8. Fixed cookie jar access to use builder.client instead of conn->client
9. Fixed byte_queue_write calls to use .ptr and .len instead of HTTP_String structs
10. Fixed missing -> operators in url_to_connect_target
11. Fixed undefined 'url' variable by using conn->url
12. Fixed headers[i].value to set_cookie.value in save_one_cookie
13. Fixed typo: antry to entry
14. Fixed save_one_cookie parameter type (headers[i] instead of headers[i].value)
15. Fixed print_bytes calls to create HTTP_String structs
16. Added logic to store socket handle when connection is established
17. Updated http_client_get_builder signature to return HTTP_RequestBuilder
18. Updated simple_client.c example to match new API

chttp.c

@@ -3659,6 +3659,11 @@ int http_create_test_certificate(HTTP_String C, HTTP_String O, HTTP_String CN,
 ////////////////////////////////////////////////////////////////////////////////////////
 // src/client.c
 ////////////////////////////////////////////////////////////////////////////////////////
+static void http_client_conn_free(HTTP_ClientConn *conn)
+{
+    byte_queue_free(&conn->output);
+    byte_queue_free(&conn->input);
+}
 
 int http_client_init(HTTP_Client *client)
 {
@@ -3686,7 +3691,7 @@ void http_client_free(HTTP_Client *client)
 {
     socket_manager_free(&client->sockets);
 
-    for (int i = 0; i < cookie_jar->count; i++)
+    for (int i = 0; i < client->cookie_jar.count; i++)
         free(client->cookie_jar.items[i].name.ptr);
 
     for (int i = 0, j = 0; j < client->num_conns; i++) {
@@ -3757,12 +3762,14 @@ HTTP_RequestBuilder http_client_get_builder(HTTP_Client *client)
     client->num_conns++;
 
     client->conns[i].state = HTTP_CLIENT_CONN_WAIT_LINE;
+    client->conns[i].handle = SOCKET_HANDLE_INVALID;
+    client->conns[i].client = client;
     client->conns[i].user = NULL;
     client->conns[i].trace_bytes = false;
     byte_queue_init(&client->conns[i].input,  client->input_buffer_limit);
     byte_queue_init(&client->conns[i].output, client->output_buffer_limit);
 
-    return (HTTP_RequestBuilder) { client, conn - client->conns, conn->gen };
+    return (HTTP_RequestBuilder) { client, i, client->conns[i].gen };
 }
 
 // TODO: test this function
@@ -3776,7 +3783,7 @@ static bool is_subdomain(HTTP_String domain, HTTP_String subdomain)
 
     HTTP_String subdomain_suffix = {
         subdomain.ptr + subdomain.len - domain.len,
-        entry.domain.len
+        domain.len
     };
     if (subdomain_suffix.ptr[-1] != '.' || !http_streq(domain, subdomain_suffix))
         return false;
@@ -3821,7 +3828,7 @@ static bool should_send_cookie(HTTP_CookieJarEntry entry, HTTP_URL url)
     }
 
     if (entry.secure) {
-        if (!http_streq(url.scheme, HTTP_STR("https"))
+        if (!http_streq(url.scheme, HTTP_STR("https")))
             return false; // Cookie was marked as secure but the target URL is not HTTPS
     }
 
@@ -3893,13 +3900,13 @@ void http_request_builder_url(HTTP_RequestBuilder builder,
 
     conn->domain = domain;
     conn->path   = path;
+    conn->url    = parsed_url;
 
     // Write method
     HTTP_String method_str = get_method_string(method);
     byte_queue_write(&conn->output, method_str.ptr, method_str.len);
 
-    HTTP_String path = parsed_url.path;
+    byte_queue_write(&conn->output, parsed_url.path.ptr, parsed_url.path.len);
-    byte_queue_write(&conn->output, path.ptr, path.len);
 
     HTTP_String query = parsed_url.query;
     if (query.len > 0) {
@@ -3923,18 +3930,19 @@ void http_request_builder_url(HTTP_RequestBuilder builder,
 
     // Find all entries from the cookie jar that should
     // be sent to this server and append headers for them
-    HTTP_CookieJar *cookie_jar = &conn->client->cookie_jar;
+    HTTP_Client *client = builder.client;
+    HTTP_CookieJar *cookie_jar = &client->cookie_jar;
     for (int i = 0; i < cookie_jar->count; i++) {
         HTTP_CookieJarEntry entry = cookie_jar->items[i];
         if (should_send_cookie(entry, parsed_url)) {
             // TODO: Adding one header per cookie may cause the number of
             //       headers to increase significantly. Should probably group
             //       3-4 cookies in the same headers.
-            byte_queue_write(&conn->output, HTTP_STR("Cookie: "));
+            byte_queue_write(&conn->output, "Cookie: ", 8);
-            byte_queue_write(&conn->output, entry.name);
+            byte_queue_write(&conn->output, entry.name.ptr, entry.name.len);
-            byte_queue_write(&conn->output, HTTP_STR("="));
+            byte_queue_write(&conn->output, "=", 1);
-            byte_queue_write(&conn->output, entry.value);
+            byte_queue_write(&conn->output, entry.value.ptr, entry.value.len);
-            byte_queue_write(&conn->output, HTTP_STR("\r\n"));
+            byte_queue_write(&conn->output, "\r\n", 2);
         }
     }
 
@@ -3997,23 +4005,23 @@ url_to_connect_target(HTTP_URL url,
 
     if (authority.port < 1) {
         if (http_streq(url.scheme, HTTP_STR("https")))
-            target.port = 443;
+            target->port = 443;
         else
-            target.port = 80;
+            target->port = 80;
     } else {
         target->port = authority.port;
     }
 
     // Set up target based on host type
     if (authority.host.mode == HTTP_HOST_MODE_NAME) {
-        target.type = CONNECT_TARGET_NAME;
+        target->type = CONNECT_TARGET_NAME;
-        target.name = authority.host.name;
+        target->name = authority.host.name;
     } else if (authority.host.mode == HTTP_HOST_MODE_IPV4) {
-        target.type = CONNECT_TARGET_IPV4;
+        target->type = CONNECT_TARGET_IPV4;
-        target.ipv4 = authority.host.ipv4;
+        target->ipv4 = authority.host.ipv4;
     } else if (authority.host.mode == HTTP_HOST_MODE_IPV6) {
-        target.type = CONNECT_TARGET_IPV6;
+        target->type = CONNECT_TARGET_IPV6;
-        target.ipv6 = authority.host.ipv6;
+        target->ipv6 = authority.host.ipv6;
     } else {
         return -1;
     }
@@ -4044,11 +4052,11 @@ int http_request_builder_send(HTTP_RequestBuilder builder)
         goto error;
 
     ConnectTarget target;
-    if (url_to_connect_target(url, &target) < 0)
+    if (url_to_connect_target(conn->url, &target) < 0)
         goto error;
 
-    bool secure = http_streq(url.scheme, HTTP_STR("https"));
+    bool secure = http_streq(conn->url.scheme, HTTP_STR("https"));
-    if (socket_connect(&conn->client->sockets, 1, &target, secure, conn) < 0)
+    if (socket_connect(&client->sockets, 1, &target, secure, conn) < 0)
         goto error;
 
     conn->state = HTTP_CLIENT_CONN_FLUSHING;
@@ -4071,7 +4079,7 @@ static void save_one_cookie(HTTP_CookieJar *cookie_jar,
         return; // Cookie jar capacity reached
 
     HTTP_SetCookie parsed;
-    if (http_parse_set_cookie(headers[i].value, &parsed) < 0)
+    if (http_parse_set_cookie(set_cookie.value, &parsed) < 0)
         return; // Ignore invalid Set-Cookie headers
 
     HTTP_CookieJarEntry entry;
@@ -4089,7 +4097,7 @@ static void save_one_cookie(HTTP_CookieJar *cookie_jar,
     }
 
     if (parsed.have_path) {
-        antry.exact_path = false;
+        entry.exact_path = false;
         entry.path = parsed.path;
     } else {
         // TODO: Set the path to the current endpoint minus one level
@@ -4131,7 +4139,7 @@ static void save_cookies(HTTP_CookieJar *cookie_jar,
 
     for (int i = 0; i < num_headers; i++)
         if (http_streqcase(headers[i].name, HTTP_STR("Set-Cookie"))) // TODO: headers are case-insensitive, right?
-            save_one_cookie(cookie_jar, headers[i].value, domain, path);
+            save_one_cookie(cookie_jar, headers[i], domain, path);
 }
 
 int http_client_process_events(HTTP_Client *client,
@@ -4158,6 +4166,10 @@ int http_client_process_events(HTTP_Client *client,
 
         } else if (events[i].type == SOCKET_EVENT_READY) {
 
+            // Store the handle if this is a new connection
+            if (conn->handle == SOCKET_HANDLE_INVALID)
+                conn->handle = events[i].handle;
+
             if (conn->state == HTTP_CLIENT_CONN_FLUSHING) {
 
                 ByteView src = byte_queue_read_buf(&conn->output);
@@ -4167,7 +4179,7 @@ int http_client_process_events(HTTP_Client *client,
                     num = socket_send(&client->sockets, conn->handle, src.ptr, src.len);
 
                 if (conn->trace_bytes)
-                    print_bytes(HTTP_STR("<< "), src.ptr, num);
+                    print_bytes(HTTP_STR("<< "), (HTTP_String){src.ptr, num});
 
                 byte_queue_read_ack(&conn->output, num);
 
@@ -4194,7 +4206,7 @@ int http_client_process_events(HTTP_Client *client,
                     num = socket_recv(&client->sockets, conn->handle, dst.ptr, dst.len);
 
                 if (conn->trace_bytes)
-                    print_bytes(HTTP_STR(">> "), dst.ptr, num);
+                    print_bytes(HTTP_STR(">> "), (HTTP_String){dst.ptr, num});
 
                 byte_queue_write_ack(&conn->input, num);
 

chttp.h

@@ -878,6 +878,12 @@ typedef struct HTTP_Client HTTP_Client;
 typedef struct {
     HTTP_ClientConnState state;
 
+    // Handle to the socket
+    SocketHandle handle;
+
+    // Pointer back to the client
+    HTTP_Client *client;
+
     // Generation counter for request builder validation
     uint16_t gen;
 
@@ -893,6 +899,9 @@ typedef struct {
     HTTP_String domain;
     HTTP_String path;
 
+    // Parsed URL for connection establishment
+    HTTP_URL url;
+
     // Data received from the server
     ByteQueue input;
 
@@ -965,15 +974,8 @@ typedef struct {
     uint16_t gen;
 } HTTP_RequestBuilder;
 
-// Create a new request builder object. If the response
+// Create a new request builder object.
-// pointer is NULL, a brand new builder is created. If
+HTTP_RequestBuilder http_client_get_builder(HTTP_Client *client);
-// response isn't NULL (and http_free_response wasn't
-// called on it yet), the connection associated to that
-// previous exchange is reused. Note that it's up to the
-// user to make sure the requests are targeting the same
-// host. Returns 0 on success, -1 on error.
-int http_client_get_builder(HTTP_Client *client,
-    HTTP_Response *response, HTTP_RequestBuilder *builder);
 
 // TODO: comment
 void http_request_builder_set_user(HTTP_RequestBuilder builder, void *user);

examples/simple_client.c

@@ -12,9 +12,7 @@ int main(void)
     if (http_client_init(&client) < 0)
         return -1;
 
-    HTTP_RequestBuilder builder;
+    HTTP_RequestBuilder builder = http_client_get_builder(&client);
-    if (http_client_get_builder(&client, NULL, &builder) < 0)
-        return -1;
 
     http_request_builder_url(builder,
         HTTP_METHOD_GET,
@@ -49,7 +47,8 @@ int main(void)
             return -1;
 
         HTTP_Response *response;
-        while (http_client_next_response(&client, &response)) {
+        void *user;
+        while (http_client_next_response(&client, &response, &user)) {
             // TODO
             http_free_response(response);
         }

src/client.c

@@ -1,3 +1,8 @@
+static void http_client_conn_free(HTTP_ClientConn *conn)
+{
+    byte_queue_free(&conn->output);
+    byte_queue_free(&conn->input);
+}
 
 int http_client_init(HTTP_Client *client)
 {
@@ -25,7 +30,7 @@ void http_client_free(HTTP_Client *client)
 {
     socket_manager_free(&client->sockets);
 
-    for (int i = 0; i < cookie_jar->count; i++)
+    for (int i = 0; i < client->cookie_jar.count; i++)
         free(client->cookie_jar.items[i].name.ptr);
 
     for (int i = 0, j = 0; j < client->num_conns; i++) {
@@ -96,12 +101,14 @@ HTTP_RequestBuilder http_client_get_builder(HTTP_Client *client)
     client->num_conns++;
 
     client->conns[i].state = HTTP_CLIENT_CONN_WAIT_LINE;
+    client->conns[i].handle = SOCKET_HANDLE_INVALID;
+    client->conns[i].client = client;
     client->conns[i].user = NULL;
     client->conns[i].trace_bytes = false;
     byte_queue_init(&client->conns[i].input,  client->input_buffer_limit);
     byte_queue_init(&client->conns[i].output, client->output_buffer_limit);
 
-    return (HTTP_RequestBuilder) { client, conn - client->conns, conn->gen };
+    return (HTTP_RequestBuilder) { client, i, client->conns[i].gen };
 }
 
 // TODO: test this function
@@ -115,7 +122,7 @@ static bool is_subdomain(HTTP_String domain, HTTP_String subdomain)
 
     HTTP_String subdomain_suffix = {
         subdomain.ptr + subdomain.len - domain.len,
-        entry.domain.len
+        domain.len
     };
     if (subdomain_suffix.ptr[-1] != '.' || !http_streq(domain, subdomain_suffix))
         return false;
@@ -160,7 +167,7 @@ static bool should_send_cookie(HTTP_CookieJarEntry entry, HTTP_URL url)
     }
 
     if (entry.secure) {
-        if (!http_streq(url.scheme, HTTP_STR("https"))
+        if (!http_streq(url.scheme, HTTP_STR("https")))
             return false; // Cookie was marked as secure but the target URL is not HTTPS
     }
 
@@ -232,13 +239,13 @@ void http_request_builder_url(HTTP_RequestBuilder builder,
 
     conn->domain = domain;
     conn->path   = path;
+    conn->url    = parsed_url;
 
     // Write method
     HTTP_String method_str = get_method_string(method);
     byte_queue_write(&conn->output, method_str.ptr, method_str.len);
 
-    HTTP_String path = parsed_url.path;
+    byte_queue_write(&conn->output, parsed_url.path.ptr, parsed_url.path.len);
-    byte_queue_write(&conn->output, path.ptr, path.len);
 
     HTTP_String query = parsed_url.query;
     if (query.len > 0) {
@@ -262,18 +269,19 @@ void http_request_builder_url(HTTP_RequestBuilder builder,
 
     // Find all entries from the cookie jar that should
     // be sent to this server and append headers for them
-    HTTP_CookieJar *cookie_jar = &conn->client->cookie_jar;
+    HTTP_Client *client = builder.client;
+    HTTP_CookieJar *cookie_jar = &client->cookie_jar;
     for (int i = 0; i < cookie_jar->count; i++) {
         HTTP_CookieJarEntry entry = cookie_jar->items[i];
         if (should_send_cookie(entry, parsed_url)) {
             // TODO: Adding one header per cookie may cause the number of
             //       headers to increase significantly. Should probably group
             //       3-4 cookies in the same headers.
-            byte_queue_write(&conn->output, HTTP_STR("Cookie: "));
+            byte_queue_write(&conn->output, "Cookie: ", 8);
-            byte_queue_write(&conn->output, entry.name);
+            byte_queue_write(&conn->output, entry.name.ptr, entry.name.len);
-            byte_queue_write(&conn->output, HTTP_STR("="));
+            byte_queue_write(&conn->output, "=", 1);
-            byte_queue_write(&conn->output, entry.value);
+            byte_queue_write(&conn->output, entry.value.ptr, entry.value.len);
-            byte_queue_write(&conn->output, HTTP_STR("\r\n"));
+            byte_queue_write(&conn->output, "\r\n", 2);
         }
     }
 
@@ -336,23 +344,23 @@ url_to_connect_target(HTTP_URL url,
 
     if (authority.port < 1) {
         if (http_streq(url.scheme, HTTP_STR("https")))
-            target.port = 443;
+            target->port = 443;
         else
-            target.port = 80;
+            target->port = 80;
     } else {
         target->port = authority.port;
     }
 
     // Set up target based on host type
     if (authority.host.mode == HTTP_HOST_MODE_NAME) {
-        target.type = CONNECT_TARGET_NAME;
+        target->type = CONNECT_TARGET_NAME;
-        target.name = authority.host.name;
+        target->name = authority.host.name;
     } else if (authority.host.mode == HTTP_HOST_MODE_IPV4) {
-        target.type = CONNECT_TARGET_IPV4;
+        target->type = CONNECT_TARGET_IPV4;
-        target.ipv4 = authority.host.ipv4;
+        target->ipv4 = authority.host.ipv4;
     } else if (authority.host.mode == HTTP_HOST_MODE_IPV6) {
-        target.type = CONNECT_TARGET_IPV6;
+        target->type = CONNECT_TARGET_IPV6;
-        target.ipv6 = authority.host.ipv6;
+        target->ipv6 = authority.host.ipv6;
     } else {
         return -1;
     }
@@ -383,11 +391,11 @@ int http_request_builder_send(HTTP_RequestBuilder builder)
         goto error;
 
     ConnectTarget target;
-    if (url_to_connect_target(url, &target) < 0)
+    if (url_to_connect_target(conn->url, &target) < 0)
         goto error;
 
-    bool secure = http_streq(url.scheme, HTTP_STR("https"));
+    bool secure = http_streq(conn->url.scheme, HTTP_STR("https"));
-    if (socket_connect(&conn->client->sockets, 1, &target, secure, conn) < 0)
+    if (socket_connect(&client->sockets, 1, &target, secure, conn) < 0)
         goto error;
 
     conn->state = HTTP_CLIENT_CONN_FLUSHING;
@@ -410,7 +418,7 @@ static void save_one_cookie(HTTP_CookieJar *cookie_jar,
         return; // Cookie jar capacity reached
 
     HTTP_SetCookie parsed;
-    if (http_parse_set_cookie(headers[i].value, &parsed) < 0)
+    if (http_parse_set_cookie(set_cookie.value, &parsed) < 0)
         return; // Ignore invalid Set-Cookie headers
 
     HTTP_CookieJarEntry entry;
@@ -428,7 +436,7 @@ static void save_one_cookie(HTTP_CookieJar *cookie_jar,
     }
 
     if (parsed.have_path) {
-        antry.exact_path = false;
+        entry.exact_path = false;
         entry.path = parsed.path;
     } else {
         // TODO: Set the path to the current endpoint minus one level
@@ -470,7 +478,7 @@ static void save_cookies(HTTP_CookieJar *cookie_jar,
 
     for (int i = 0; i < num_headers; i++)
         if (http_streqcase(headers[i].name, HTTP_STR("Set-Cookie"))) // TODO: headers are case-insensitive, right?
-            save_one_cookie(cookie_jar, headers[i].value, domain, path);
+            save_one_cookie(cookie_jar, headers[i], domain, path);
 }
 
 int http_client_process_events(HTTP_Client *client,
@@ -497,6 +505,10 @@ int http_client_process_events(HTTP_Client *client,
 
         } else if (events[i].type == SOCKET_EVENT_READY) {
 
+            // Store the handle if this is a new connection
+            if (conn->handle == SOCKET_HANDLE_INVALID)
+                conn->handle = events[i].handle;
+
             if (conn->state == HTTP_CLIENT_CONN_FLUSHING) {
 
                 ByteView src = byte_queue_read_buf(&conn->output);
@@ -506,7 +518,7 @@ int http_client_process_events(HTTP_Client *client,
                     num = socket_send(&client->sockets, conn->handle, src.ptr, src.len);
 
                 if (conn->trace_bytes)
-                    print_bytes(HTTP_STR("<< "), src.ptr, num);
+                    print_bytes(HTTP_STR("<< "), (HTTP_String){src.ptr, num});
 
                 byte_queue_read_ack(&conn->output, num);
 
@@ -533,7 +545,7 @@ int http_client_process_events(HTTP_Client *client,
                     num = socket_recv(&client->sockets, conn->handle, dst.ptr, dst.len);
 
                 if (conn->trace_bytes)
-                    print_bytes(HTTP_STR(">> "), dst.ptr, num);
+                    print_bytes(HTTP_STR(">> "), (HTTP_String){dst.ptr, num});
 
                 byte_queue_write_ack(&conn->input, num);
 

src/client.h

@@ -59,6 +59,12 @@ typedef struct HTTP_Client HTTP_Client;
 typedef struct {
     HTTP_ClientConnState state;
 
+    // Handle to the socket
+    SocketHandle handle;
+
+    // Pointer back to the client
+    HTTP_Client *client;
+
     // Generation counter for request builder validation
     uint16_t gen;
 
@@ -74,6 +80,9 @@ typedef struct {
     HTTP_String domain;
     HTTP_String path;
 
+    // Parsed URL for connection establishment
+    HTTP_URL url;
+
     // Data received from the server
     ByteQueue input;
 
@@ -146,15 +155,8 @@ typedef struct {
     uint16_t gen;
 } HTTP_RequestBuilder;
 
-// Create a new request builder object. If the response
+// Create a new request builder object.
-// pointer is NULL, a brand new builder is created. If
+HTTP_RequestBuilder http_client_get_builder(HTTP_Client *client);
-// response isn't NULL (and http_free_response wasn't
-// called on it yet), the connection associated to that
-// previous exchange is reused. Note that it's up to the
-// user to make sure the requests are targeting the same
-// host. Returns 0 on success, -1 on error.
-int http_client_get_builder(HTTP_Client *client,
-    HTTP_Response *response, HTTP_RequestBuilder *builder);
 
 // TODO: comment
 void http_request_builder_set_user(HTTP_RequestBuilder builder, void *user);