Skip to content

Commit 0efed2c

Browse files
matteiusclaude
andcommitted
fix(streams): reject names with spaces/special chars (#396)
Stream names with spaces broke MSE ("stream not found") and hung HLS. Names flow through URL-encoded client requests, go2rtc (decoded keys), and filesystem paths (sanitize_stream_name collapses to [A-Za-z0-9_-]), so "Front Door" and "Front_Door" also collide on disk. Validate at POST /api/streams and mirror the rule in the create form; add a 26-case Unity test covering the validator and sanitizer agreement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1e6a7b1 commit 0efed2c

7 files changed

Lines changed: 258 additions & 1 deletion

File tree

include/core/path_utils.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef LIGHTNVR_PATH_UTILS_H
22
#define LIGHTNVR_PATH_UTILS_H
33

4+
#include <stdbool.h>
45
#include <stddef.h>
56
#include <sys/types.h>
67

@@ -14,6 +15,25 @@
1415
*/
1516
void sanitize_stream_name(const char *input, char *output, size_t output_size);
1617

18+
/**
19+
* Check whether a stream name is valid for use as an identifier.
20+
*
21+
* A stream name flows through filesystem paths, URLs, WebSocket query
22+
* parameters, and the go2rtc stream table. Allowing spaces or other
23+
* special characters leads to URL-encoding mismatches (e.g. "mse: stream
24+
* not found" when the client requests "Front%20Door%20Camera" but go2rtc
25+
* registered "Front Door Camera"), and also causes filesystem collisions
26+
* because sanitize_stream_name() collapses all non-[A-Za-z0-9-] characters
27+
* to underscores.
28+
*
29+
* Allowed characters: [A-Za-z0-9._-]. The name must be non-empty and may
30+
* not start with '.' (to avoid hidden-file paths).
31+
*
32+
* @param name NUL-terminated stream name, may be NULL
33+
* @return true if the name is valid, false otherwise
34+
*/
35+
bool is_valid_stream_name(const char *name);
36+
1737
/**
1838
* Ensure the specified directory exists, creating it if necessary. Does not recur.
1939
* Sets errno on failure.

src/core/path_utils.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,23 @@ void sanitize_stream_name(const char *input, char *output, size_t output_size) {
2828
output[i] = '\0';
2929
}
3030

31+
bool is_valid_stream_name(const char *name) {
32+
if (!name || name[0] == '\0') {
33+
return false;
34+
}
35+
// Reject leading '.' so names can't produce hidden-file paths.
36+
if (name[0] == '.') {
37+
return false;
38+
}
39+
for (size_t i = 0; name[i] != '\0'; i++) {
40+
unsigned char c = (unsigned char) name[i];
41+
if (!(isalpha(c) || isdigit(c) || c == '-' || c == '_' || c == '.')) {
42+
return false;
43+
}
44+
}
45+
return true;
46+
}
47+
3148
// Ensure the specified directory exists, creating it if necessary. Does not recur.
3249
int ensure_dir(const char *path) {
3350
struct stat st;

src/web/api_handlers_streams_modify.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#define LOG_COMPONENT "StreamsAPI"
1515
#include "core/logger.h"
1616
#include "core/config.h"
17+
#include "core/path_utils.h"
1718
#include "core/url_utils.h"
1819
#include "utils/strings.h"
1920
#include "video/stream_manager.h"
@@ -469,6 +470,18 @@ void handle_post_stream(const http_request_t *req, http_response_t *res) {
469470
while (name_len > 0 && (config.name[name_len - 1] == ' ' || config.name[name_len - 1] == '\t')) {
470471
config.name[--name_len] = '\0';
471472
}
473+
474+
// Reject names with characters that would mismatch between URL-encoded
475+
// client requests (MSE/HLS/WebRTC) and the decoded key stored by go2rtc
476+
// and the filesystem sanitizer. See issue #396.
477+
if (!is_valid_stream_name(config.name)) {
478+
log_error("Invalid stream name: '%s' (allowed: letters, digits, '.', '-', '_')", config.name);
479+
cJSON_Delete(stream_json);
480+
http_response_set_json_error(res, 400,
481+
"Stream name can only contain letters, digits, '.', '-', and '_'");
482+
return;
483+
}
484+
472485
safe_strcpy(config.url, url->valuestring, sizeof(config.url), 0);
473486

474487
// Optional fields with defaults

tests/unit/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ add_layer2_test(test_config)
125125
add_layer2_test(test_logger)
126126
add_layer2_test(test_strings)
127127
add_layer2_test(test_memory)
128+
add_layer2_test(test_path_utils)
128129
add_layer2_test(test_request_response)
129130
add_layer2_test(test_shutdown_coordinator)
130131
add_layer2_test(test_detection_config)

tests/unit/test_path_utils.c

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
/**
2+
* @file test_path_utils.c
3+
* @brief Layer 2 unit tests — stream name validation and sanitization
4+
*
5+
* Covers is_valid_stream_name() (added for #396) and sanitize_stream_name().
6+
*
7+
* Stream names flow through filesystem paths, URLs, and the go2rtc stream
8+
* table, so they must survive URL-encoding round-trips unchanged. Names
9+
* containing spaces or other special characters produce "mse: stream not
10+
* found" errors because the client sends the URL-encoded form while
11+
* go2rtc registered the decoded form.
12+
*/
13+
14+
#define _POSIX_C_SOURCE 200809L
15+
#define _GNU_SOURCE
16+
17+
#include <string.h>
18+
19+
#include "unity.h"
20+
#include "core/path_utils.h"
21+
22+
void setUp(void) {}
23+
void tearDown(void) {}
24+
25+
/* ================================================================
26+
* is_valid_stream_name — accepts
27+
* ================================================================ */
28+
29+
void test_valid_simple_lowercase(void) {
30+
TEST_ASSERT_TRUE(is_valid_stream_name("front_door"));
31+
}
32+
33+
void test_valid_mixed_case(void) {
34+
TEST_ASSERT_TRUE(is_valid_stream_name("FrontDoor"));
35+
}
36+
37+
void test_valid_digits(void) {
38+
TEST_ASSERT_TRUE(is_valid_stream_name("cam01"));
39+
}
40+
41+
void test_valid_hyphen(void) {
42+
TEST_ASSERT_TRUE(is_valid_stream_name("front-door"));
43+
}
44+
45+
void test_valid_underscore(void) {
46+
TEST_ASSERT_TRUE(is_valid_stream_name("front_door_camera"));
47+
}
48+
49+
void test_valid_dot_in_middle(void) {
50+
TEST_ASSERT_TRUE(is_valid_stream_name("cam.1"));
51+
}
52+
53+
void test_valid_single_char(void) {
54+
TEST_ASSERT_TRUE(is_valid_stream_name("a"));
55+
}
56+
57+
/* ================================================================
58+
* is_valid_stream_name — rejects (the #396 cases)
59+
* ================================================================ */
60+
61+
void test_reject_null(void) {
62+
TEST_ASSERT_FALSE(is_valid_stream_name(NULL));
63+
}
64+
65+
void test_reject_empty(void) {
66+
TEST_ASSERT_FALSE(is_valid_stream_name(""));
67+
}
68+
69+
void test_reject_space_in_middle(void) {
70+
/* The primary #396 case: "Front Door Camera" breaks MSE/HLS. */
71+
TEST_ASSERT_FALSE(is_valid_stream_name("Front Door Camera"));
72+
}
73+
74+
void test_reject_leading_space(void) {
75+
TEST_ASSERT_FALSE(is_valid_stream_name(" front_door"));
76+
}
77+
78+
void test_reject_trailing_space(void) {
79+
TEST_ASSERT_FALSE(is_valid_stream_name("front_door "));
80+
}
81+
82+
void test_reject_tab(void) {
83+
TEST_ASSERT_FALSE(is_valid_stream_name("front\tdoor"));
84+
}
85+
86+
void test_reject_slash(void) {
87+
/* Would break path routing (/api/streams/{name}/...). */
88+
TEST_ASSERT_FALSE(is_valid_stream_name("front/door"));
89+
}
90+
91+
void test_reject_backslash(void) {
92+
TEST_ASSERT_FALSE(is_valid_stream_name("front\\door"));
93+
}
94+
95+
void test_reject_question_mark(void) {
96+
/* Would become a query-string boundary. */
97+
TEST_ASSERT_FALSE(is_valid_stream_name("front?door"));
98+
}
99+
100+
void test_reject_ampersand(void) {
101+
TEST_ASSERT_FALSE(is_valid_stream_name("front&door"));
102+
}
103+
104+
void test_reject_hash(void) {
105+
/* URL fragment boundary. */
106+
TEST_ASSERT_FALSE(is_valid_stream_name("front#door"));
107+
}
108+
109+
void test_reject_percent(void) {
110+
/* Looks like URL-encoding. */
111+
TEST_ASSERT_FALSE(is_valid_stream_name("front%20door"));
112+
}
113+
114+
void test_reject_leading_dot(void) {
115+
/* Would produce a hidden-file path. */
116+
TEST_ASSERT_FALSE(is_valid_stream_name(".hidden"));
117+
}
118+
119+
void test_reject_dot_dot(void) {
120+
/* Path traversal. */
121+
TEST_ASSERT_FALSE(is_valid_stream_name(".."));
122+
}
123+
124+
void test_reject_non_ascii(void) {
125+
/* Non-ASCII round-trips poorly through URL encoding. */
126+
TEST_ASSERT_FALSE(is_valid_stream_name("caméra"));
127+
}
128+
129+
void test_reject_null_byte_is_not_applicable(void) {
130+
/* C strings terminate at NUL; nothing to test beyond empty case. */
131+
TEST_PASS();
132+
}
133+
134+
/* ================================================================
135+
* sanitize_stream_name regression coverage
136+
*
137+
* The validator and sanitizer must agree on the "safe" character set:
138+
* sanitize_stream_name() maps spaces to underscores, which means
139+
* "Front Door" and "Front_Door" both collapse to "Front_Door" on disk.
140+
* That collision is exactly why is_valid_stream_name() rejects spaces.
141+
* ================================================================ */
142+
143+
void test_sanitize_maps_space_to_underscore(void) {
144+
char out[64];
145+
sanitize_stream_name("Front Door", out, sizeof(out));
146+
TEST_ASSERT_EQUAL_STRING("Front_Door", out);
147+
}
148+
149+
void test_sanitize_preserves_valid_names(void) {
150+
/* A name that passes is_valid_stream_name must be unchanged by sanitize. */
151+
char out[64];
152+
sanitize_stream_name("front_door-1", out, sizeof(out));
153+
TEST_ASSERT_EQUAL_STRING("front_door-1", out);
154+
}
155+
156+
void test_sanitize_two_names_that_would_collide(void) {
157+
/* Demonstrates the collision that motivates rejecting spaces at creation. */
158+
char out_a[64], out_b[64];
159+
sanitize_stream_name("Front Door", out_a, sizeof(out_a));
160+
sanitize_stream_name("Front_Door", out_b, sizeof(out_b));
161+
TEST_ASSERT_EQUAL_STRING(out_a, out_b);
162+
}
163+
164+
/* ================================================================
165+
* main
166+
* ================================================================ */
167+
168+
int main(void) {
169+
UNITY_BEGIN();
170+
171+
RUN_TEST(test_valid_simple_lowercase);
172+
RUN_TEST(test_valid_mixed_case);
173+
RUN_TEST(test_valid_digits);
174+
RUN_TEST(test_valid_hyphen);
175+
RUN_TEST(test_valid_underscore);
176+
RUN_TEST(test_valid_dot_in_middle);
177+
RUN_TEST(test_valid_single_char);
178+
179+
RUN_TEST(test_reject_null);
180+
RUN_TEST(test_reject_empty);
181+
RUN_TEST(test_reject_space_in_middle);
182+
RUN_TEST(test_reject_leading_space);
183+
RUN_TEST(test_reject_trailing_space);
184+
RUN_TEST(test_reject_tab);
185+
RUN_TEST(test_reject_slash);
186+
RUN_TEST(test_reject_backslash);
187+
RUN_TEST(test_reject_question_mark);
188+
RUN_TEST(test_reject_ampersand);
189+
RUN_TEST(test_reject_hash);
190+
RUN_TEST(test_reject_percent);
191+
RUN_TEST(test_reject_leading_dot);
192+
RUN_TEST(test_reject_dot_dot);
193+
RUN_TEST(test_reject_non_ascii);
194+
RUN_TEST(test_reject_null_byte_is_not_applicable);
195+
196+
RUN_TEST(test_sanitize_maps_space_to_underscore);
197+
RUN_TEST(test_sanitize_preserves_valid_names);
198+
RUN_TEST(test_sanitize_two_names_that_would_collide);
199+
200+
return UNITY_END();
201+
}

web/js/components/preact/StreamConfigModal.jsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,10 +433,14 @@ export function StreamConfigModal({
433433
onChange={onInputChange}
434434
disabled={isEditing}
435435
required
436+
pattern="[A-Za-z0-9_\-][A-Za-z0-9._\-]*"
437+
title={t('streamsConfig.streamNameAllowedChars')}
436438
placeholder={t('streamsConfig.streamNamePlaceholder')}
437439
/>
438-
{isEditing && (
440+
{isEditing ? (
439441
<p className="mt-1 text-xs text-muted-foreground">{t('streamsConfig.streamNameImmutable')}</p>
442+
) : (
443+
<p className="mt-1 text-xs text-muted-foreground">{t('streamsConfig.streamNameAllowedChars')}</p>
440444
)}
441445
</div>
442446

web/public/locales/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@
753753
"streamsConfig.streamName": "Stream Name",
754754
"streamsConfig.streamNamePlaceholder": "e.g., front_door, backyard",
755755
"streamsConfig.streamNameImmutable": "Stream name cannot be changed after creation",
756+
"streamsConfig.streamNameAllowedChars": "Only letters, digits, '.', '-', and '_' are allowed (no spaces).",
756757
"streamsConfig.streamUrl": "Stream URL",
757758
"streamsConfig.streamUrlPlaceholder": "rtsp://username:password@192.168.1.100:554/stream",
758759
"streamsConfig.subStreamUrl": "Sub-stream URL (optional)",

0 commit comments

Comments
 (0)