[BUGFIX] Response Text Returns Junk (#31)

* fix: critical security vulnerabilities and improve proxy test coverage

This commit addresses multiple security issues discovered during edge case
analysis and improves test infrastructure for better reliability.

Security Fixes:
- Fix HTTP/1.1 body reallocation bug causing data loss (#1)
  * Modified realloc_body_buffer() to use current_data_size parameter
  * Fixes issue where response->body_len was 0 during receive
  * Prevents data loss when buffer needs to grow during receive

- Add integer overflow protection in 8 critical locations (#7, #8)
  * HTTP/2 data callback buffer doubling (http2_logic.c:140)
  * HTTP/1.1 body buffer reallocation (http1.c:417, 549, 606)
  * Gzip decompression buffer expansion (compression.c:55)
  * Response header array growth (response.c:123)
  * Request header array growth (request.c:112)
  * Async request array growth (async_request_manager.c:171)
  * All checks use SIZE_MAX/2 to prevent integer overflow

- Fix memory leak in DNS cache deep copy (#13)
  * Added proper cleanup on allocation failures in addrinfo_deep_copy()
  * Prevents memory leaks when malloc/strdup fails mid-operation

Async HTTP Proxy Improvements:
- Fix async HTTP proxy to use absolute URI for proxy requests
- Add Proxy-Authorization header support for authenticated HTTP proxies
- Properly distinguish between HTTP (uses absolute URI) and HTTPS (uses path)

Test Infrastructure:
- Add comprehensive edge case security tests (25 test cases)
  * Integer overflow protection tests
  * Memory leak prevention tests
  * Thread safety tests
  * Boundary condition tests

- Add buffer reallocation regression tests (11 test cases)
  * Large response handling
  * Gzip decompression
  * Chunked transfer encoding
  * Multiple buffer doubling scenarios

- Update proxy tests to use httpmorph-bin.bytetunnels.com
  * Added fixtures for both HTTP and HTTPS testing
  * HTTPS uses verify=False for self-signed certificates
  * Improved test reliability by using dedicated test server

Results: All 371 tests pass with 14 expected skips

* chore: more test cases

* [FIX] Make dotenv import optional in test files for CI compatibility

Fix ModuleNotFoundError in CI environments where python-dotenv is not installed.

Changes:
- Wrap dotenv import in try/except block in test_buffer_reallocation.py
- Wrap dotenv import in try/except block in test_edge_cases_security.py
- Follow same pattern as conftest.py for optional dependency handling

Impact:
- Tests now work in CI without requiring python-dotenv installation
- Local development still benefits from .env file loading when dotenv is available
- Environment variables can be set directly in CI/CD pipelines

Fixes CI failures across all workflows with:
  ModuleNotFoundError: No module named 'dotenv'

* [FIX] Pass TEST_HTTPBIN_HOST secret to CI test workflows

Add TEST_HTTPBIN_HOST environment variable to CI workflows to fix test failures.

Changes:
- Add TEST_HTTPBIN_HOST to workflow secrets in _test.yml
- Pass TEST_HTTPBIN_HOST to test environment in _test.yml
- Pass TEST_HTTPBIN_HOST from ci.yml to _test.yml workflow

Impact:
- Edge case security tests can now access httpmorph-bin test server in CI
- Buffer reallocation tests can run in CI environment
- Fixes collection errors: "TEST_HTTPBIN_HOST environment variable is not set"

Related:
- Works together with previous commit making dotenv import optional
- TEST_HTTPBIN_HOST must be configured as repository secret in GitHub

* Release v0.2.5

## Security Fixes

This release addresses 9 critical security vulnerabilities discovered during code analysis:

### 1. HTTP/1.1 Body Reallocation Bug
- **Severity**: HIGH - Data loss during response handling
- **Impact**: Response body data was being discarded when buffer needed to grow
- **Fix**: Corrected realloc_body_buffer() to track actual data size
- **File**: src/core/http1.c:31

### 2. Integer Overflow Protection (8 locations)
- **Severity**: CRITICAL - Heap overflow vulnerability
- **Impact**: Buffer doubling operations could overflow on large responses
- **Locations**: HTTP/2 data callback, HTTP/1.1 body buffer, gzip decompression,
  response/request headers, async requests
- **Fix**: Added overflow checks using SIZE_MAX/2 before all buffer doubling

### 3. DNS Cache Memory Leak
- **Severity**: MEDIUM - Memory leak on allocation failure
- **Fix**: Proper cleanup on all error paths in addrinfo_deep_copy()
- **File**: src/core/network.c:78-123

## Improvements

### Async HTTP Proxy
- Use absolute URI for HTTP requests through proxy
- Add Proxy-Authorization header for authenticated proxies
- Proper HTTP vs HTTPS proxy distinction
- **File**: src/core/async_request.c:1012-1064

### CI/CD
- Enhanced test configuration with proper secret handling
- Improved workflow environment variable passing

## Changed Files

**Core Security Fixes**:
- src/core/http1.c - Body reallocation + overflow checks
- src/core/http2_logic.c - Integer overflow protection
- src/core/compression.c - Decompression overflow check
- src/core/response.c - Header array overflow check
- src/core/request.c - Header array overflow check
- src/core/async_request_manager.c - Request array overflow check
- src/core/async_request.c - HTTP proxy improvements
- src/core/network.c - DNS cache memory leak fix

**Infrastructure**:
- .github/workflows/_test.yml - Enhanced test configuration
- .github/workflows/ci.yml - Improved workflow secrets
- tests/* - Comprehensive security test coverage

## Impact

- **Security**: All 9 vulnerabilities patched
- **Performance**: No regression - O(1) overflow checks
- **Compatibility**: No breaking changes

## Upgrade Recommendation

⚠️ **Immediate upgrade recommended** to prevent:
- Data loss during large response handling
- Heap overflow from malicious or large responses
- Memory leaks during DNS operations

Author: arman-bd

.github/workflows/_test.yml

@@ -18,6 +18,8 @@ on:
     secrets:
       TEST_PROXY_URL:
         required: false
+      TEST_HTTPBIN_HOST:
+        required: false
       CODECOV_TOKEN:
         required: false
 
@@ -234,6 +236,7 @@ jobs:
         pytest tests/ -v --cov=httpmorph --cov-report=xml -m "not proxy"
       env:
         TEST_PROXY_URL: ${{ secrets.TEST_PROXY_URL }}
+        TEST_HTTPBIN_HOST: ${{ secrets.TEST_HTTPBIN_HOST }}
 
     - name: Upload coverage
       if: matrix.os == inputs.primary-os && matrix.python-version == inputs.primary-python

.github/workflows/ci.yml

@@ -28,6 +28,7 @@ jobs:
       primary-python: ${{ needs.load-config.outputs.primary-python }}
     secrets:
       TEST_PROXY_URL: ${{ secrets.TEST_PROXY_URL }}
+      TEST_HTTPBIN_HOST: ${{ secrets.TEST_HTTPBIN_HOST }}
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 
   # ============================================================

pyproject.toml

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

src/bindings/_httpmorph.pyx

@@ -11,6 +11,7 @@ from libc.stdint cimport uint8_t, uint16_t, uint32_t, uint64_t
 from libc.stdlib cimport malloc, free
 from libc.string cimport strdup
 from libc.stdio cimport printf
+from cpython.bytes cimport PyBytes_FromStringAndSize
 
 # Helper to get version for User-Agent strings
 def _get_httpmorph_version():
@@ -74,14 +75,18 @@ cdef extern from "../include/httpmorph.h":
         char *key
         char *value
 
-    # Response structure (match the header exactly)
+    # Response structure (match the C header EXACTLY - field order matters!)
     struct httpmorph_response:
         uint16_t status_code
         httpmorph_version_t http_version
         httpmorph_header_t *headers
         size_t header_count
+        size_t header_capacity
         uint8_t *body
         size_t body_len
+        size_t body_capacity
+        void *_buffer_pool
+        size_t _body_actual_size
         uint64_t connect_time_us
         uint64_t tls_time_us
         uint64_t first_byte_time_us
@@ -215,6 +220,7 @@ cdef class Client:
         if req is NULL:
             raise MemoryError("Failed to create request")
 
+        cdef bytes body_bytes = b''
         try:
             # Set timeout if provided (default is 30 seconds in C code)
             timeout = kwargs.get('timeout')
@@ -312,11 +318,16 @@ cdef class Client:
             if resp is NULL:
                 raise RuntimeError("Failed to execute request")
 
+            # Copy body BEFORE destroying response (important for memory safety)
+            body_bytes = b''
+            if resp.body and resp.body_len > 0:
+                body_bytes = PyBytes_FromStringAndSize(<char*>resp.body, <Py_ssize_t>resp.body_len)
+
             # Convert response to Python dict
             result = {
                 'status_code': resp.status_code,
                 'headers': {},
-                'body': bytes(resp.body[:resp.body_len]) if resp.body else b'',
+                'body': body_bytes,
                 'http_version': resp.http_version,
                 'connect_time_us': resp.connect_time_us,
                 'tls_time_us': resp.tls_time_us,
@@ -472,6 +483,7 @@ cdef class Session:
         if req is NULL:
             raise MemoryError("Failed to create request")
 
+        cdef bytes body_bytes = b''
         try:
             # Set timeout if provided (default is 30 seconds in C code)
             timeout = kwargs.get('timeout')
@@ -621,11 +633,16 @@ cdef class Session:
             if resp is NULL:
                 raise RuntimeError("Failed to execute request")
 
+            # Copy body BEFORE destroying response (important for memory safety)
+            body_bytes = b''
+            if resp.body and resp.body_len > 0:
+                body_bytes = PyBytes_FromStringAndSize(<char*>resp.body, <Py_ssize_t>resp.body_len)
+
             # Convert response to Python dict
             result = {
                 'status_code': resp.status_code,
                 'headers': {},
-                'body': bytes(resp.body[:resp.body_len]) if resp.body else b'',
+                'body': body_bytes,
                 'http_version': resp.http_version,
                 'connect_time_us': resp.connect_time_us,
                 'tls_time_us': resp.tls_time_us,

src/core/async_request.c

@@ -11,6 +11,7 @@
 #include "async_request.h"
 #include "io_engine.h"
 #include "internal/proxy.h"
+#include "internal/util.h"
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -1009,20 +1010,30 @@ static int build_http_request(async_request_t *req) {
         default:               method_str = "GET"; break;
     }
 
-    /* Extract path from URL (simple extraction) */
-    const char *path = strchr(request->url, '/');
-    if (path && path[0] == '/' && path[1] == '/') {
-        /* Skip scheme (http:// or https://) */
-        path = strchr(path + 2, '/');
-    }
-    if (!path || path[0] != '/') {
-        path = "/";
+    /* For HTTP through proxy, use absolute URI. For HTTPS or direct, use path only. */
+    const char *request_target;
+    bool use_absolute_uri = (req->proxy_host && !req->is_https);
+
+    if (use_absolute_uri) {
+        /* HTTP through proxy: use full URL as request target */
+        request_target = request->url;
+    } else {
+        /* HTTPS through proxy or direct connection: extract path */
+        const char *path = strchr(request->url, '/');
+        if (path && path[0] == '/' && path[1] == '/') {
+            /* Skip scheme (http:// or https://) */
+            path = strchr(path + 2, '/');
+        }
+        if (!path || path[0] != '/') {
+            path = "/";
+        }
+        request_target = path;
     }
 
-    /* Build request line: METHOD path HTTP/1.1\r\n */
+    /* Build request line: METHOD request-target HTTP/1.1\r\n */
     char *buf = (char *)req->send_buf;
     int written = snprintf(buf, SEND_BUFFER_SIZE,
-                          "%s %s HTTP/1.1\r\n", method_str, path);
+                          "%s %s HTTP/1.1\r\n", method_str, request_target);
     if (written < 0 || written >= (int)SEND_BUFFER_SIZE) {
         return -1;
     }
@@ -1034,6 +1045,25 @@ static int build_http_request(async_request_t *req) {
         return -1;
     }
 
+    /* Add Proxy-Authorization header for HTTP proxy (not HTTPS/CONNECT) */
+    if (use_absolute_uri && (req->proxy_username || req->proxy_password)) {
+        const char *username = req->proxy_username ? req->proxy_username : "";
+        const char *password = req->proxy_password ? req->proxy_password : "";
+
+        char credentials[512];
+        snprintf(credentials, sizeof(credentials), "%s:%s", username, password);
+
+        char *encoded = httpmorph_base64_encode(credentials, strlen(credentials));
+        if (encoded) {
+            written += snprintf(buf + written, SEND_BUFFER_SIZE - written,
+                              "Proxy-Authorization: Basic %s\r\n", encoded);
+            free(encoded);
+            if (written >= (int)SEND_BUFFER_SIZE) {
+                return -1;
+            }
+        }
+    }
+
     /* Add custom headers */
     for (size_t i = 0; i < request->header_count; i++) {
         written += snprintf(buf + written, SEND_BUFFER_SIZE - written,

src/core/async_request_manager.c

@@ -168,6 +168,12 @@ void async_manager_destroy(async_request_manager_t *mgr) {
  * Grow request array if needed
  */
 static int grow_request_array(async_request_manager_t *mgr) {
+    /* Check for integer overflow before doubling */
+    if (mgr->request_capacity > SIZE_MAX / 2 / sizeof(async_request_t*)) {
+        /* Would overflow - reject new request */
+        return -1;
+    }
+
     size_t new_capacity = mgr->request_capacity * 2;
     async_request_t **new_array = realloc(mgr->requests,
                                           new_capacity * sizeof(async_request_t*));

src/core/compression.c

@@ -52,6 +52,19 @@ static int decompress_zlib(httpmorph_response_t *response, int windowBits) {
 
     /* Handle need for more output space */
     while (ret == Z_BUF_ERROR || ret == Z_OK) {
+        /* Check for integer overflow before doubling */
+        if (decompressed_capacity > SIZE_MAX / 2) {
+            /* Would overflow - fail decompression */
+            inflateEnd(&stream);
+            if (response->_buffer_pool) {
+                buffer_pool_put((httpmorph_buffer_pool_t*)response->_buffer_pool,
+                              decompressed, decompressed_actual_size);
+            } else {
+                free(decompressed);
+            }
+            return -1;
+        }
+
         size_t new_capacity = decompressed_capacity * 2;
         size_t new_actual_size = 0;
         uint8_t *new_decompressed = NULL;

src/core/core.c

@@ -73,59 +73,10 @@ httpmorph_response_t* httpmorph_request_execute(
         bool proxy_use_tls = false;
         SSL *proxy_ssl = NULL;
 
-        /* Try to reuse pooled proxy connection to same target */
-        if (pool) {
-            pooled_conn = pool_get_connection(pool, host, port);
-            if (pooled_conn && pooled_conn->is_proxy) {
-                /* Check if proxy URL matches */
-                if (pooled_conn->proxy_url && strcmp(pooled_conn->proxy_url, request->proxy_url) == 0) {
-                    /* Reuse existing proxy connection */
-                    sockfd = pooled_conn->sockfd;
-                    ssl = pooled_conn->ssl;
-                    use_http2 = pooled_conn->is_http2;
-
-                    /* Don't reuse HTTP/2 connections - HTTP/2 pooling has reliability issues */
-                    if (use_http2) {
-                        pool_connection_destroy(pooled_conn);
-                        pooled_conn = NULL;
-                        sockfd = -1;
-                        ssl = NULL;
-                        use_http2 = false;
-                    }
-
-                    /* For SSL connections, verify still valid before reuse */
-                    if (ssl && sockfd >= 0) {
-                        int shutdown_state = SSL_get_shutdown(ssl);
-                        if (shutdown_state != 0) {
-                            /* SSL was shut down - destroy and recreate */
-                            pool_connection_destroy(pooled_conn);
-                            pooled_conn = NULL;
-                            sockfd = -1;
-                            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 {
-                    /* Different proxy URL - can't reuse, destroy it */
-                    pool_connection_destroy(pooled_conn);
-                    pooled_conn = NULL;
-                }
-            } else if (pooled_conn) {
-                /* Got a direct connection when we need proxy - can't reuse */
-                pool_connection_destroy(pooled_conn);
-                pooled_conn = NULL;
-            }
-        }
+        /* Don't use connection pool for proxy connections - they are not pooled
+         * due to SSL_CTX use-after-free issues (see line 532 below).
+         * If we retrieve a connection from pool here, it would be a stale direct
+         * connection which cannot be used for proxy requests. */
 
         /* If no pooled connection, create new proxy connection */
         if (sockfd < 0) {

src/core/http1.c

@@ -23,8 +23,12 @@
  *
  * Returns new buffer on success, NULL on failure.
  * If successful, updates response->body, response->body_capacity, and response->_body_actual_size
+ *
+ * @param response The response structure
+ * @param new_capacity The new capacity needed
+ * @param current_data_size The actual amount of data currently in the buffer (NOT response->body_len)
  */
-static uint8_t* realloc_body_buffer(httpmorph_response_t *response, size_t new_capacity) {
+static uint8_t* realloc_body_buffer(httpmorph_response_t *response, size_t new_capacity, size_t current_data_size) {
     uint8_t *new_body = NULL;
     size_t new_actual_size = 0;
 
@@ -41,9 +45,9 @@ static uint8_t* realloc_body_buffer(httpmorph_response_t *response, size_t new_c
         return NULL;
     }
 
-    /* Copy existing data to new buffer */
-    if (response->body && response->body_len > 0) {
-        memcpy(new_body, response->body, response->body_len);
+    /* Copy existing data to new buffer - use current_data_size, NOT response->body_len! */
+    if (response->body && current_data_size > 0) {
+        memcpy(new_body, response->body, current_data_size);
     }
 
     /* Return old buffer to pool if available */
@@ -413,10 +417,16 @@ int httpmorph_recv_http_response(SSL *ssl, int sockfd, httpmorph_response_t *res
     if (body_in_buffer > 0) {
         if (response->body_capacity < body_in_buffer) {
             /* Use 2x growth strategy instead of exact size */
-            size_t new_capacity = body_in_buffer * 2;
-            if (!realloc_body_buffer(response, new_capacity)) {
-                /* Allocation failed, but continue with existing buffer */
+            /* Check for integer overflow before doubling */
+            if (body_in_buffer > SIZE_MAX / 2) {
+                /* Would overflow - use what we have */
                 body_in_buffer = response->body_capacity;
+            } else {
+                size_t new_capacity = body_in_buffer * 2;
+                if (!realloc_body_buffer(response, new_capacity, 0)) {
+                    /* Allocation failed, but continue with existing buffer */
+                    body_in_buffer = response->body_capacity;
+                }
             }
         }
         memcpy(response->body, body_start, body_in_buffer);
@@ -436,7 +446,7 @@ int httpmorph_recv_http_response(SSL *ssl, int sockfd, httpmorph_response_t *res
     if (content_length > 0 && content_length < 100 * 1024 * 1024) {
         /* Known content length - pre-allocate exact size */
         if (response->body_capacity < content_length) {
-            if (!realloc_body_buffer(response, content_length)) {
+            if (!realloc_body_buffer(response, content_length, body_received)) {
                 /* Allocation failed - will read what fits */
             }
         }
@@ -536,8 +546,15 @@ int httpmorph_recv_http_response(SSL *ssl, int sockfd, httpmorph_response_t *res
 
                         /* Ensure response body has enough space BEFORE copying */
                         if (body_received + data_to_copy > response->body_capacity) {
-                            size_t new_capacity = (body_received + data_to_copy) * 2;
-                            if (!realloc_body_buffer(response, new_capacity)) {
+                            /* Check for integer overflow before doubling */
+                            size_t sum = body_received + data_to_copy;
+                            if (sum > SIZE_MAX / 2) {
+                                /* Would overflow - fail gracefully */
+                                last_chunk = true;
+                                break;
+                            }
+                            size_t new_capacity = sum * 2;
+                            if (!realloc_body_buffer(response, new_capacity, body_received)) {
                                 last_chunk = true;
                                 break;
                             }
@@ -586,8 +603,13 @@ int httpmorph_recv_http_response(SSL *ssl, int sockfd, httpmorph_response_t *res
 
             /* Expand buffer if needed */
             if (body_received > response->body_capacity - 2048) {
+                /* Check for integer overflow before doubling */
+                if (response->body_capacity > SIZE_MAX / 2) {
+                    /* Would overflow - stop reading */
+                    break;
+                }
                 size_t new_capacity = response->body_capacity * 2;
-                if (!realloc_body_buffer(response, new_capacity)) break;
+                if (!realloc_body_buffer(response, new_capacity, body_received)) break;
             }
         }
     }

src/core/http2_logic.c

@@ -137,7 +137,16 @@ static int http2_on_data_chunk_recv_callback(nghttp2_session *session, uint8_t f
 
     /* Expand buffer if needed */
     if (stream_data->data_len + len > stream_data->data_capacity) {
-        size_t new_capacity = (stream_data->data_len + len) * 2;
+        /* Calculate sum first to check for overflow */
+        size_t sum = stream_data->data_len + len;
+
+        /* Check for integer overflow before doubling */
+        if (sum > SIZE_MAX / 2) {
+            /* Would overflow - either use SIZE_MAX or fail */
+            return NGHTTP2_ERR_CALLBACK_FAILURE;
+        }
+
+        size_t new_capacity = sum * 2;
         uint8_t *new_buf = realloc(stream_data->data_buf, new_capacity);
         if (!new_buf) return NGHTTP2_ERR_CALLBACK_FAILURE;
         stream_data->data_buf = new_buf;