Skip to content

Commit fb0f5b1

Browse files
committed
Properly handle bad characters in stream names
* Factor out path_utils.c:sanitize_stream_name() for path name and MQTT topics * Factor out url_utils.c:simple_url_escape() for URLs * Call url_decode() in http_request_extract_path_param() * Remove duplicate calls to url_decode() * Remove duplicate url_decode() prototype in web/web_server.h * Use MAX_STREAM_NAME and MAX_PATH_LENGTH consistently instead of integer literals * Remove inconsistent MAX_STREAM_NAME_LENGTH from storage_manager.c * Add and use MAX_TOPIC_LENGTH in MQTT instead of integer literals This is a general cleanup of string handling around paths and URLs in addition to specifically handling input validation and encoding of stream names.
1 parent a7e0c3c commit fb0f5b1

38 files changed

Lines changed: 357 additions & 352 deletions

include/core/path_utils.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#ifndef LIGHTNVR_PATH_UTILS_H
2+
#define LIGHTNVR_PATH_UTILS_H
3+
4+
#include <stddef.h>
5+
6+
/**
7+
* Sanitize a stream name for use as a path or object name.
8+
* Replaces non-alphanumeric characters with underscores and lowercases.
9+
*
10+
* @param input The input string
11+
* @param output The output buffer
12+
* @param output_size The size of the output buffer
13+
*/
14+
void sanitize_stream_name(const char *input, char *output, size_t output_size);
15+
16+
#endif /* LIGHTNVR_PATH_UTILS_H */

