A saved time based action listed its schedule as "0 21 * * *" rather than
"Every day at 21:00". The cron parser was not at fault; cronToParts('0 21 * *
*') returns the right parts. The fault was one operator:
$drawable = ($action->Schedule() === '') or ($schedule_parts !== null);
"or" binds looser than "=", so this parses as ($drawable = ($action->Schedule()
=== '')) or (...). The right-hand clause is evaluated and thrown away, and
$drawable gets only the left one -- false for any schedule that is actually
set, which is every saved action. So the editor took every saved schedule for
one it could not draw and fell back to the read-only cron field, which is the
branch that exists for genuinely undrawable specs like "*/5 * * * *".
Changed to ||, which binds tighter than "=" and does what the line looks like
it does.
Tests: tests/php/test_assignment_precedence.php, 21 checks. It tokenizes every
php file under web/ and reports assignments whose right-hand side has an
unbracketed "and", "or" or "xor" -- tokenized rather than grepped so that the
many safely bracketed ones, in conditions and ternaries, are not reported.
There is a self-check for each distinction it has to make, since a scan that
quietly matched nothing would pass the tree-wide check on its own. Verified it
fails when the reported line is put back, and when a new instance is added
elsewhere.
The scan found six more in the rest of the tree, which the test carries as a
list of open questions rather than as exemptions. Two are the deliberate
"$fp = fopen(...) or error_log(...)" idiom and are correct. The other four are
this same bug, and each changes behaviour to fix, so none is swept up here --
in particular filter.php:401 assigns only $filter->Id() to $canDelete, so
delete is offered for any existing filter whether or not the user may edit it,
and that is access control rather than something to guess at. Anything not on
the list fails the test, so a new one cannot be added, and the list is checked
in both directions so it shrinks as the entries are dealt with.
Ran: tests/php/test_assignment_precedence.php (21 passed), the rest of
tests/php and tests/js, npx eslint . (clean), utils/check-homoglyphs.py
(clean). test_bandwidth_clamp.php fails on this box for an unrelated reason:
it cannot read /etc/zm/conf.d/03custom.conf and the permission warning lands
in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Looking for other instances of the CronTab.php fatal in the previous commit
found eleven, in ten files, dating from 2019 to 2023. Each is a call to a
logger function from the global namespace without the \ZM\ prefix, so each is
a fatal the moment its branch runs:
web/ajax/console.php, events.php, reports.php, add_monitors.php, status.php
web/includes/actions/snapshot.php (two), donate.php
web/includes/functions.php, session.php
web/views/image.php
They have survived this long because of where they sit. Every one is on an
error or diagnostic path -- an invalid sort order, a zero byte download, a
failed read lock -- so the call is only reached when something has already
gone wrong, and then it turns a handled problem into a blank 500. That is the
shape of the bug on the monitor save: the write had worked, and the thing that
broke was the attempt to log.
Verified before changing any of them that each file is in the global
namespace, does not define the function itself and does not import it, so the
call really does not resolve.
Tests: tests/php/test_logger_calls_qualified.php, 16 checks. It tokenizes
every php file under web/ and reports unqualified calls, with the function
names read out of logger.php rather than listed, so a new one is covered the
day it is added. web/api is skipped as cakephp with its own logging, and
assets as third party.
Php's own tokenizer rather than a regex, because the distinctions matter: a
method call on an object that happens to have an Error() method is fine, as
is a definition, a mention, a qualified call, the same call from inside
namespace ZM, and the name appearing in a comment or a string. There is a
check for each of those, since a scan that quietly matched nothing would pass
the tree-wide check on its own.
Verified the scan fails when the Debug() in CronTab.php is put back.
Ran: tests/php/test_logger_calls_qualified.php (16 passed), the rest of
tests/php and tests/js, npx eslint . (clean), utils/check-homoglyphs.py
(clean). test_bandwidth_clamp.php fails on this box for an unrelated reason:
it cannot read /etc/zm/conf.d/03custom.conf and the permission warning lands
in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Saving a time based action on a monitor died with
PHP Fatal error: Uncaught Error: Call to undefined function Debug() in
includes/CronTab.php:280
and the browser showed only "An error occurred while saving data", because
the save goes over ajax and a fatal gives it a 500 with nothing to report.
The action itself had already been written to the database; what failed was
the crontab rebuild that follows it.
Debug(), Info(), Warning() and Error() are defined in includes/logger.php,
which is namespace ZM. CronTab.php is in the global namespace, so they have to
be called as \ZM\Debug(...) -- php resolves a function name against the
current namespace and then the global one, and these are in neither from
there. Four calls were bare: the Debug at the end of cronRebuild and three
Error/Warning calls on failure paths.
Nothing catches this at include time, php -l is happy with it, and the file
works until the one branch holding the call is taken. The Error calls here are
on paths that only run when crontab itself fails, so they could have sat
unnoticed for much longer than the Debug did.
A follow-up commit adds a test that scans for this, and fixes the ten other
places in the tree that have the same bug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Pin When to 11em and raise Schedule from 24em to 28em.
Both columns are sized by a select, and a select is as wide as its longest
option whether or not the column is. When's is "Manually triggered", which
needs about 11em; it had been sharing the leftover space three ways with
Device and Does and taking more than that. Schedule has to fit "Every
<frequency> at <time>" on one line, which comes to about 25em -- so the 24em
it was given was short, and the frequency select was the thing being
truncated.
The space comes out of Device and Does, which now divide the remainder
between them. Device is left loose on purpose: monitor names are the only
contents in this table of unpredictable length, so it is the right column to
absorb whatever is spare.
Worth knowing for later: Schedule is wide almost entirely because of one
frequency option, "weekend day (Sat & Sun)", which sets that select's width
by itself. Rewording just that label would give several em back to the rest
of the table. Not done here, since it is a visible label and not mine to
change.
Tests: tests/php/test_monitor_actions_columns.php, now 30 checks. Added a
check per select column that it fits its longest option, with the option keys
read out of the view and their text out of the language file so neither is
restated. That is the measurement these widths were chosen from, so it is the
one worth keeping: a label reworded longer would start truncating with
nothing to say it had.
The estimate for Schedule initially cleared 24em, which would have made it
too generous to catch what was reported. The two terms it was missing are
real and identifiable -- .table-sm's padding, and the collapsed space between
four controls written on separate source lines -- and together they come to
about an em and a half, which is roughly the difference complained of. There
is a check that the estimate still fails 24em, labelled as the calibration it
is rather than as a property of the css.
Verified the checks fail with Schedule back at 24em (1), with When too narrow
for its longest option (1), and with Device pinned so nothing is left loose
(3).
Ran: tests/php/test_monitor_actions_columns.php (30 passed), the rest of
tests/php and tests/js, npx eslint . (clean), utils/check-homoglyphs.py
(clean). test_bandwidth_clamp.php fails on this box for an unrelated reason:
it cannot read /etc/zm/conf.d/03custom.conf and the permission warning lands
in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
4em was set from what the cells hold -- one checkbox -- and that is the wrong
thing to measure. A fixed layout will not widen a column to fit its header,
it lets the text run on into the next one, and skin.css renders every th
uppercase at 0.8em. So what has to fit is "ENABLED" plus .table-sm's padding,
which at 4em fit so exactly that it touched the When header. 5.5em leaves a
clear gap.
Tests: tests/php/test_monitor_actions_columns.php, now 21 checks. Dropped the
check that Enabled was the narrowest column "being one checkbox", since that
is the reasoning that produced the bug: "ENABLED" is a wider word than
"SOUND" whatever sits in the cells below them, so there is no ordering to
assert between the two. In its place each narrow column is checked against an
estimate of its own header's width, calibrated so that the 4em this started
with fails and 5.5em passes. The header text is read out of the language file
rather than written into the test, so a translation long enough to break the
layout fails too -- under a fixed layout it really would break it.
Ran: tests/php/test_monitor_actions_columns.php (21 passed), the rest of
tests/php and tests/js, npx eslint . (clean), utils/check-homoglyphs.py
(clean). test_bandwidth_clamp.php fails on this box for an unrelated reason:
it cannot read /etc/zm/conf.d/03custom.conf and the permission warning lands
in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
ul.form indents every row by 300px to make a gutter for its label, and pulls
the label back into that gutter with an equal negative margin. It is a neat
trick for a labelled field and wrong for anything else: the actions tab's
rows have no label, so the table started 300px in with the space beside it
left empty.
Add a noLabel class that cancels the indent, and put it on the four rows of
the tab that have no label -- the table, the help text above and below it,
and the no-capable-devices warning that replaces the lot. The mobile rules
already drop the indent for every row, so there is no counterpart to add
there.
This is also why the Schedule column felt cramped. The fixed widths in
6eb390007a divide whatever the table is given, and the table was being given
300px less than the page; those widths are unchanged and now divide the whole
of it. Only one table on this view sits in a ul.form, so nothing else was
affected.
Tests: tests/php/test_monitor_actions_columns.php grows to 19 checks. Which
class cancels the indent is read out of the css rather than named in the test,
since a drift between the two would bring the indent back without erroring.
It also checks that no row carrying the class has a label, because a label in
a row with no gutter to be pulled into would hang off the left edge -- the
same mistake in the other direction. Verified the checks fail when the
table's row loses the class (2), when the css renames it and the view does
not (2), and when the class is put on a labelled row (1).
Ran: tests/php/test_monitor_actions_columns.php (19 passed), the rest of
tests/php and tests/js, npx eslint . (clean), utils/check-homoglyphs.py
(clean). test_bandwidth_clamp.php fails on this box for an unrelated reason:
it cannot read /etc/zm/conf.d/03custom.conf and the permission warning lands
in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
submitTab() resubmits contentForm without saving, so the view re-renders for a
value that changes which fields it should show. It did
form.attr('tab', tab), which sets an attribute named "tab" on the <form>
element and has no effect whatever: an attribute on a form is not a submitted
value. The view reads $_REQUEST['tab'], which comes from the hidden field.
Writing it correctly would have been worse than leaving it broken. The value
it was being handed, data-tab-name, is the tab the page loaded on, while the
hidden field now holds the tab that is on screen. Assigning the attribute's
value would have undone that tracking and sent anyone who had switched tabs
back to the one they arrived on -- the bug this looks like it would fix. So
the tab is not written here at all, and the field is left to do its job.
Drop data-tab-name from the Method select, since nothing reads it now. An
attribute nothing reads is a trap: the next person here would reasonably
assume it was load bearing.
The submit stays on $j(...).submit() rather than the element's own submit(),
which is worth saying out loud in a comment because the difference is
invisible: jQuery triggers contentForm's submit handler, which validates the
form and blanks the password fields, and a native submit skips both.
Noted, not changed: skin.js also binds submitTab to ".pillList a" on click,
and no markup anywhere in the tree carries that class, so it matches nothing.
It was the only caller that would have wanted data-tab-name's meaning. Left
alone as dead but harmless, and in a file every view loads.
Tests: tests/js/monitor-tab-submit.test.js, 9 checks. skin.js and monitor.js
are loaded into one sandbox, as a page has them, because the behaviour needs
both: monitor.js tracks the tab into the hidden field and skin.js submits it.
The cases are built so that the loaded tab and the showing tab differ, since
any test where they agree would pass whether or not the bug is fixed.
Verified the checks fail when the old attr call is restored (2), when the
naive fix of writing data-tab-name into the field is applied (4), and when
the submit is changed to a native one (7).
Ran: tests/js/monitor-tab-submit.test.js (9 passed), monitor-tab-url.test.js
(14 passed), the rest of tests/js and tests/php, npx eslint . (clean),
utils/check-homoglyphs.py (clean). test_bandwidth_clamp.php fails on this box
for an unrelated reason: it cannot read /etc/zm/conf.d/03custom.conf and the
permission warning lands in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
The view already honoured ?tab=<name> on the way in: php marks that pill
active and renders its pane shown. What was missing was the other half. The
pills are switched client side, so the url kept saying whatever it said when
the page loaded, and a reload -- or a shared link, or the browser restoring
the page -- dropped you back on General.
Write the tab into the url on shown.bs.tab. replaceState rather than
pushState, as filter.js and event.js do: switching tabs in a settings form is
not navigation, and making Back walk back through it would be unhelpful on a
form holding unsaved edits. Only the tab parameter is touched, since the url
carries mid and may carry more.
Update the hidden tab field at the same time. Changing Type or Method
resubmits the form without action=save to redraw the fields that type needs,
and that field is what tells the view where to come back to; php writes it
once with the tab the page loaded on, so that submit also used to land you
back on whichever tab you arrived on rather than the one you were working in.
Noted while reading, not changed here: submitTab() in skin.js sets a `tab`
attribute on the form element rather than the hidden input's value, which
does nothing. It is invisible today because its data-tab-name is the tab the
page loaded on, the same value the input already held, and with this change
the input is now correct anyway. It is shared by every view, so it is left
alone.
Tests: tests/js/monitor-tab-url.test.js, 14 checks, run against monitor.js in
a vm sandbox as the other view script tests are. Most of them are about what
the rewrite leaves alone: rebuilding a url to change one parameter is an easy
way to drop the others, and mid is the one the whole page depends on. The
handler finds the pills by a selector and reads the tab name out of an href,
both of which are shapes in monitor.php, and if either changes the handler
stops firing without erroring -- the tabs would keep working and only the url
would be wrong -- so the markup is checked against the selector the handler
registered rather than against a copy of it. Verified the checks fail when
the pill list is renamed (1), when the no-op guard is removed (1) and when
the query is rebuilt from scratch instead of amended (3).
Ran: tests/js/monitor-tab-url.test.js (14 passed), the rest of tests/js and
tests/php, npx eslint . (clean), utils/check-homoglyphs.py (clean).
test_bandwidth_clamp.php fails on this box for an unrelated reason: it cannot
read /etc/zm/conf.d/03custom.conf and the permission warning lands in its
captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
The Enabled column already asked for 5em, but under the default automatic
table layout a width on a cell is only a suggestion: the browser sizes each
column from its contents and may hand one more than it asked for. So Enabled
kept claiming space on the strength of its header text while holding a single
checkbox, and Schedule -- which has to fit "Every weekend day (Sat & Sun) at
08:00" across a label, a select and a time input -- was the column that got
squeezed.
Make the layout fixed, which binds the declared widths exactly. Enabled and
Sound are pinned to what a checkbox and a number need, Schedule is pinned to
what its sentence needs, and When, Device and Does divide what is left: those
three are selects, and a select that runs short truncates its option text
rather than breaking the row. zone.css already lays a table out this way, so
it is not a new approach in the skin.
A fixed layout will not shrink a control to fit its column, so the selects
and inputs are capped at the column width. Without that a long monitor name
in Device would spill over Does.
Tests: tests/php/test_monitor_actions_columns.php, 13 checks. The widths are
addressed positionally, as :nth-child(1), (3) and (6), because the cells carry
no classes -- which is fine until a column is inserted, when every pin after
it silently moves one column right and nothing errors. So the css is read for
which positions it pins, the view for what is in each position, and the two
are compared. Verified the checks fail when table-layout is removed (1
failure) and when a column is inserted before Schedule (6).
Not verified in a browser: the install under /usr/share/zoneminder/www is a
copy rather than a symlink of this tree, so the em values are a judgement
about what each column holds rather than something measured on screen.
Ran: tests/php/test_monitor_actions_columns.php (13 passed), the rest of
tests/php and tests/js, npx eslint . (clean), utils/check-homoglyphs.py
(clean). test_bandwidth_clamp.php fails on this box for an unrelated reason:
it cannot read /etc/zm/conf.d/03custom.conf and the permission warning lands
in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
The scheduled tasks view carried an inline form above the list, so the page
was two things at once: the form was always on screen whether or not anyone
was editing, and the list it belonged to was pushed below the fold.
Make the view the list, and move adding and editing into a modal built by
web/ajax/modals/cronjob.php, following the pattern the Options modals use.
The Add button and the task name both open it; the modal posts back to
?view=cron, so includes/actions/cron.php still does the validating and the
crontab rebuild and did not have to change.
Two things follow from the form being built over ajax rather than with the
page. The modal emits getCSRFinputHTML() itself, since nothing else wraps it.
And cron.js binds the schedule pickers after the html is inserted instead of
on DOMContentLoaded: the controls do not exist until then, and a fresh set
arrives each time the modal opens.
While moving the When row, give the pickers a placeholder when they cannot
draw the stored schedule. Previously an existing task set to something like
"*/5 * * * *" rendered the pickers at their default, so the page showed
"Every day at 03:00" in large friendly controls beside a cron field reading
"*/5 * * * *" -- two different answers at once, with the wrong one the more
prominent. Adding a task still defaults to daily at 03:00, which is a default
rather than a contradiction.
Tests: tests/php/test_cron_modal.php, 42 checks. The field names are now a
contract between files that do not mention each other, so each side is parsed
out of its own source and compared rather than restated. The three states of
the When row are checked by rendering the modal against stubs, since which
picker is selected and whether the cron field is open are decisions the php
makes. Verified the new checks fail when the placeholder, a field name or the
csrf input is removed.
Ran: tests/php/test_cron_modal.php (42 passed), test_crontab.php (88 passed),
test_allowed_commands.php (31 passed), tests/js/cron-schedule.test.js (12
passed), the rest of tests/php and tests/js, npx eslint . (clean),
utils/check-homoglyphs.py (clean). test_bandwidth_clamp.php fails on this box
for an unrelated reason: it cannot read /etc/zm/conf.d/03custom.conf and the
permission warning lands in its captured output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The form put four fields of very different widths into one twelve column row
-- Name 3, Command 5, Server 2, Enabled 2 -- so the longest value on the page
had the least room and the checkbox sat under a label with a <br/> that lined
up with nothing. The submit button then shared a row with the schedule
controls.
Worse than any of that, the pickers and the cron field stood side by side as
equal peers: two controls for one value, with nothing saying which won. That
is the question the layout invited and never answered.
Lay it out as a horizontal form instead, one concept per row, label left:
name, command, when, server, enabled, then the buttons on their own row. The
command gets the full width since it is the longest thing anyone types here.
The cron field moves inside a <details> under the pickers, labelled "Enter as
cron". It is still the control that is submitted, because cron can express
schedules the pickers cannot and one set that way has to survive being
edited; but it now reads as a consequence of the pickers rather than a rival
to them. A task whose schedule the pickers cannot draw opens the disclosure
on load, so it is in front of whoever opened the row rather than hidden from
them.
The list loses its separate Edit column -- the name is the link, as it is on
the other options pages -- and the schedule column carries the raw spec as a
title, so it can be checked without opening the row.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two places in ZoneMinder run a command the operator typed: a filter with
"Execute command", and the scheduled tasks added for #4957. Both run it as the
web user through a shell, which is close to shell access on the server. Until
now the only control over that was who may edit, and the answer was anyone
with System rights.
ZM_ALLOWED_COMMANDS names the programs that may be run, comma separated.
Empty is the default and places no restriction, so an upgrade changes nothing
and the filters people already have keep working; setting it narrows both
features at once.
Only the program is matched, because an exact whole-command match could never
work for filters: zmfilter.pl appends the event path to whatever the filter
stored, so no two runs have the same command line. Arguments are therefore
free, which means the program being on the list has to be the whole of the
guarantee -- so while a list is in force a command may not contain the
characters that start another program: semicolon, pipe, ampersand, backtick
and dollar-paren. Without that, "/usr/local/bin/backup.sh; anything" would
pass on the strength of its first word. Redirection is deliberately allowed;
it starts no program, and refusing it would break the ordinary
"... >> /var/log/mine.log".
Refused in three places, because each answers a different question. The
filter and scheduled task editors refuse on save, so the operator is told at
the time rather than by a log line days later. zmfilter.pl refuses at the
moment it runs, because a row can reach the database without passing through
the web ui and because the setting may have been narrowed since. And
cronRebuild leaves a newly disallowed task out of the crontab, so tightening
the setting takes jobs away rather than waiting for somebody to edit each one.
The rule exists twice, in web/includes/Command.php and ZoneMinder::General,
since one is PHP and the other Perl. Rather than hope they stay in step,
tests/php/test_allowed_commands.php puts every case to both and fails if they
disagree. That earned itself immediately: both split the command on
whitespace before handling quotes, so a program path containing a space was
cut in half and compared as the wrong string. The cross-check found it in
both at once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is the rest of what #4957 asked for -- run Perl, PHP, bash and binaries
on a flexible schedule, from a visual interface -- and it needs no daemon,
because cron is the daemon. 1.39.37 already taught ZoneMinder to rewrite a
marked block in the web user's crontab for scheduled monitor actions while
leaving every line outside that block alone. A CronJobs row is another line in
the same block.
So the parts that would otherwise have to be written and then supervised --
timing, catch-up, restart on boot, running as a known user -- are cron's, and
have been since 1975. What ZoneMinder adds is the table, the page, and the
rule that the crontab is a projection of the table: rebuilt whenever a row
changes, never the other way round, so the two cannot drift.
The schedule uses the same pickers the monitor actions got, reading "Every
Monday at 7pm", with the cron field beside them for anything the pickers
cannot say. The pickers build the spec in the browser so the field fills in as
you choose, and the server builds and validates it again on save, because the
browser is not what keeps a crontab line honest. The two mappings are checked
against each other in tests/js/cron-schedule.test.js, which reads the PHP map
rather than restating it.
The command is not judged on what it does; running things is the point. It is
judged on whether it stays on its own line and means in cron what the operator
meant: a newline is refused because it would end the entry and let the rest
become a line with a schedule of its own, and an unescaped % is refused
because cron stops the command there and feeds the remainder in as stdin,
which would silently run less than was typed. Cron's own escape, \%, is
accepted.
Editing takes System edit rights, and every add, change and delete is logged
with the command. Anyone who can edit here can run commands as the web user,
which is worth saying plainly -- though it is the same trust Filters'
AutoExecuteCmd has carried for years, not a new one.
ServerId NULL means every server writes the job into its own crontab, which
is what a single server install wants without having to say so; naming a
server restricts it to that one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five cron fields are a reasonable thing to store and an unreasonable thing to
put in front of an operator. The editor now reads as the sentence it means:
Every [day v] at [08:00]
Every [Monday v] at [19:00]
Every [weekday (Mon-Fri) v] at [07:30]
Cron stays the stored form, because it is what goes in the crontab and it can
say things the two controls cannot. cronFromParts turns what the editor
collected into a spec and cronToParts turns one back, so a saved schedule
loads into the same controls that made it.
cronToParts is deliberately partial. A spec the controls cannot draw -- every
fifteen minutes, the first of the month, hourly, a pair of weekdays -- comes
back null, and the editor shows it as it stands and saves it back untouched
through a read-only field. Rounding someone's schedule to the nearest thing
the pickers understand would be worse than admitting the editor cannot draw
it.
The time comes from <input type="time">, so it is shown in whatever format
the operator's locale uses; 8am and 08:00 are the same control. The schedule
is still built and validated server side, since the browser is not what keeps
a crontab line honest.
Tested for the round trip over every frequency and a spread of times: what
the editor can build must survive being saved and loaded back, or editing a
row would quietly change when it runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
that already exist -- lights on, lights off, play a sound -- at set times.
MonitorActions already says what to do and to which device, with triggers for
EventStart, Alarm, Manual and so on. So this adds a trigger rather than a
subsystem: an action whose TriggerOn is 'Schedule' carries a five field cron
spec, and cron runs it.
Cron, rather than a daemon of our own, because the web user owns a crontab it
can rewrite without privilege, and because the line cron runs,
30 22 * * * /usr/bin/zmcontrol.pl --id 24 --command=lightOff
is the same invocation web/includes/Monitor.php already falls back to when no
zmcontrol daemon is listening. A scheduled action therefore takes the path a
manual one does rather than a parallel one of its own. It also means no new
code runs as root, nothing new has to be supervised, and the feature is a
table column plus a crontab block.
Nothing a form can set reaches a shell. The schedule is refused unless
cronScheduleIsValid() accepts it -- five fields, numbers and the usual
*/,-/ only, with newline, carriage return and % rejected outright because
each would end the line or the command. The command itself comes from
MonitorAction::commandFor()'s fixed list and the ids are integers, so the
whole line is assembled from constrained parts. That is a narrower surface
than Filters.AutoExecuteCmd, which has taken an arbitrary command for years.
The crontab is not ours to own, so ZoneMinder rewrites only a block between
two markers and leaves every other line byte for byte. A block whose end
marker is missing, which is what an interrupted write leaves, is treated as
no block at all rather than swallowing everything after it: losing our own
stale block is the safe failure, losing the admin's entries is not. The block
is rebuilt from the table on every save, so the crontab is a projection of
the database rather than a second place to keep the truth.
zmc recognises the new trigger and skips it, so having one does not warn on
every reload, and nothing fires a scheduled action from the capture process.
An action targeting a monitor on another server is left out and logged: cron
runs zmcontrol.pl locally, and the web ui's proxy through the API has no cron
equivalent. Single server installs are unaffected.
The two pieces that can lose data or run the wrong thing are pure, and
tests/php/test_crontab.php covers them: schedule validation including the
injection cases, and splicing a block in, out and over without disturbing the
lines around it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When a monitor's zms exits, the status poll gets reason=no_socket and the
stream is reloaded under a new connkey. A no_socket reply takes over a second
(stream.php waits for the socket file to appear) while the poll runs every
second, and every request on the page shares one ajaxQueue, so a second poll
for the dead connkey is usually already queued when the first reply arrives.
Its reply then reloaded the stream again: it sent CMD_QUIT to the zms the
first reload had just started and replaced the connkey a second time.
Each restart of an unreachable monitor therefore left an orphaned zms behind,
and a poll queued for that orphan between the two reloads could find it
shutting down. stream.php then waited the full MSG_TIMEOUT and logged
"Timed out waiting for msg ... after waiting 5000 milliseconds", while every
other monitor's poll on the page waited behind it in the shared queue.
Bind each reply to the connkey its request was sent with and drop replies
for a connkey we no longer hold, as EventStream already does. alarmCommand()
passes its connkey explicitly, since binding getStreamCmdResponse directly
would hand it jQuery's textStatus in that argument.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
monitors/alarm/command:on now takes an optional cause (query or POST data)
and passes it to zmu --cause, so the event records it in place of
"Forced Web". Without a cause nothing changes.
The cause is stored as the event's Cause and in its Notes, so alarmCause()
keeps it to plain text: letters, digits, spaces and plain punctuation, cut
to the 31 bytes zmu stores on a character boundary, falling back to the
default when nothing is left. The value is quoted with escapeshellarg()
for the zmu command line.
Test: tests/php/test_monitor_alarm_cause.php
fixes#5348
Follow-up hardening to the multi-server control forwarding added in
#5343:
- remoteControlUrl() now forwards an allowlist of known control
parameters (id, control, xge, yge, x, y, scale, preset, newLabel,
action, aid, response) instead of passing everything that is not on a
denylist, so no stray key or cookie-folded value reaches the other
server.
- forwardControlRequest() sets a 15s timeout so an unreachable recording
server can't hang an interactive control click for default_socket_timeout,
and, with ignore_errors, treats a non-2xx response (e.g. an auth-relay
login redirect's HTML) as a failure.
- relayControlReply() refuses to pass a non-JSON body back to the browser
under a JSON content-type.
Extends the test to assert unknown parameters are not forwarded.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
For a monitor recorded by another server, sendControlCommand() put any
zmcontrol command into a monitors/daemonControl URL, which only takes
start, stop and quit. Those still go there; other commands now log why
they were not sent and return false instead of requesting a malformed
URL.
fixes#5342
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In a multi-server setup a PTZ move, preset or light command sent to one
server for a monitor that another server records never reached the
camera. Monitor::sendControlCommand() passed the zmcontrol option string
to the other server's monitors/daemonControl API, which only starts and
stops zmcontrol.pl, so the request built a URL like
daemonControl/4/ --panspeed=51 ... --command=moveConLeft/zmcontrol.pl.json
and answered "Failed". The web UI's own PTZ buttons send to the
monitor's server and were not affected; clients that send every request
to the server they log in to (zm-py, pyzmNg, zmNinjaNg, aiozoneminder)
and monitor actions whose target is on another server were.
ajax/control.php now hands such a request, as received, to the index.php
of the server that records the monitor (or the action's target), with
the auth relay in place of the caller's credentials, and answers with
that server's reply. That server checks the permissions and builds the
command itself. A request is forwarded at most once.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
changeScale() looked up document.getElementById('videoFeedStream'+id)
and set stream.style.overflow in every scale branch without checking
that the element exists. Gapless playback sizes the video.js player
from the DOMContentLoaded handler, before that element is in the DOM,
so changeScale threw "Cannot read properties of null (reading 'style')"
and the event-gapless-layout JS test failed.
Collect the overflow value into a local and apply it once behind a
null guard. Behaviour on a built page is unchanged: scale 100 still
gets 'auto hidden', every other scale 'hidden'.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ZM_Object's constructor takes either an id (which it loads from the database)
or an array, which it trusts as the record's own fields. Several request-fed
call sites passed the raw request value straight to new ZM\Event(), so a request
that sends an array instead of a scalar -- eid[Id]=...&eid[MonitorId]=... -- had
its fields adopted as the event row. The request then chose the MonitorId that
canView() checks and the StorageId/Scheme/Id that Path() is built from.
Impact: view_video.php turned this into an arbitrary file read (forge a viewable
MonitorId, point Path() at any directory, append file=); ajax/events.php let a
forged row pass the per-event permission check so archive/delete acted on a row
the user could not see, and delete ran unlink on the forged Path().
validInt()/validCardinal() already return '' for a non-scalar, so route the
request value through them first; an array becomes '' and ZM\Event loads the
real row (or none). Fixed at the two affected sinks and centrally in
getAffectedIds(), which several action handlers use to build the id list.
Addresses GHSA-6h8f-gx4m-qqw4.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
packageControl() runs zmpkg.pl (which starts, stops and switches the run state
of ZoneMinder) with the state name from the request. It was public, so CakePHP
routed GET /api/states/packageControl/<command> straight to it, skipping the
System=Edit check in change() -- the method's only intended caller. With
beforeFilter() admitting any user whose System permission is not None, a
System=View user could stop, restart or change the run state.
Make it protected. change() still calls it after its System=Edit check; the
direct route now raises PrivateActionException (404).
Addresses GHSA-9gc6-9xg2-5636.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove onStatsResize call when event stats are shown.
We are already executing `changeScale()` within the `DOMContentLoaded` event listener. `changeScale()` itself calls `onStatsResize()`.
Calling `onStatsResize()` again—while specifying the original dimensions of the event—results in an incorrect event size when `Scale=Auto` is used and statistics are displayed; furthermore, it leads to unnecessary processing.
Prevent the "audio-motion canvas" background from disappearing when stream playback is paused and the browser window is resized in Chromium. Applicable to Event pages.
Updated event scaling logic to use real event dimensions for better accuracy. Adjusted eventData width and height references to ensure correct scaling based on video metadata.
getNearEvents() built the prev/next query from the full (sort value, Id) tuple,
which already orders events strictly in the chosen sort, but then also added
AND E.StartDateTime >= / <= the current event's time. That extra predicate is
only consistent when the sort field is StartDateTime. When events are sorted by
another field -- MonitorName to group cameras, or Id -- it dropped events that
are correctly next in that order but started earlier or later than the current
one: the next monitor's first event (which often started before the current
monitor's last), or a concurrently recorded event with a higher Id. Gapless
playback then skipped over the events of every monitor but the first.
A fresh, single-timeline database rarely shows this because Id order tracks
time order; a database with several monitors recording in parallel, or one
migrated from another system, makes the two diverge and exposes it.
Drop the StartDateTime constraint and let the tuple be the whole ordering. It
cannot reintroduce the earlier playback loop because (sort value, Id) is a
strict total order. refs #4186
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- On the user page, "Default" is now displayed instead of an empty string in the language selection settings.
- Added a Russian translation for "Default".
- When calling `monitorStream.manageAvailablePlayers()`, now pass the candidate player as an argument instead of determining the player from stored cookies inside `manageAvailablePlayers()`. This is necessary because cookies could belong to either the Watch page or the Zone page.
- On the Zone page, load `hls.min.js` if Go2RTC or RTSP2Web is being used for the monitor.
- `monitorStream.setPlayer()` is now executed within `monitorStream.manageAvailablePlayers()`. Previously, `manageAvailablePlayers()` only handled the assignment of `monitorStream.player`.
- Added a new function, `this.setRTSP2WebType()`, to avoid code duplication. This ensures that `RTSP2WebType` is checked and set even if `setPlayer()` is not called on the page.
- Minor optimization regarding variable usage.
- If `hls.min.js` is not loaded, attempting to play via HLS will now trigger a "The HLS library is not loaded" warning in the browser console and initiate a stream restart. Previously, this would result in an error.
