[BUGFIX] Track allocated size in response and prevent double-free in cleanup (#35)

* [BUGFIX] Track allocated size in response and prevent double-free in cleanup

* [TEST] Add connection pool tests for double-free bug fix (Issue #33)

- Add comprehensive test suite for connection pool edge cases
- Test empty body responses after multiple non-empty body requests
- Test alternating request patterns and stress scenarios
- Verify fix for SSL double-free crash (SIGABRT)
- All 5 tests pass, confirming the bug is fixed

* [TEST] Add critical double-free bug reproduction test

- test_double_free_bug_reproduction() now first test, most important
- Documents exact bug scenario and crash conditions
- Uses real httpmorph-bin server with /status/200 empty body endpoint
- Triggers Connection: close -> pool_connection_destroy -> double-free
- Without fix: crashes with SIGABRT (exit code 134)
- With fix: passes successfully
- All 6 connection pool tests pass

* [ENHANCEMENT] Add retry logic to real proxy tests for improved reliability

* [BUGFIX] Enhance error handling and memory safety in various components

* [ENHANCEMENT] Bump version to 0.2.7 in pyproject.toml

Author: arman-bd

.env.example

@@ -0,0 +1,17 @@
+# Example environment variables for httpmorph testing
+# Copy this file to .env and fill in your actual values
+
+# Proxy configuration for testing
+# Format: http://username:password@proxy-host:port
+# Or for country-specific proxy: http://username:password_country-CountryName@proxy-host:port
+TEST_PROXY_URL=http://your-username:your-password@proxy.example.com:31112
+
+# Proxy configuration for examples (separate username/password)
+PROXY_URL=http://proxy.example.com:31112
+PROXY_USERNAME=your-username
+PROXY_PASSWORD=your-password
+
+# HTTPBin testing host
+# Use httpmorph-bin.bytetunnels.com for reliable testing
+# (httpbin.org can be flaky and rate-limited)
+TEST_HTTPBIN_HOST=httpbin-of-your-own.com

pyproject.toml

@@ -9,7 +9,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "httpmorph"
-version = "0.2.6"
+version = "0.2.7"
 description = "A Python HTTP client focused on mimicking browser fingerprints."
 readme = "README.md"
 requires-python = ">=3.8"

src/core/async_request.c

@@ -1339,6 +1339,43 @@ static int step_receiving_headers(async_request_t *req) {
                 }
                 async_request_set_error(req, -1, "SSL connection closed before complete headers");
                 return ASYNC_STATUS_ERROR;
+            } else if (err == SSL_ERROR_SYSCALL && received == 0) {
+                /* SSL_ERROR_SYSCALL with received==0 and errno==0 means clean connection close (EOF) */
+                #ifdef _WIN32
+                int sys_err = WSAGetLastError();
+                bool is_eof = (sys_err == 0);
+                #else
+                bool is_eof = (errno == 0);
+                #endif
+
+                if (is_eof) {
+                    /* Connection closed cleanly - treat like SSL_ERROR_ZERO_RETURN */
+                    if (req->recv_len >= 4) {
+                        bool has_complete_headers = false;
+                        for (size_t i = 0; i <= req->recv_len - 4; i++) {
+                            if (memcmp(req->recv_buf + i, "\r\n\r\n", 4) == 0) {
+                                has_complete_headers = true;
+                                break;
+                            }
+                        }
+                        if (has_complete_headers) {
+                            /* Have complete headers, process them */
+                            received = 0;  /* Set to 0 to skip recv_len increment below */
+                            goto ssl_process_headers;
+                        }
+                    }
+                    async_request_set_error(req, -1, "Connection closed by peer before complete headers");
+                    return ASYNC_STATUS_ERROR;
+                }
+                /* Fall through to regular error handling if not EOF */
+                #ifdef _WIN32
+                snprintf(req->error_msg, sizeof(req->error_msg), "SSL read failed: system error %d (WSAERR)", sys_err);
+                #else
+                snprintf(req->error_msg, sizeof(req->error_msg), "SSL read failed: system error %d (errno)", errno);
+                #endif
+                req->state = ASYNC_STATE_ERROR;
+                req->error_code = err;
+                return ASYNC_STATUS_ERROR;
             } else {
                 /* Get detailed SSL error */
                 char err_buf[256];
@@ -1707,6 +1744,7 @@ static int step_receiving_body(async_request_t *req) {
                     if (req->response->body) {
                         memcpy(req->response->body, req->recv_buf + body_start, req->content_length);
                         req->response->body_len = req->content_length;
+                        req->response->_body_actual_size = req->content_length;  /* Track allocated size */
                     }
                 }
                 req->response->status_code = 200;  // TODO: Parse from headers
@@ -1914,6 +1952,7 @@ static int step_receiving_body(async_request_t *req) {
                     if (req->response->body) {
                         memcpy(req->response->body, req->recv_buf + body_start, req->content_length);
                         req->response->body_len = req->content_length;
+                        req->response->_body_actual_size = req->content_length;  /* Track allocated size */
                     }
                 }
                 req->response->status_code = 200;  // TODO: Parse from headers

src/core/cookies.c

@@ -112,11 +112,13 @@ char* httpmorph_get_cookies_for_request(httpmorph_session_t *session,
     if (!session || !domain || !path) return NULL;
     if (session->cookie_count == 0) return NULL;
 
-    /* Build cookie header value */
-    char *cookie_header = malloc(4096);
+    /* Build cookie header value with bounds checking */
+    const size_t buffer_size = 4096;
+    char *cookie_header = malloc(buffer_size);
     if (!cookie_header) return NULL;
 
     cookie_header[0] = '\0';
+    size_t used = 0;
     bool first = true;
 
     cookie_t *cookie = session->cookies;
@@ -128,12 +130,22 @@ char* httpmorph_get_cookies_for_request(httpmorph_session_t *session,
         bool secure_match = (!cookie->secure || is_secure);
 
         if (domain_match && path_match && secure_match) {
+            /* Calculate space needed: "; " + name + "=" + value */
+            size_t needed = strlen(cookie->name) + 1 + strlen(cookie->value);
+            if (!first) needed += 2;  /* "; " prefix */
+
+            /* Check if we have space (leave room for null terminator) */
+            if (used + needed >= buffer_size - 1) {
+                /* Buffer would overflow - stop adding cookies */
+                break;
+            }
+
+            /* Safe concatenation with bounds checking */
             if (!first) {
-                strcat(cookie_header, "; ");
+                used += snprintf(cookie_header + used, buffer_size - used, "; ");
             }
-            strcat(cookie_header, cookie->name);
-            strcat(cookie_header, "=");
-            strcat(cookie_header, cookie->value);
+            used += snprintf(cookie_header + used, buffer_size - used, "%s=%s",
+                           cookie->name, cookie->value);
             first = false;
         }
 

src/core/core.c

@@ -166,6 +166,17 @@ httpmorph_response_t* httpmorph_request_execute(
                 ssl = pooled_conn->ssl;
                 use_http2 = pooled_conn->is_http2;  /* Use same protocol as pooled connection */
 
+                /* Restore TLS info from pooled connection BEFORE potential destruction */
+                if (sockfd >= 0) {
+                    /* Connection reused - no connect/TLS time */
+                    connect_time = 0;
+                    response->tls_time_us = 0;
+
+                    if (pooled_conn->ja3_fingerprint) {
+                        response->ja3_fingerprint = strdup(pooled_conn->ja3_fingerprint);
+                    }
+                }
+
                 /* For SSL connections, verify still valid before reuse */
                 if (ssl) {
                     int shutdown_state = SSL_get_shutdown(ssl);
@@ -177,17 +188,6 @@ httpmorph_response_t* httpmorph_request_execute(
                         ssl = NULL;
                     }
                 }
-
-                if (sockfd >= 0) {
-                    /* Connection reused - no connect/TLS time */
-                    connect_time = 0;
-                    response->tls_time_us = 0;
-
-                    /* Restore TLS info from pooled connection */
-                    if (pooled_conn->ja3_fingerprint) {
-                        response->ja3_fingerprint = strdup(pooled_conn->ja3_fingerprint);
-                    }
-                }
             } else {
             }
         }
@@ -457,6 +457,9 @@ httpmorph_response_t* httpmorph_request_execute(
             /* Server wants to close - don't pool */
             if (pooled_conn) {
                 pool_connection_destroy(pooled_conn);
+                /* Clear local references to prevent double-free - pool_connection_destroy already freed them */
+                sockfd = -1;
+                ssl = NULL;
                 pooled_conn = NULL;
             }
             /* Let normal cleanup close the connection */
@@ -473,16 +476,27 @@ httpmorph_response_t* httpmorph_request_execute(
                 conn_to_pool = NULL;
             } else {
                 conn_to_pool = pool_connection_create(host, port, sockfd, ssl, use_http2);
-                /* Store TLS info in pooled connection for future reuse */
+                /* Store TLS info in pooled connection for future reuse with error checking */
                 if (conn_to_pool && ssl) {
+                    bool alloc_failed = false;
+
                     if (response->ja3_fingerprint) {
                         conn_to_pool->ja3_fingerprint = strdup(response->ja3_fingerprint);
+                        if (!conn_to_pool->ja3_fingerprint) alloc_failed = true;
                     }
-                    if (response->tls_version) {
+                    if (response->tls_version && !alloc_failed) {
                         conn_to_pool->tls_version = strdup(response->tls_version);
+                        if (!conn_to_pool->tls_version) alloc_failed = true;
                     }
-                    if (response->tls_cipher) {
+                    if (response->tls_cipher && !alloc_failed) {
                         conn_to_pool->tls_cipher = strdup(response->tls_cipher);
+                        if (!conn_to_pool->tls_cipher) alloc_failed = true;
+                    }
+
+                    /* If any allocation failed, destroy connection instead of pooling */
+                    if (alloc_failed) {
+                        pool_connection_destroy(conn_to_pool);
+                        conn_to_pool = NULL;
                     }
                 }
                 /* Store proxy info for proxy connections */

src/core/http2_logic.c

@@ -106,6 +106,11 @@ static int http2_on_header_callback(nghttp2_session *session,
         stream_data = (http2_stream_data_t *)user_data;
     }
 
+    /* Safety check: if stream_data is still NULL, reject callback */
+    if (!stream_data) {
+        return NGHTTP2_ERR_CALLBACK_FAILURE;
+    }
+
     if (frame->hd.type != NGHTTP2_HEADERS || frame->headers.cat != NGHTTP2_HCAT_RESPONSE) {
         return 0;
     }
@@ -135,6 +140,11 @@ static int http2_on_data_chunk_recv_callback(nghttp2_session *session, uint8_t f
         stream_data = (http2_stream_data_t *)user_data;
     }
 
+    /* Safety check: if stream_data is still NULL, reject callback */
+    if (!stream_data) {
+        return NGHTTP2_ERR_CALLBACK_FAILURE;
+    }
+
     /* Expand buffer if needed */
     if (stream_data->data_len + len > stream_data->data_capacity) {
         /* Calculate sum first to check for overflow */

src/core/network.c

@@ -221,9 +221,24 @@ static void dns_cache_add(const char *hostname, uint16_t port,
         return;
     }
 
+    /* Allocate hostname with error checking */
     entry->hostname = strdup(hostname);
-    entry->port = port;
+    if (!entry->hostname) {
+        free(entry);
+        dns_cache_unlock();
+        return;
+    }
+
+    /* Deep copy addrinfo with error checking */
     entry->result = addrinfo_deep_copy(result);
+    if (!entry->result) {
+        free(entry->hostname);
+        free(entry);
+        dns_cache_unlock();
+        return;
+    }
+
+    entry->port = port;
     entry->expires = time(NULL) + DNS_CACHE_TTL_SECONDS;
     entry->next = dns_cache_head;
 

src/core/request_builder.c

@@ -21,9 +21,20 @@ static int ensure_capacity(request_builder_t *builder, size_t needed) {
         return 0;  /* Enough space */
     }
 
-    /* Calculate new capacity */
+    /* Calculate new capacity with overflow protection */
     size_t new_capacity = builder->capacity;
-    while (new_capacity < builder->len + needed) {
+    size_t target = builder->len + needed;
+
+    /* Check for overflow in target calculation */
+    if (target < builder->len) {
+        return -1;  /* Overflow detected */
+    }
+
+    while (new_capacity < target) {
+        /* Check for overflow before multiplication */
+        if (new_capacity > SIZE_MAX / GROWTH_FACTOR) {
+            return -1;  /* Would overflow */
+        }
         new_capacity *= GROWTH_FACTOR;
     }
 

src/core/tls.c

@@ -124,14 +124,27 @@ int httpmorph_configure_ssl_ctx(SSL_CTX *ctx, const browser_profile_t *profile)
         }
 
         if (name) {
+            size_t name_len = strlen(name);
             if (is_tls13) {
+                /* Check bounds before adding to TLS 1.3 buffer */
+                size_t space_needed = name_len + (p13 != tls13_ciphers ? 1 : 0);  /* +1 for ':' */
+                if ((size_t)(p13 - tls13_ciphers) + space_needed >= sizeof(tls13_ciphers)) {
+                    continue;  /* Skip this cipher - would overflow */
+                }
                 if (p13 != tls13_ciphers) *p13++ = ':';
-                strcpy(p13, name);
-                p13 += strlen(name);
+                memcpy(p13, name, name_len);
+                p13 += name_len;
+                *p13 = '\0';  /* Ensure null termination */
             } else {
+                /* Check bounds before adding to TLS 1.2 buffer */
+                size_t space_needed = name_len + (p12 != tls12_ciphers ? 1 : 0);  /* +1 for ':' */
+                if ((size_t)(p12 - tls12_ciphers) + space_needed >= sizeof(tls12_ciphers)) {
+                    continue;  /* Skip this cipher - would overflow */
+                }
                 if (p12 != tls12_ciphers) *p12++ = ':';
-                strcpy(p12, name);
-                p12 += strlen(name);
+                memcpy(p12, name, name_len);
+                p12 += name_len;
+                *p12 = '\0';  /* Ensure null termination */
             }
         }
     }
@@ -141,9 +154,9 @@ int httpmorph_configure_ssl_ctx(SSL_CTX *ctx, const browser_profile_t *profile)
     if (strlen(tls13_ciphers) > 0 && strlen(tls12_ciphers) > 0) {
         snprintf(combined_ciphers, sizeof(combined_ciphers), "%s:%s", tls13_ciphers, tls12_ciphers);
     } else if (strlen(tls13_ciphers) > 0) {
-        strcpy(combined_ciphers, tls13_ciphers);
+        snprintf(combined_ciphers, sizeof(combined_ciphers), "%s", tls13_ciphers);
     } else if (strlen(tls12_ciphers) > 0) {
-        strcpy(combined_ciphers, tls12_ciphers);
+        snprintf(combined_ciphers, sizeof(combined_ciphers), "%s", tls12_ciphers);
     }
 
     /* Use strict cipher list to preserve exact order */