The database driver was migrated from the legacy DBD::mysql to DBD::MariaDB throughout the entire codebase.
All DSN strings were updated from DBI:mysql: to DBI:MariaDB:, with host forced to 127.0.0.1 (MariaDB refuses port-based connections when localhost is used, falling back to Unix sockets). The deprecated mysql_insertid attribute was replaced with the standard $dbh->last_insert_id() in 11 locations across 6 files. All mysql_* connection attributes were removed or replaced with their MariaDB equivalents, and charset was moved into the DSN string directly.
last_insert_id() correctnessBeyond the driver migration, a subtle bug was fixed in userAdd() (Helpers.pm): last_insert_id() was being called twice — once for the USER_HOSTMASK insert and again for the return value. Since the second call happened after the hostmask insert, it was returning the wrong ID. The ID is now captured once, immediately after the INSERT INTO USER, and reused for both the hostmask registration and the return value.
Several queries were still building WHERE clauses via direct string interpolation (WHERE $where). These were replaced with explicit literal SQL and parameterized ? placeholders throughout Auth.pm and ChannelCommands.pm.
open FH, "cmd $var |"All remaining two-argument open calls with shell commands were converted to the three-argument list form (open $fh, "-|", @args), eliminating shell metacharacter injection in External.pm, Helpers.pm, and Radio.pm.
youtube-dl → yt-dlpReferences to the deprecated youtube-dl binary were unified under yt-dlp, with the path made configurable via radio.YTDLP_PATH in the configuration file.
The Partyline TCP admin interface received rate limiting (10 commands per 5 seconds), IRC nick validation, and a whitelist of allowed IRC verbs to prevent injection via the .raw command.
27 implicit JOINs of the form FROM TABLE_A, TABLE_B WHERE TABLE_A.id = TABLE_B.id were converted to explicit JOIN ... ON ... or LEFT JOIN ... ON ... syntax across 9 files. This affects query plan generation and readability.
SELECT * qualifiedOver 20 SELECT * queries were replaced with explicit column lists. Existence checks were converted to SELECT 1. Where all columns are genuinely needed (e.g., User->create() building a full object), the queries were left as-is with a comment. One remaining SELECT * in User.pm was converted to an explicit column list in this session.
LIKERadio.pm contained a LIKE '%' . $sSearch . '%' query built by string concatenation, with manual escaping of single quotes and semicolons. The manual escaping was removed and the pattern passed as a ? parameter, which is both safer and cleaner.
sth->finish leaksMultiple database statement handles were not being closed after use. Fixed in Channel.pm (8+ accessors and setters), UserCommands.pm (mbModUser_ctx, userInfo_ctx), Hailo.pm, Helpers.pm, ChannelCommands.pm, and others. In get_user_level(), finish was being called before fetchrow_hashref, causing the method to always return the default value of 0.
Channel.pm setters — silent failureAll six setters (set_topic, set_tmdb_lang, set_key, set_description, set_chanmode, set_auto_join) called execute() without checking the return value, then updated the in-memory object attribute unconditionally. If the UPDATE failed (e.g., DB connection lost), the object state became inconsistent with the database. Each setter now checks execute() and only updates the in-memory attribute on success, logging the error otherwise.
decode_json without evalThree decode_json calls in Helpers.pm and Radio.pm were not protected by eval {}. A malformed JSON response from an external API would crash the bot’s event loop. All are now wrapped in eval { decode_json ... } with appropriate error handling.
14 bare filehandles (LIQUIDSOAP_HARBOR, LIQUIDSOAP, LIQUIDSOAP2, LIQUIDSOAP_TELNET_SERVER, YOUTUBE_INFOS) in Radio.pm and PIDFILE in Mediabot.pm were converted to lexical my $fh_xxx variables declared before the unless (open ...) block (declaring inside the unless would limit scope to that block only, causing “requires explicit package name” errors).
Log.pm — nested sub declarationssub info, sub debug, and sub error were declared syntactically inside the body of sub log (at {} depth 2). Perl treats named subs as global regardless of nesting, but this is a source of Variable will not stay shared warnings and significant confusion during code review. The three helpers were moved to package level.
userInfo_ctx — wrong signatureuserInfo_ctx was defined as my ($self, $ctx) = @_ but dispatched as userInfo_ctx($ctx) (one argument). This meant $self = $ctx and $ctx = undef, causing an immediate return unless $ctx with no response sent to the user. Corrected to the standard ($ctx) signature with my $self = $ctx->bot.
userLogin_ctx performed UPDATE USER SET auth=1 but never inserted the caller’s hostmask into USER_HOSTMASK. After login, get_user_from_message() could not find the user by hostmask, resulting in “User not found with this hostmask” on every subsequent command. The hostmask is now registered on login (with a duplicate check).
userLogout_ctx — missing @EXPORTThe userLogout_ctx sub was defined in LoginCommands.pm but absent from @EXPORT. When Mediabot.pm called it via use Mediabot::LoginCommands, the symbol was not imported, causing a fatal “Undefined subroutine” error that crashed the Net::Async::IRC event loop with “EOF from client”.
get_user_from_message() — hostmask cacheThis function performs a GROUP_CONCAT across the entire USER + USER_HOSTMASK tables on every incoming PRIVMSG. On servers with many registered users this becomes a noticeable bottleneck. A TTL-based cache (5 seconds, keyed by full IRC hostmask) was added. The cache is explicitly invalidated on login and logout via clear_user_cache(), ensuring authentication state changes are reflected immediately.
(@) removed95 occurrences of sub foo(@) across 12 files were removed. In modern Perl with use strict, these prototypes serve no purpose: they do not enforce types, do not affect argument passing, and can generate spurious warnings. Their presence was misleading for anyone reading the code.
Conf.pm get() — silent key missConf->get() returned undef silently for any unknown configuration key, making it hard to diagnose typos or missing entries. A warn is now emitted (activatable via MEDIABOT_DEBUG_CONF=1) when a requested key does not exist in the loaded configuration.
Over 30 log messages were reclassified from level 0 (always visible) to level 1 (debug) or level 2 (verbose debug) across Helpers.pm, DBCommands.pm, ChannelCommands.pm, Mediabot.pm, UserCommands.pm, and mediabot.pl. This reduces noise in production logs while keeping the information available at higher debug levels. Messages retained at level 0 are those that should always be visible: live IRC traffic traces, fatal errors, and startup/shutdown events.
A comprehensive automated live test suite was built and brought to a full pass. The suite spins up a fresh MariaDB test database, generates a configuration file, connects a spy IRC client, launches the bot as a subprocess, and runs 12 test files covering authentication, dispatch routing, channel commands, user commands, quotes, external commands, ignore/responder management, and dynamic DB commands.
Key issues resolved along the way:
_printQuoteSyntax sends 7 NOTICE lines in rapid succession. The spy client was reading only the first, then immediately sending the next command. The remaining 6 built up in the TCP buffer and triggered the server’s flood protection, disconnecting the bot mid-suite. Fixed by draining the buffer after commands that generate multi-line responses.03_auth.t performs a login but has no logout. When 05_dispatch_private.t runs its “without auth” checks, _ensure_logged_in_state reads auth=1 from the database and grants access. Fixed by sending logout at the start of 05_dispatch_private.t.!q del response not matched: The deletion response "(user) done. (id: N)" did not match the test pattern expecting del|delet|remov. Changed to "(user) deleted. (id: N)".userinfo mbtest causing Excess Flood: userInfo_ctx was sending 11 NOTICE lines. Compacted to 2 lines.!q view 1 test assumed the first quote in the database always has id=1. After other test runs, IDs increment. The test now captures the actual ID from the !q add response..gitignore UpdatedThe .gitignore was extended to cover bot.log, bot.log.* (logrotate rotations), t/live/test.conf (auto-generated per run), and .DS_Store.
| Area | Changes |
|---|---|
| Driver migration | DBD::mysql → DBD::MariaDB, 11× last_insert_id fix |
| Security | 5 phases: SQL injection, shell injection, rate limiting |
| SQL hygiene | 27 implicit JOINs, 20+ SELECT *, parameterized LIKE |
| Bug fixes | ~25 distinct bugs across 10 modules |
| Code quality | Cache, log levels, prototypes, Conf warn |
| Test suite | 67/67 passing, ~220s runtime |
You must be logged in to reply.