include/core/url_utils.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,18 @@ int url_build_onvif_device_service_url(const char *url, int onvif_port,
7373
*/
7474
int url_redact_for_logging(const char *url, char *out_url, size_t out_size);
7575

76+
/**
77+
* Format the URL for the API endpoint with query parameters (simple method)
78+
*
79+
* This is the method that works according to user feedback
80+
* URL encode the stream_url to handle special characters
81+
* TODO: it's unclear why we wouldn't use curl_easy_escape here
82+
*
83+
* @param input The input string
84+
* @param output The output buffer
85+
* @param output_size The size of the output buffer
86+
*/
87+
void simple_url_escape(const char *input, char *output, size_t output_size);
88+
89+
7690
#endif /* LIGHTNVR_URL_UTILS_H */

include/database/db_streams.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ int set_stream_retention_config(const char *stream_name, const stream_retention_
110110
/**
111111
* Get all stream names for retention policy processing
112112
*
113-
* @param names Array of stream name buffers (each should be at least 64 chars)
113+
* @param names Array of stream name buffers (each should be MAX_STREAM_NAME chars)
114114
* @param max_count Maximum number of stream names to return
115115
* @return Number of streams found, or -1 on error
116116
*/
117-
int get_all_stream_names(char names[][64], int max_count);
117+
int get_all_stream_names(char names[][MAX_STREAM_NAME], int max_count);
118118

119119
/**
120120
* Get storage usage for a stream in bytes

include/video/packet_buffer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ typedef struct {
7575
time_t newest_packet_time; // Timestamp of newest packet in buffer
7676

7777
// Disk-based buffer (if mode is DISK or HYBRID)
78-
char disk_buffer_path[512]; // Path to disk buffer directory
78+
char disk_buffer_path[MAX_PATH_LENGTH]; // Path to disk buffer directory
7979
FILE *disk_buffer_file; // File handle for disk buffer
8080

8181
// Thread safety

include/video/pre_detection_buffer.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
#ifndef LIGHTNVR_PRE_DETECTION_BUFFER_H
1515
#define LIGHTNVR_PRE_DETECTION_BUFFER_H
1616

17+
#include "core/config.h"
18+
1719
#include <stdint.h>
1820
#include <stdbool.h>
1921
#include <time.h>
@@ -50,7 +52,7 @@ typedef enum {
5052
* Segment information for HLS-based strategies
5153
*/
5254
typedef struct {
53-
char path[512]; // Path to segment file
55+
char path[MAX_PATH_LENGTH]; // Path to segment file
5456
time_t timestamp; // Creation timestamp
5557
float duration; // Estimated duration in seconds
5658
size_t size_bytes; // File size

include/web/request_response.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ typedef void (*request_handler_t)(const http_request_t *request, http_response_t
7373
const char* http_request_get_header(const http_request_t *req, const char *name);
7474

7575
/**
76-
* @brief Get a query parameter value from the request
76+
* @brief Get a URL-decoded query parameter value from the request
7777
* @param req HTTP request
7878
* @param name Parameter name
7979
* @param value Buffer to store the value
@@ -92,7 +92,7 @@ int http_request_get_query_param(const http_request_t *req, const char *name, ch
9292
int http_request_get_body_str(const http_request_t *req, char *buf, size_t buf_len);
9393

9494
/**
95-
* @brief Extract a path parameter from the URI (e.g., /api/streams/:id)
95+
* @brief Extract a URL-decoded path parameter from the URI (e.g., /api/streams/:id)
9696
* @param req HTTP request
9797
* @param prefix URL prefix to strip (e.g., "/api/streams/")
9898
* @param param_buf Buffer to store the extracted parameter

include/web/web_server.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,6 @@ const char *get_request_header(const http_request_t *request, const char *name);
3030
int set_response_header(http_response_t *response, const char *name, const char *value);
3131
int get_web_server_stats(int *active_connections, double *requests_per_second, uint64_t *bytes_sent, uint64_t *bytes_received);
3232

33-
// URL utilities
34-
int url_decode(const char *src, char *dst, size_t dst_size);
35-
3633
// Helper functions
3734
void cleanup_request(http_request_t *request);
3835
void cleanup_response(http_response_t *response);

src/core/logger.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <libgen.h>
1616
#include <syslog.h>
1717

18+
#include "core/config.h"
1819
#include "core/logger.h"
1920
#include "core/logger_json.h"
2021

@@ -550,8 +551,8 @@ int log_rotate(size_t max_size, int max_files) {
550551
}
551552

552553
// Rotate log files
553-
char old_path[512];
554-
char new_path[512];
554+
char old_path[MAX_PATH_LENGTH];
555+
char new_path[MAX_PATH_LENGTH];
555556

556557
// Remove oldest log file if it exists
557558
snprintf(old_path, sizeof(old_path), "%s.%d", logger.log_filename, max_files);

src/core/logger_json.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <libgen.h>
1111
#include <time.h>
1212

13+
#include "core/config.h"
1314
#include "core/logger.h"
1415
#include "core/logger_json.h"
1516
#include <cjson/cJSON.h>
@@ -444,8 +445,8 @@ int json_log_rotate(size_t max_size, int max_files) {
444445
}
445446

446447
// Rotate log files
447-
char old_path[512];
448-
char new_path[512];
448+
char old_path[MAX_PATH_LENGTH];
449+
char new_path[MAX_PATH_LENGTH];
449450

450451
// Remove oldest log file if it exists
451452
snprintf(old_path, sizeof(old_path), "%s.%d", json_logger.log_filename, max_files);

src/core/mqtt_client.c

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
1313

1414
#include "core/mqtt_client.h"
1515
#include "core/logger.h"
16+
#include "core/path_utils.h"
1617
#include "core/version.h"
1718
#include "database/db_streams.h"
1819
#include "video/go2rtc/go2rtc_snapshot.h"
1920

21+
#define MAX_TOPIC_LENGTH 512
22+
2023
// MQTT client state
2124
static struct mosquitto *mosq = NULL;
2225
static const config_t *mqtt_config = NULL;
@@ -129,7 +132,7 @@ int mqtt_init(const config_t *config) {
129132

130133
// Set up Last Will and Testament for HA availability tracking
131134
if (config->mqtt_ha_discovery) {
132-
char lwt_topic[512];
135+
char lwt_topic[MAX_TOPIC_LENGTH];
133136
snprintf(lwt_topic, sizeof(lwt_topic), "%s/availability", config->mqtt_topic_prefix);
134137
rc = mosquitto_will_set(mosq, lwt_topic, (int)strlen("offline"), "offline",
135138
config->mqtt_qos, true);
@@ -217,15 +220,15 @@ static void on_connect(struct mosquitto *m, void *userdata, int rc) {
217220

218221
// Publish availability "online" for HA discovery
219222
if (mqtt_config && mqtt_config->mqtt_ha_discovery) {
220-
char avail_topic[512];
223+
char avail_topic[MAX_TOPIC_LENGTH];
221224
snprintf(avail_topic, sizeof(avail_topic), "%s/availability",
222225
mqtt_config->mqtt_topic_prefix);
223226
mqtt_publish_raw(avail_topic, "online", true);
224227
log_info("MQTT: Published availability 'online' to %s", avail_topic);
225228

226229
// Subscribe to HA birth topic so we can re-publish discovery
227230
// when Home Assistant restarts
228-
char status_topic[512];
231+
char status_topic[MAX_TOPIC_LENGTH];
229232
snprintf(status_topic, sizeof(status_topic), "%s/status",
230233
mqtt_config->mqtt_ha_discovery_prefix);
231234
int sub_rc = mosquitto_subscribe(m, NULL, status_topic, 0);
@@ -290,7 +293,7 @@ static void on_message(struct mosquitto *m, void *userdata, const struct mosquit
290293

291294
// Check if this is the HA birth message (status topic → "online")
292295
if (mqtt_config && mqtt_config->mqtt_ha_discovery) {
293-
char status_topic[512];
296+
char status_topic[MAX_TOPIC_LENGTH];
294297
snprintf(status_topic, sizeof(status_topic), "%s/status",
295298
mqtt_config->mqtt_ha_discovery_prefix);
296299

@@ -348,7 +351,7 @@ int mqtt_publish_detection(const char *stream_name, const detection_result_t *re
348351
}
349352

350353
// Build topic: {prefix}/detections/{stream_name}
351-
char topic[512];
354+
char topic[MAX_TOPIC_LENGTH];
352355
snprintf(topic, sizeof(topic), "%s/detections/%s",
353356
mqtt_config->mqtt_topic_prefix, stream_name);
354357

@@ -470,25 +473,6 @@ int mqtt_publish_binary(const char *topic, const void *data, size_t len, bool re
470473
return 0;
471474
}
472475

473-
/**
474-
* Sanitize a stream name for use as a Home Assistant unique_id / object_id.
475-
* Replaces non-alphanumeric characters with underscores and lowercases.
476-
*/
477-
static void sanitize_stream_name(const char *input, char *output, size_t output_size) {
478-
size_t i = 0;
479-
for (; i < output_size - 1 && input[i] != '\0'; i++) {
480-
char c = input[i];
481-
if ((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9')) {
482-
output[i] = c;
483-
} else if (c >= 'A' && c <= 'Z') {
484-
output[i] = (char)(c + ('a' - 'A'));
485-
} else {
486-
output[i] = '_';
487-
}
488-
}
489-
output[i] = '\0';
490-
}
491-
492476
/**
493477
* Build the common HA device JSON block for lightNVR.
494478
* Caller must free the returned cJSON object.
@@ -555,12 +539,12 @@ int mqtt_publish_ha_discovery(void) {
555539
continue;
556540
}
557541

558-
char safe_name[256];
542+
char safe_name[MAX_STREAM_NAME];
559543
sanitize_stream_name(streams[i].name, safe_name, sizeof(safe_name));
560544

561545
// --- 1. Camera entity (snapshot image via MQTT) ---
562546
{
563-
char topic[512];
547+
char topic[MAX_TOPIC_LENGTH];
564548
snprintf(topic, sizeof(topic), "%s/camera/lightnvr/%s/config", prefix, safe_name);
565549

566550
cJSON *payload = cJSON_CreateObject();
@@ -570,11 +554,9 @@ int mqtt_publish_ha_discovery(void) {
570554
snprintf(unique_id, sizeof(unique_id), "lightnvr_%s_camera", safe_name);
571555
cJSON_AddStringToObject(payload, "unique_id", unique_id);
572556

573-
char name[256];
574-
snprintf(name, sizeof(name), "%s", streams[i].name);
575-
cJSON_AddStringToObject(payload, "name", name);
557+
cJSON_AddStringToObject(payload, "name", streams[i].name);
576558

577-
char image_topic[512];
559+
char image_topic[MAX_TOPIC_LENGTH];
578560
snprintf(image_topic, sizeof(image_topic), "%s/cameras/%s/snapshot",
579561
topic_prefix, safe_name);
580562
cJSON_AddStringToObject(payload, "topic", image_topic);
@@ -584,7 +566,7 @@ int mqtt_publish_ha_discovery(void) {
584566

585567
// Availability
586568
cJSON *avail = cJSON_CreateObject();
587-
char avail_topic[512];
569+
char avail_topic[MAX_TOPIC_LENGTH];
588570
snprintf(avail_topic, sizeof(avail_topic), "%s/availability", topic_prefix);
589571
cJSON_AddStringToObject(avail, "topic", avail_topic);
590572
cJSON_AddStringToObject(avail, "payload_available", "online");
@@ -616,7 +598,7 @@ int mqtt_publish_ha_discovery(void) {
616598

617599
// --- 2. Binary sensor for motion detection ---
618600
{
619-
char topic[512];
601+
char topic[MAX_TOPIC_LENGTH];
620602
snprintf(topic, sizeof(topic), "%s/binary_sensor/lightnvr/%s_motion/config",
621603
prefix, safe_name);
622604

@@ -631,7 +613,7 @@ int mqtt_publish_ha_discovery(void) {
631613
snprintf(name, sizeof(name), "%s Motion", streams[i].name);
632614
cJSON_AddStringToObject(payload, "name", name);
633615

634-
char state_topic[512];
616+
char state_topic[MAX_TOPIC_LENGTH];
635617
snprintf(state_topic, sizeof(state_topic), "%s/cameras/%s/motion",
636618
topic_prefix, safe_name);
637619
cJSON_AddStringToObject(payload, "state_topic", state_topic);
@@ -641,7 +623,7 @@ int mqtt_publish_ha_discovery(void) {
641623

642624
// Availability
643625
cJSON *avail = cJSON_CreateObject();
644-
char avail_topic[512];
626+
char avail_topic[MAX_TOPIC_LENGTH];
645627
snprintf(avail_topic, sizeof(avail_topic), "%s/availability", topic_prefix);
646628
cJSON_AddStringToObject(avail, "topic", avail_topic);
647629
cJSON_AddStringToObject(avail, "payload_available", "online");
@@ -673,7 +655,7 @@ int mqtt_publish_ha_discovery(void) {
673655

674656
// --- 3. Sensor for detection count (generic) ---
675657
{
676-
char topic[512];
658+
char topic[MAX_TOPIC_LENGTH];
677659
snprintf(topic, sizeof(topic), "%s/sensor/lightnvr/%s_detection_count/config",
678660
prefix, safe_name);
679661

@@ -688,15 +670,15 @@ int mqtt_publish_ha_discovery(void) {
688670
snprintf(name, sizeof(name), "%s Detections", streams[i].name);
689671
cJSON_AddStringToObject(payload, "name", name);
690672

691-
char state_topic[512];
673+
char state_topic[MAX_TOPIC_LENGTH];
692674
snprintf(state_topic, sizeof(state_topic), "%s/cameras/%s/detection_count",
693675
topic_prefix, safe_name);
694676
cJSON_AddStringToObject(payload, "state_topic", state_topic);
695677
cJSON_AddStringToObject(payload, "icon", "mdi:motion-sensor");
696678

697679
// Availability
698680
cJSON *avail = cJSON_CreateObject();
699-
char avail_topic[512];
681+
char avail_topic[MAX_TOPIC_LENGTH];
700682
snprintf(avail_topic, sizeof(avail_topic), "%s/availability", topic_prefix);
701683
cJSON_AddStringToObject(avail, "topic", avail_topic);
702684
cJSON_AddStringToObject(avail, "payload_available", "online");
@@ -728,7 +710,7 @@ int mqtt_publish_ha_discovery(void) {
728710

729711
// --- Publish initial states so entities don't show "Unknown" ---
730712
{
731-
char topic[512];
713+
char topic[MAX_TOPIC_LENGTH];
732714

733715
// Motion: initially OFF
734716
snprintf(topic, sizeof(topic), "%s/cameras/%s/motion",
@@ -832,7 +814,7 @@ void mqtt_set_motion_state(const char *stream_name, const detection_result_t *re
832814

833815
// Publish motion ON
834816
if (should_publish_on) {
835-
char topic[512];
817+
char topic[MAX_TOPIC_LENGTH];
836818
snprintf(topic, sizeof(topic), "%s/cameras/%s/motion",
837819
mqtt_config->mqtt_topic_prefix, safe_name);
838820
mqtt_publish_raw(topic, "ON", false);
@@ -841,7 +823,7 @@ void mqtt_set_motion_state(const char *stream_name, const detection_result_t *re
841823

842824
// Publish detection count
843825
{
844-
char topic[512];
826+
char topic[MAX_TOPIC_LENGTH];
845827
snprintf(topic, sizeof(topic), "%s/cameras/%s/detection_count",
846828
mqtt_config->mqtt_topic_prefix, safe_name);
847829
char count_str[16];
@@ -851,7 +833,7 @@ void mqtt_set_motion_state(const char *stream_name, const detection_result_t *re
851833

852834
// Publish per-object-class counts
853835
for (int i = 0; i < num_labels; i++) {
854-
char topic[512];
836+
char topic[MAX_TOPIC_LENGTH];
855837
snprintf(topic, sizeof(topic), "%s/cameras/%s/%s",
856838
mqtt_config->mqtt_topic_prefix, safe_name, labels_copy[i]);
857839
char count_str[16];
@@ -889,7 +871,7 @@ static void *ha_snapshot_thread_func(void *arg) {
889871
if (go2rtc_get_snapshot(streams[i].name, &jpeg_data, &jpeg_size)) {
890872
char safe_name[256];
891873
sanitize_stream_name(streams[i].name, safe_name, sizeof(safe_name));
892-
char topic[512];
874+
char topic[MAX_TOPIC_LENGTH];
893875
snprintf(topic, sizeof(topic), "%s/cameras/%s/snapshot",
894876
mqtt_config->mqtt_topic_prefix, safe_name);
895877
mqtt_publish_binary(topic, jpeg_data, jpeg_size, false);
@@ -944,7 +926,7 @@ static void *ha_motion_thread_func(void *arg) {
944926
pthread_mutex_unlock(&motion_mutex);
945927

946928
// Publish motion OFF
947-
char topic[512];
929+
char topic[MAX_TOPIC_LENGTH];
948930
snprintf(topic, sizeof(topic), "%s/cameras/%s/motion",
949931
mqtt_config->mqtt_topic_prefix, safe_name);
950932
mqtt_publish_raw(topic, "OFF", false);

0 commit comments

Comments
 (0)