Fix dangling pointer bug in URL storage
The previous implementation stored parsed_url which contained pointers into the user's string literal that could be invalidated. This fix: 1. Allocates a copy of the URL string (url_buffer) 2. Parses the copy so all pointers reference owned memory 3. Removes redundant domain and path fields from HTTP_ClientConn 4. Updates save_cookies to use url.authority.host.text and url.path 5. Updates cleanup code to free url_buffer instead of domain.ptr This ensures the URL and all its components remain valid for the lifetime of the connection.
This commit is contained in:
@@ -3879,27 +3879,20 @@ void http_request_builder_url(HTTP_RequestBuilder builder,
|
|||||||
if (conn->state != HTTP_CLIENT_CONN_WAIT_LINE)
|
if (conn->state != HTTP_CLIENT_CONN_WAIT_LINE)
|
||||||
return; // Request line already written
|
return; // Request line already written
|
||||||
|
|
||||||
// Parse the URL to extract components
|
// Allocate a copy of the URL string so the parsed URL pointers remain valid
|
||||||
|
char *url_copy = malloc(url.len);
|
||||||
|
if (url_copy == NULL)
|
||||||
|
return; // TODO: set error
|
||||||
|
memcpy(url_copy, url.ptr, url.len);
|
||||||
|
|
||||||
|
// Parse the copied URL
|
||||||
HTTP_URL parsed_url;
|
HTTP_URL parsed_url;
|
||||||
if (http_parse_url(url.ptr, url.len, &parsed_url) != 1)
|
if (http_parse_url(url_copy, url.len, &parsed_url) != 1) {
|
||||||
|
free(url_copy);
|
||||||
return; // TODO: set error
|
return; // TODO: set error
|
||||||
|
}
|
||||||
|
|
||||||
HTTP_String domain = parsed_url.authority.host.text;
|
conn->url_buffer = url_copy;
|
||||||
HTTP_String path = parsed_url.path;
|
|
||||||
char *p = malloc(domain.len + path.len);
|
|
||||||
if (p == NULL)
|
|
||||||
return; // TODO: set error
|
|
||||||
|
|
||||||
memcpy(p, domain.ptr, domain.len);
|
|
||||||
domain.ptr = p;
|
|
||||||
p += domain.len;
|
|
||||||
|
|
||||||
memcpy(p, path.ptr, path.len);
|
|
||||||
path.ptr = p;
|
|
||||||
p += path.len;
|
|
||||||
|
|
||||||
conn->domain = domain;
|
|
||||||
conn->path = path;
|
|
||||||
conn->url = parsed_url;
|
conn->url = parsed_url;
|
||||||
|
|
||||||
// Write method
|
// Write method
|
||||||
@@ -4065,7 +4058,7 @@ int http_request_builder_send(HTTP_RequestBuilder builder)
|
|||||||
|
|
||||||
error:
|
error:
|
||||||
conn->state = HTTP_CLIENT_CONN_FREE;
|
conn->state = HTTP_CLIENT_CONN_FREE;
|
||||||
free(conn->domain.ptr);
|
free(conn->url_buffer);
|
||||||
byte_queue_free(&conn->input);
|
byte_queue_free(&conn->input);
|
||||||
byte_queue_free(&conn->output);
|
byte_queue_free(&conn->output);
|
||||||
client->num_conns--;
|
client->num_conns--;
|
||||||
@@ -4248,8 +4241,8 @@ int http_client_process_events(HTTP_Client *client,
|
|||||||
save_cookies(&client->cookie_jar,
|
save_cookies(&client->cookie_jar,
|
||||||
conn->response.headers,
|
conn->response.headers,
|
||||||
conn->response.num_headers,
|
conn->response.num_headers,
|
||||||
conn->domain,
|
conn->url.authority.host.text,
|
||||||
conn->path);
|
conn->url.path);
|
||||||
|
|
||||||
// TODO: Handle redirects here
|
// TODO: Handle redirects here
|
||||||
}
|
}
|
||||||
@@ -4313,7 +4306,7 @@ void http_free_response(HTTP_Response *response)
|
|||||||
return;
|
return;
|
||||||
|
|
||||||
conn->state = HTTP_CLIENT_CONN_FREE;
|
conn->state = HTTP_CLIENT_CONN_FREE;
|
||||||
free(conn->domain.ptr);
|
free(conn->url_buffer);
|
||||||
byte_queue_free(&conn->input);
|
byte_queue_free(&conn->input);
|
||||||
byte_queue_free(&conn->output);
|
byte_queue_free(&conn->output);
|
||||||
client->num_conns--;
|
client->num_conns--;
|
||||||
|
|||||||
@@ -894,12 +894,10 @@ typedef struct {
|
|||||||
// TODO: comment
|
// TODO: comment
|
||||||
bool trace_bytes;
|
bool trace_bytes;
|
||||||
|
|
||||||
// Endpoint domain and path. These must be saved
|
|
||||||
// in case the server wants to set some cookies.
|
|
||||||
HTTP_String domain;
|
|
||||||
HTTP_String path;
|
|
||||||
|
|
||||||
// Parsed URL for connection establishment
|
// Parsed URL for connection establishment
|
||||||
|
// The url_buffer contains the allocated copy of the URL string,
|
||||||
|
// and url.* pointers reference into this buffer
|
||||||
|
char *url_buffer;
|
||||||
HTTP_URL url;
|
HTTP_URL url;
|
||||||
|
|
||||||
// Data received from the server
|
// Data received from the server
|
||||||
|
|||||||
+15
-22
@@ -218,27 +218,20 @@ void http_request_builder_url(HTTP_RequestBuilder builder,
|
|||||||
if (conn->state != HTTP_CLIENT_CONN_WAIT_LINE)
|
if (conn->state != HTTP_CLIENT_CONN_WAIT_LINE)
|
||||||
return; // Request line already written
|
return; // Request line already written
|
||||||
|
|
||||||
// Parse the URL to extract components
|
// Allocate a copy of the URL string so the parsed URL pointers remain valid
|
||||||
|
char *url_copy = malloc(url.len);
|
||||||
|
if (url_copy == NULL)
|
||||||
|
return; // TODO: set error
|
||||||
|
memcpy(url_copy, url.ptr, url.len);
|
||||||
|
|
||||||
|
// Parse the copied URL
|
||||||
HTTP_URL parsed_url;
|
HTTP_URL parsed_url;
|
||||||
if (http_parse_url(url.ptr, url.len, &parsed_url) != 1)
|
if (http_parse_url(url_copy, url.len, &parsed_url) != 1) {
|
||||||
|
free(url_copy);
|
||||||
return; // TODO: set error
|
return; // TODO: set error
|
||||||
|
}
|
||||||
|
|
||||||
HTTP_String domain = parsed_url.authority.host.text;
|
conn->url_buffer = url_copy;
|
||||||
HTTP_String path = parsed_url.path;
|
|
||||||
char *p = malloc(domain.len + path.len);
|
|
||||||
if (p == NULL)
|
|
||||||
return; // TODO: set error
|
|
||||||
|
|
||||||
memcpy(p, domain.ptr, domain.len);
|
|
||||||
domain.ptr = p;
|
|
||||||
p += domain.len;
|
|
||||||
|
|
||||||
memcpy(p, path.ptr, path.len);
|
|
||||||
path.ptr = p;
|
|
||||||
p += path.len;
|
|
||||||
|
|
||||||
conn->domain = domain;
|
|
||||||
conn->path = path;
|
|
||||||
conn->url = parsed_url;
|
conn->url = parsed_url;
|
||||||
|
|
||||||
// Write method
|
// Write method
|
||||||
@@ -404,7 +397,7 @@ int http_request_builder_send(HTTP_RequestBuilder builder)
|
|||||||
|
|
||||||
error:
|
error:
|
||||||
conn->state = HTTP_CLIENT_CONN_FREE;
|
conn->state = HTTP_CLIENT_CONN_FREE;
|
||||||
free(conn->domain.ptr);
|
free(conn->url_buffer);
|
||||||
byte_queue_free(&conn->input);
|
byte_queue_free(&conn->input);
|
||||||
byte_queue_free(&conn->output);
|
byte_queue_free(&conn->output);
|
||||||
client->num_conns--;
|
client->num_conns--;
|
||||||
@@ -587,8 +580,8 @@ int http_client_process_events(HTTP_Client *client,
|
|||||||
save_cookies(&client->cookie_jar,
|
save_cookies(&client->cookie_jar,
|
||||||
conn->response.headers,
|
conn->response.headers,
|
||||||
conn->response.num_headers,
|
conn->response.num_headers,
|
||||||
conn->domain,
|
conn->url.authority.host.text,
|
||||||
conn->path);
|
conn->url.path);
|
||||||
|
|
||||||
// TODO: Handle redirects here
|
// TODO: Handle redirects here
|
||||||
}
|
}
|
||||||
@@ -652,7 +645,7 @@ void http_free_response(HTTP_Response *response)
|
|||||||
return;
|
return;
|
||||||
|
|
||||||
conn->state = HTTP_CLIENT_CONN_FREE;
|
conn->state = HTTP_CLIENT_CONN_FREE;
|
||||||
free(conn->domain.ptr);
|
free(conn->url_buffer);
|
||||||
byte_queue_free(&conn->input);
|
byte_queue_free(&conn->input);
|
||||||
byte_queue_free(&conn->output);
|
byte_queue_free(&conn->output);
|
||||||
client->num_conns--;
|
client->num_conns--;
|
||||||
|
|||||||
+3
-5
@@ -75,12 +75,10 @@ typedef struct {
|
|||||||
// TODO: comment
|
// TODO: comment
|
||||||
bool trace_bytes;
|
bool trace_bytes;
|
||||||
|
|
||||||
// Endpoint domain and path. These must be saved
|
|
||||||
// in case the server wants to set some cookies.
|
|
||||||
HTTP_String domain;
|
|
||||||
HTTP_String path;
|
|
||||||
|
|
||||||
// Parsed URL for connection establishment
|
// Parsed URL for connection establishment
|
||||||
|
// The url_buffer contains the allocated copy of the URL string,
|
||||||
|
// and url.* pointers reference into this buffer
|
||||||
|
char *url_buffer;
|
||||||
HTTP_URL url;
|
HTTP_URL url;
|
||||||
|
|
||||||
// Data received from the server
|
// Data received from the server
|
||||||
|
|||||||
Reference in New Issue
Block a user