Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions src/sender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,31 @@ void update_conns(char *source_ip_file) {
garbage, or unreadable file). Without this guard every existing link would
be marked removed and torn down, killing the stream — and setup_conns()
would exit() on an unreadable file. Keep streaming on the current links. */
if (!srtla::sender::reload_should_apply(
srtla::sender::count_parseable_source_ips(source_ip_file))) {
spdlog::error("Ignoring source IP reload from {}: no valid source IPs "
"(parse error); keeping existing connections",
source_ip_file);
int error_line_num = 0;
auto error = srtla::sender::analyze_reload_error(source_ip_file, &error_line_num);

if (error != srtla::sender::ReloadError::None) {
switch (error) {
case srtla::sender::ReloadError::FileNotFound:
spdlog::error("ips file not found/unreadable: {}, refusing reload",
source_ip_file);
break;
case srtla::sender::ReloadError::FileEmpty:
spdlog::error("ips file is empty: {}, refusing reload",
source_ip_file);
break;
case srtla::sender::ReloadError::ZeroValidIPs:
spdlog::error("Ignoring source IP reload from {}: no valid source IPs "
"(parse error); keeping existing connections",
source_ip_file);
break;
case srtla::sender::ReloadError::InvalidLine:
spdlog::error("invalid IP on line {}: skipping invalid lines in {}",
error_line_num, source_ip_file);
break;
default:
break;
}
return;
}

Expand Down
65 changes: 65 additions & 0 deletions src/sender_logic.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,15 @@ inline bool keepalive_due(time_t last_sent, time_t now) {
return (last_sent + SENDER_IDLE_TIME) < now;
}

// Detailed reload error information for logging.
enum class ReloadError {
None, // No error, reload should apply
FileNotFound, // File cannot be opened
FileEmpty, // File exists but contains no valid IPs
InvalidLine, // File contains invalid IP lines (but may have valid ones)
ZeroValidIPs, // File exists but all lines are invalid
};

// Count how many parseable IPv4 source addresses a source-ip file contains,
// without mutating any global connection state. Returns 0 for a file that
// cannot be opened, is empty, or contains only unparseable lines.
Expand Down Expand Up @@ -139,6 +148,62 @@ inline int count_parseable_source_ips(const char *path) {
return count;
}

// Detailed reload error analysis: returns specific error type and optionally
// populates error_line_num with the line number of the first invalid line
// (1-indexed). Used for precise error logging in update_conns().
inline ReloadError analyze_reload_error(const char *path, int *error_line_num = nullptr) {
FILE *f = fopen(path, "r");
if (f == nullptr) {
return ReloadError::FileNotFound;
}

int count = 0;
int line_num = 0;
char *line = nullptr;
size_t line_len = 0;
bool has_invalid_line = false;
int first_invalid_line = 0;

while (getline(&line, &line_len, f) >= 0) {
line_num++;
char *nl = strchr(line, '\n');
if (nl != nullptr) {
*nl = '\0';
}
// Skip empty lines
if (line[0] == '\0') {
continue;
}
struct sockaddr_in src;
if (parse_ip(&src, line) == 0) {
count++;
} else {
has_invalid_line = true;
if (first_invalid_line == 0) {
first_invalid_line = line_num;
}
}
}

free(line);
fclose(f);

if (error_line_num != nullptr && first_invalid_line > 0) {
*error_line_num = first_invalid_line;
}

if (count == 0 && line_num == 0) {
return ReloadError::FileEmpty;
}
if (count == 0 && has_invalid_line) {
return ReloadError::ZeroValidIPs;
}
if (has_invalid_line) {
return ReloadError::InvalidLine;
}
return ReloadError::None;
}

// Predicate form of the reload guard, for callers that already have a count.
inline bool reload_should_apply(int parseable_ip_count) {
return parseable_ip_count > 0;
Expand Down
52 changes: 52 additions & 0 deletions tests/test_sender_bootstrap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,56 @@ TEST(SenderReloadGuard, UnreadableFileYieldsZeroAndIsRefused) {
reload_should_apply(count_parseable_source_ips("/tmp/srtla_does_not_exist_xyz")));
}

TEST(SenderReloadError, FileNotFoundReturnsFileNotFound) {
auto error = analyze_reload_error("/tmp/srtla_does_not_exist_xyz");
EXPECT_EQ(error, ReloadError::FileNotFound);
}

TEST(SenderReloadError, EmptyFileReturnsFileEmpty) {
TempIpsFile f("");
auto error = analyze_reload_error(f.path());
EXPECT_EQ(error, ReloadError::FileEmpty);
}

TEST(SenderReloadError, AllGarbageReturnsZeroValidIPs) {
TempIpsFile f("not-an-ip\nlol\n???\n");
auto error = analyze_reload_error(f.path());
EXPECT_EQ(error, ReloadError::ZeroValidIPs);
}

TEST(SenderReloadError, MixedValidAndInvalidReturnsInvalidLine) {
TempIpsFile f("10.0.0.10\ngarbage\n10.0.1.10\n");
auto error = analyze_reload_error(f.path());
EXPECT_EQ(error, ReloadError::InvalidLine);
}

TEST(SenderReloadError, AllValidReturnsNone) {
TempIpsFile f("10.0.0.10\n10.0.1.10\n192.168.1.50\n");
auto error = analyze_reload_error(f.path());
EXPECT_EQ(error, ReloadError::None);
}

TEST(SenderReloadError, InvalidLineNumberIsReported) {
TempIpsFile f("10.0.0.10\ngarbage\n10.0.1.10\n");
int line_num = 0;
auto error = analyze_reload_error(f.path(), &line_num);
EXPECT_EQ(error, ReloadError::InvalidLine);
EXPECT_EQ(line_num, 2);
}

TEST(SenderReloadError, FirstInvalidLineNumberReportedWhenMultiple) {
TempIpsFile f("10.0.0.10\ngarbage1\ngarbage2\n10.0.1.10\n");
int line_num = 0;
auto error = analyze_reload_error(f.path(), &line_num);
EXPECT_EQ(error, ReloadError::InvalidLine);
EXPECT_EQ(line_num, 2);
}

TEST(SenderReloadError, EmptyLinesAreSkipped) {
TempIpsFile f("10.0.0.10\n\n10.0.1.10\n");
auto error = analyze_reload_error(f.path());
EXPECT_EQ(error, ReloadError::None);
EXPECT_EQ(count_parseable_source_ips(f.path()), 2);
}

} // namespace
Loading