Running cmake in the tree, and the perl module build, leave 22 files and
directories behind that nothing ignored: CTestTestfile.cmake at the root and
under tests/, DartConfiguration.tcl, _deps/, a tests_include-<hash>.cmake
whose name changes with the test list, __pycache__/, and the
ExtUtils::MakeMaker leftovers (MYMETA.json, MYMETA.yml, MakefilePerl, blib/,
pm_to_blib, output/) under each of scripts/ZoneMinder, onvif/modules and
onvif/proxy.
They were untracked and unignored, which makes git add -A a trap: it sweeps
all of them into the commit. That is how 98k lines of generated perl nearly
went in with the Importance logging change, and how the AGPL licensed
audioMotion-analyzer.js and two cmake files did go into a commit on the
audio-level-graph branch before being taken back out.
The MakeMaker names are matched without a path because they appear under
three different directories; output/ is matched by full path instead, since a
bare output/ would claim any directory of that name anywhere in the tree.
Verified that no tracked file is shadowed by any of these, and that git add -A
now stages nothing generated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Reverts the previous commit's always-on measurement. Decoding audio for every
monitor that has it spends CPU on a number almost nothing reads, which is the
wrong trade even though it did solve the chicken and egg of picking a
threshold without ever seeing a level.
Measure when something is actually going to use the reading instead:
- AudioDetection is on, as before, so nothing changes for a monitor that
scores on audio; or
- somebody asked. SharedData gains audio_level_until, a wall clock second
the capture thread keeps measuring up to. The monitor editor's new level
meter pushes it forward while it is on screen and the measurement lapses a
few seconds after the page is left, so nothing has to send a stop and a
crashed browser cannot leave a monitor decoding forever.
When the reading stops being wanted the decoder is released and the published
level and peak are cleared, so a stale number is not left looking current and
an old peak does not land on the next frame row written.
audio_level_until is carved out of analysis_pad rather than appended, so
SharedData stays 888 bytes and no existing offset moves; the static_asserts,
Memory.pm and Monitor.php are updated together and all three now agree the
field is at +880.
The meter itself is on the audio settings, shown whether or not
AudioDetection is checked, because the level is what you need in order to
choose a threshold. It draws the threshold currently in the input as a mark on
the bar so a reading can be judged against it before saving, and a monitor
whose zmc is not running reads "no reading" rather than a confident 0, which
would be indistinguishable from silence.
Frames.AudioLevel is therefore 0 again on monitors that do not score on audio.
That is what the graph already treats as "no audio data", so it draws no line
rather than a flat one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Gating the measurement on AudioDetection made the graph useless for the job
it is most wanted for. AudioThreshold is a per-device number -- the floor on
one camera's mic is nothing like another's -- so it has to be measured before
it can be set, but nothing was measured until it was already set. Enabling
detection with a guessed threshold to find out what the real one should be is
backwards.
The level is now read for every monitor with decodable audio. AudioDetection
governs only whether crossing the threshold contributes a score, which is
what the setting is named for. shared_data->audio_alarm stays 0 when it is
off, so nothing downstream changes for a monitor that does not want audio
alarms.
Nothing here depends on Analysing either. The measurement is in
Monitor::Capture, which runs on whatever Analysing is set to, and frame rows
come from Event::AddFrame, which a continuously recording monitor reaches
through the RECORDING_ALWAYS path with motion detection off. So a monitor
that only records continuously still gets levels on its rows.
Since Monitor::Capture retries Open on every audio packet until it succeeds,
and that now happens for every monitor with audio rather than the handful with
detection on, AudioDetector remembers a codec it has already failed to find a
decoder for. Without it a stream ZoneMinder cannot decode logs a warning at
the audio packet rate for as long as the monitor runs. A reconnect bringing a
different codec is still tried.
The cost is one audio decode per monitor with audio, where before it was one
per monitor with detection enabled. That is small next to the video path, but
it is not nothing on a box with many cameras.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
The cue strip under the video showed one flat red band per alarm period. It
tried to encode the score as a bar height, 'height: '+frame.Score+'px', but
never could: the frames ajax did not return Score, so every bar got
'height: undefinedpx' and fell back to the stylesheet's height: 100%. So the
motion level it looks like it is drawing has never actually been drawn.
Replace it with a line graph of both series over the length of the event. The
alarm periods stay, as a pale wash behind the lines, so nothing that was
readable before is lost.
The two series do not share a vertical scale. Audio level is 0-100 by
construction, but a motion score is a sum over zones with no upper bound, so
pinning both to 0-100 would flatten the audio line against the floor on any
event scoring above 100. Each is scaled to its own maximum, and hovering reads
out the exact values, which is what the numbers are wanted for. An event whose
rows are all zero -- recorded before the column existed, or by a monitor with
AudioDetection off -- draws no audio line at all, rather than a flat line
claiming silence was measured.
The readout goes into the existing #indicator, which already tracks the mouse
across the whole progress bar, instead of a second tooltip competing for the
same pixels.
The geometry lives in web/js/LevelGraph.js so it can be tested without a DOM,
and so the polyline and the hover readout cannot disagree about where a given
second sits. The bar grows from 1.25em to fit the graph, which needed two
consequential CSS changes: #indicator now spans the taller bar, and
.progressBox drops from 0.66 opacity to 0.25, because at full strength the
played part of the graph is unreadable behind it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
zm_update-1.39.31.sql gave monitors audio detection, but the level only ever
existed in shared memory, so it was gone the moment the frame passed and there
was nothing for the event view to plot. Add Frames.AudioLevel next to Score, on
the same 0-100 dBFS-derived scale the threshold uses.
What is stored is the peak since the previous row, not the level at the instant
the row was written. Frames rows are written well below the capture rate --
only alarm, bulk and score-increasing frames get one -- so sampling at write
time would drop exactly the short loud noises worth seeing on a timeline.
AudioDetector accumulates the peak as it decodes and Event::AddFrame takes it
where the row is built, which clears it so each row covers its own interval.
The Event constructor takes and discards it once, otherwise an event's first
row reports the loudest moment since the previous event ended.
This needs no shared memory change: zma is now an offline re-analysis tool and
the live analysis runs in a thread of zmc, alongside the capture thread that
runs the decoder, so the peak can stay in the AudioDetector. SharedData keeps
its documented 888-byte layout and its fixed offsets.
The frames ajax returns the column, and Score with it. elements in
web/ajax/status.php is a whitelist that never listed Score, which is why the
event view's cue strip has been reading an undefined Score off every frame.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
changeRate's reverse branch did not use the selected rate as the reverse
speed. It looked it up as rates[rates.indexOf(-rate)-1]/100, stepping one
entry further down the shared rate list, so every reverse rate ran a notch
too slow: -1/2x played at 1/4x and -16x at 10x. -1/4x was worse than slow,
because one step below 25 in that list is 0, so revSpeed came out 0 and the
video sat still while the ui claimed it was rewinding.
The rate the user picked is the speed, so use it.
Leaving reverse through the dropdown also leaked the rewind interval, which
only pauseClicked and vjsPlay ever stopped. Picking a forward rate after a
reverse one left it running, so it went on dragging currentTime backwards and
resetting playbackRate to 0 on every tick while the player was supposedly
running forwards. The teardown is now stopRewind(), split out of
stopFastRev() because stopFastRev rewrites the rate select to 1x, which would
undo the choice changeRate is in the middle of applying.
stopFastRev no longer reads the rate back out of the player to decide what to
put in the select and the cookie, for the same reason streamFastFwd stopped
doing it: videojs can defer the set until the tech is ready, so the getter
still answers with the rate we just left.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Clicking fast forward once too many at 16x threw out of video.js:
TypeError: HTMLMediaElement.playbackRate setter: Value being assigned is
not a finite floating-point value.
playbackRate@video.min.js
streamFastFwd@..._views_js_event-....js
streamFastFwd stepped the shared rate list by indexing it directly,
rates[rates.indexOf(current)+1]. At the top of the list that is rates[15],
undefined, and undefined/100 is NaN, which Firefox refuses outright.
The guard meant to prevent this ran after the assignment rather than before
it, and read the rate back from the player to decide, so it could only
disable the button once the bad value had already been sent. It was reachable
in normal use because streamPlay() re-enables the button whatever rate we are
at, so play-then-fast-forward at 16x throws every time; picking 16x from the
rate dropdown gets there too, since changeRate does not touch button state.
indexOf also answers -1 for a rate that is not in the list, and -1+1 indexes
rates[0], which is -1600: stepping forwards from an unlisted rate asked for
16x reverse.
streamFastRev had the same fault at the other end. rates[0-1] is undefined, so
revSpeed became NaN and every tick of the rewind interval then handed
currentTime a NaN.
Both now go through stepRate, which snaps an unlisted rate to the nearest
listed one and returns null rather than walking off either end, so the caller
disables the button and leaves the player alone instead of assigning
something it cannot use. streamFastFwd also sets the dropdown and cookie from
the rate it just asked for rather than reading it back, because the stack in
the report shows videojs deferring the set until the tech is ready, at which
point the getter still answers with the old rate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
A speaker that has dropped off the network fails on every command until
somebody fixes it, so that is the one error in IPSpeaker that repeats forever
rather than once. On a monitor deliberately marked unimportant it is noise,
and it buries the failures worth acting on.
Nothing about that is specific to speakers, so the policy goes in Logger as
importanceLevel, with ErrorImportance and WarningImportance alongside the
plain Error and Warning for callers to use. It takes the importance value
itself rather than a monitor, so Logger needs to know nothing about monitors
and callers that have some other notion of how much something matters can
still use it.
zmwatch.pl has been open coding the same idea as
WARNING+$monitor->ImportanceNumber() in three places; it now calls
WarningImportance instead, which is the same arithmetic and so leaves its
levels exactly as they were, including the Not important case that lands in
DEBUG1. That case is pinned by a test rather than quietly corrected: it is
long standing behaviour and not this change's business. zmwatch no longer
needs the logger object it was keeping for logPrint, so logInit() is called
bare there as it is in every other script.
Anything that is not a number counts as Normal, so a caller with no monitor
to ask still reports in full: not knowing how much a monitor matters is no
reason to hide its faults.
IPSpeaker is then a one line change at the call site. Only the failure to
reach the device is weighed; a refusal or unparseable content means the device
answered and something is really wrong, which is worth an error however
unimportant the monitor is, and does not repeat the same way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
The loader looked for a named AudioMotionAnalyzer export and otherwise fell
back to window.AudioMotionAnalyzer, which is what the UMD bundle sets. The
src/ ES module that the import path actually expects exports the class both by
name and as its default, and default is the form upstream documents, so take
module.default as well. Without it a build that exports only a default would
silently fall through to an undefined window global and throw on .version.
Throw a named error when none of the three yields a class, so the existing
catch reports LoadFailed instead of a TypeError on undefined.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
The library is AGPL-3.0-or-later so it is not shipped, and the admin installs
it at skins/<skin>/assets/audioMotion-analyzer/src/audioMotion-analyzer.js.
The instructions for doing that had drifted from what the code loads:
- help.txt and the OPTIONS_WHATTODISPLAY help gave the bare
https://cdn.jsdelivr.net/npm/audiomotion-analyzer@X.X.X URL, which resolves
to the package's "main" entry, the minified UMD bundle dist/index.js. The
install path is the package's src/ ES module, so the URL needs the explicit
/src/audioMotion-analyzer.js suffix. Same for the download links in the
AudioMotionVersionNotInstalled and AudioMotionVersionWrongVersion messages.
- The install path was written as /skins/MySkin/..., a placeholder that does
not correspond to any skin.
- RequiresAudioMotionEnabled named only the file, not where it goes.
- assets/version documented every other asset in that directory but not this
one, leaving no explanation for the otherwise empty directory.
help.txt no longer restates the required version, so 4.5.4 stays declared only
by SUPPORTED_AUDIO_MOTION_ANALYZER_VERSION as intended, and assets/version
points at that constant rather than duplicating it.
Add tests/js/audiomotion-paths.test.js to hold the PHP feature probe, the
dynamic import, help.txt and both lang catalogues to the same path, the same
download URL and the same version.
Also gitignore the installed library so a local install is not committed back
into a GPL-2.0 tree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
LWP::UserAgent defaults to 180 seconds. Actions for a monitor are serialised
through one control daemon, so a speaker that has dropped off the network holds
that daemon for three minutes per attempt and every command queued behind it
waits. AMLink already sets a timeout; this did not.
Ten seconds, matching AMLink. The speaker answers in milliseconds when it is
reachable at all, so this only ever bounds the case where it is not - which is
the case right now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
The camera does not report an expired session as a JSON error. It answers with
a bare printable string, "Invalid session in request", where a masked base64
payload belongs. Unmasking that and base64 decoding it produces noise, so a
routine timeout was logged as "malformed JSON string ... at character offset
0" - which named neither the session nor the camera's own explanation.
Detect it before unmasking, log what the camera actually said, and treat it as
a lost session so the existing recovery re-establishes it. session_error is
pure so the wording seen on the wire is pinned by a test rather than by a
camera happening to be in the right state.
refs #4423
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
zmcontrol sent keepAlive only from the branch that runs when select() times
out. A monitor taking a steady stream of commands never reaches that branch,
so it never pinged at all, and cameras that expire a session on a timer
refresh it on the keepAlive rather than on ordinary requests. The monitors
with the most traffic were therefore the ones that lost their session.
Measured on monitor 1: it pinged twice while idle, then took a command every
three seconds for three minutes and the camera rejected the next one 181
seconds after the last ping. Its light commands were being dropped for as long
as the motion kept coming, which is exactly when they matter.
Move the decision into Control::keepAliveDue, timed from the last ping, and
check it at the top of the loop so the 'next' paths in the command handling
cannot skip it. A clock step backwards resets the baseline rather than holding
the ping off until real time catches up.
refs #4423
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
An undecodable reply means the camera and this module no longer agree about
the session, but nothing put that right. Dahua_RPC re-logs-in only when it gets
a parseable error back, so rpc_call returning undef left the session poisoned:
in the logs a CoaxialControlIO.control failure is followed by every later
keepAlive and logout on that session failing the same way, until something else
forces a login. A light switched on by an alarm stays on for that whole period.
Split the single attempt out as rpc_once, recording why it failed, and have
rpc_call re-login and retry once when the failure was a decode failure on a
masked Request.
The guards matter more than the retry:
- bootstrap channels never recover, since login() issues them
- login() calls logout(), a Request on the dead session, so a re-entrancy guard
stops its own decode failure starting another recovery
- one retry only, and a 5 second backoff, so a camera that always answers
unreadably is not met with a login per command
refs #4423
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
The decode failure diagnostic appended its fields after $@, which ends in a
newline, so the payload length, alignment, unmasked-decode result and leading
bytes were pushed onto a second line where nothing looked for them.
$@ was also read after the unmasked-decode eval had already reset it, so the
error reported was that eval's outcome rather than the failure being described.
Add log_safe to flatten a message before logging, use it for the three places
that interpolate $@, capture the real error before running another eval, and
report the decoded byte count and the decoded leading bytes - the base64 alone
does not say whether what failed to parse was truncated or simply not ours.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
This can happen, for example, on the Console page when hovering over an image thumbnail, because the `video-stream` element created for go2rtc in `createGo2rtcStream()` is not assigned an ID.
html_radio() emits <div>s, and a <div> closes an open <p>, so the Email
Format radios were parsed out of their row and landed on the line below
their label. Wrap that row in a div instead. The row label's width now
applies to direct children only, so the radios' own labels keep their
natural size.
Separate the email settings from the options above them with a rule,
rather than having Email To run straight on from Lock Rows.
zmfilter.pl falls back to ZM_EMAIL_HOST when a filter names no server, so
show that (or localhost, its default) as the Email Server placeholder
instead of leaving the field looking unset.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The panel's messages came from the global translate object that
views/js/log.js.php defined, which the footer only loads for the Log
view. On any other view Clear Logs threw "translate is not defined" out
of deleteLogs() before it sent anything, and a failed table query would
have thrown the same way.
Carry the five strings on the panel element as data-i18n and read them
in initLogPanel(), so a panel is self-contained wherever it is embedded.
views/js/log.js.php held nothing else, so it goes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The legend sat directly on the line the form draws below its buttons.
A fieldset's legend renders in the border box, above the padding, so
padding-top does not move it; give the fieldset a top margin instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Actions and Options rows are flex lines, so a checkbox sat next to a
growing text input or select was shrinkable and rendered 16.5px against
the 18px of the rows that have nothing after them -- visible on Execute
command on all matches, and on Copy/Move once their storage select is
shown. Take the checkboxes out of the shrinking.
#content is a flex column, so once the log panel became a second child
the form shrank below its content and, having overflow:auto from
skin.css, grew a scrollbar of its own that cut the Actions list in half.
Let the form keep its height and leave the scrolling to #content, which
now shows the whole view in one scroller.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
renderMenuIcon() emits the material default of 24px, so every top navbar
item was 40px tall rather than the 36px the bar was before it had icons,
and the menu was wide enough that the collapse toggle wrapped onto a row
of its own: 76px of header where 1.38.4 used 55px, on every view.
Render the navbar's menu icons at 18px, which is the size the statistics
bar below already uses, and lay each link out as a flex row so the icon
and its label stay centred on each other. Drop the collapse toggle's
1rem side padding, which was 32px of the width that caused the wrap.
#navbar-container is 55px again, matching 1.38.4 exactly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the static 50-row table with the panel the Log view uses, locked
to this filter's component. That brings the date range, level and server
filters, search, sorting, paging, the column chooser and export, and
lets a System editor clear the rows it lists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Log view was the only way to read a log, and its toolbar, its
bootstrap-table and the script behind them were written into the view, so
a second view could only have a log by copying all three.
Move the markup to skins/classic/includes/logpanel.php as
getLogPanelHTML(), and the behaviour to skins/classic/js/logpanel.js,
which picks up every .logPanel in the page and drives it. The script
keeps its request state, its selection and its in-flight XHR in a
closure, so two panels on one page no longer share them; the table gets
its ajax function from that closure instead of the global that
data-ajax="ajaxRequest" resolved to. Options cover what the call sites
differ on: the element id prefix, a fixed set of components, the page
size and whether the Back and Refresh page buttons are shown.
A panel locked to components sends embedded=1, and ajax/log.php then
leaves zmLogComponent and zmLogFilterLevel alone, so an embedded panel
cannot overwrite what the user chose in the Log view. Such a panel also
drops the component selector and column, and the unfiltered total, which
counts every log row and means nothing once the panel is scoped.
The level colours and the table's column rules move from the per-view
log.css files to the matching skin.css, since they now apply wherever a
panel is embedded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both the background daemons (zmpkg starts one zmfilter.pl per background
filter) and the Execute button run "zmfilter.pl --filter_id=N", and
zmfilter.pl calls logInit(id => 'zmfilter_N'), so the Logs rows with that
component are exactly the selected filter's output. Until now the only
way to read them was the Log view, where they had to be picked out of
every other component.
Show the 50 most recent under a Filter Log section at the bottom of the
view, for a saved filter and a user who may view System. The section is
rendered with the page rather than fetched, because every action that
writes to this log -- Execute, Save, switching filters -- reloads the
view anyway. Level colours are duplicated from log.css, which is not
loaded here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Actions and Options were floated halves with a clearfix and a fixed
320px label width. That left a wide gutter between every label and its
checkbox, pushed the AutoExecuteCmd input onto its own line at the far
left, and overflowed the Options column so the "seconds" after Execute
Interval was clipped. Make the pair a wrapping flex row, centre a
max-content block in each column, and lay each row out as a flex line so
labels stay right aligned against controls that follow inline.
skin.css sets .filterTable { display: flex } for the <div> variant on
the events toolbar. The filter view's elements really are tables, so the
rule blockified their tbody and left #sortTable aligned to the left edge
instead of centred. Restore table layout for table.filterTable.
Also give the term value column a width and let its input fill the cell
so rows line up whichever attribute is selected, replace the 500px
min-widths on the filter selector and the Name input with
min(500px, 100%) so narrow screens no longer scroll sideways, and let
#contentButtons wrap rather than float and clip its last buttons.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The term rows and the sort/limit row sat above Actions and Options with
no heading of their own, so the top half of the form read as loose
controls. Wrap them in a fieldset with a Query legend to match the two
sections below it.
The EmailFormat radios were rendered inside the <label> that titles
them, which made the label the control's own click target and left the
radios unstyleable as a row. Close the label before the radios and mark
the paragraph with an EmailFormat class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Filter::widget() opened <table id="fieldsTable"><tbody> and returned
without closing either. On the filter view the parser recovered because
the next sibling was another <table>, but on the events page the
following </div> closed the toolbar column while the table stayed open,
so the remaining toolbar columns were foster-parented out of place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cycleStart() took a fresh setInterval id straight into cycleIntervalId without
clearing what was already there. Once overwritten the old id is unrecoverable,
so the orphan keeps calling nextCycleView every second and cyclePause() can only
ever stop the last one armed.
Several callers reach cycleStart() with no cyclePause() in between: the play
button, the are-you-still-watching modal closing, and startPage(). That last one
is the routine path - it runs from visibilitychange, from resume and from
pageshow, and a restore fires more than one of those, so a tab coming back while
cycling was active armed two. The monitor restart in the same function is
protected, since it nulls prevStateStarted on the way through, but the cycle
branch below it never cleared prevStateCycle.
Clear the interval at the top of cycleStart(), which makes every caller safe
whatever order they arrive in, and clear prevStateCycle in startPage() the way
prevStateStarted already is. cycle.js has the same shape in its own cycleStart()
behind a play button, so it gets the same guard.
Tests in tests/js/watch-cycle-interval.test.js drive watch.js under stubbed
timers and count what is left running: two starts leave one interval, a pause
after three starts leaves none, and a second startPage() does not re-arm. Three
of the four fail against the unfixed file.
Full JS suite passes, ESLint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkQwahn9pi1y4wJe9BTxjM
The staleness window was unsound. calculateAuthHash() keys the hash to the
clock hour it was minted in and getAuthUser() accepts the last
ZM_AUTH_HASH_TTL hourly buckets, so a hash dies at the top of an hour rather
than at some age. generateAuthHash() then serves the cached one until it is
half a TTL old, so what arrives can already be nearly spent: on the defaults a
hash minted at 10:59 is still handed out at 11:58 and is refused at 12:00. A
client stamping that arrival as fresh for an hour skips the probe until 12:58
and restarts its streams on a dead hash - the exact failure this was written to
prevent. No fixed window is safe, because the remaining life of a hash we hold
can be anything down to zero, and AUTH_STALE_MS also ignored the configured
ZM_AUTH_HASH_TTL.
So drop AUTH_STALE_MS, authIsStale() and authFreshAt, and have whenAuthFresh()
revalidate. The one case that can still skip the probe is having no hash at all
- authentication off, or a relay form that does not use one - where there is
nothing that can expire and nothing a probe would report. revalidateAuth()
already shares one request between concurrent callers, so a resume that wakes
several of these still costs a single probe, and that is what the montage code
did unconditionally before any of this.
refreshTablesPendingVisibility() now returns as soon as it finds nothing was
deferred. It is bound on every classic page including the unauthenticated ones,
and the version before this ran the whole auth path on an empty queue, so
merely becoming visible could fire a probe with no work behind it.
Tests: two authIsStale cases removed with the function, two whenAuthFresh cases
added - a probe is sent and the callback held until it answers, and no probe is
sent when there is no hash. Reintroducing a fast path fails the first. Full JS
suite green, ESLint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkQwahn9pi1y4wJe9BTxjM
The probe went out as zmAuth.appendTo(...), carrying the very hash it existed to
replace. zm_authenticate_request() resolves a request against exactly one source:
a non-empty auth= in the URL enters the ZM_AUTH_HASH_LOGINS branch (on by
default), and when getAuthUser() rejects it the chain has already been taken, so
the userFromSession() arm below it never runs. A live session cookie then
authenticates as nobody.
Past ZM_AUTH_HASH_TTL - a tab hidden longer than two hours on the defaults, which
is exactly the case this change is for - the probe was therefore the one request
guaranteed to fail, and its failure is read as 'login', so the user was bounced
to the login page with a perfectly good session. That is worse than the 403s in
the log this set out to remove.
Send the probe bare. The session cookie is what answers, which is the question
being asked: who am I, and what is my current hash?
That also makes the failure handling mean what its comment claimed. A rejection
now really is a dead session rather than a dead hash, so redirecting to login on
it is right - and both 401 and 403 reach it, which the comment now says.
Also correct the AUTH_STALE_MS comment: authIsStale() is a strict comparison, so
a credential confirmed exactly AUTH_STALE_MS ago is still fresh, as the test
asserts.
Tests: tests/js/auth-helpers.test.js, 51 passed (4 new for revalidateAuth,
covering the bare probe, shared in-flight request, callbacks surviving a
transient failure, and login on a rejected session). Reverting the probe to the
credentialed form fails two of them.
refs #5093
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nr76CednxtDt2nPuq6WrbL
whenAuthFresh() gated on how long the tab had been hidden, which only covers
one of the ways the page stops hearing about the credential. montage's idle
timeout is another: hitting ZM_WEB_VIEWING_TIMEOUT stops every monitor, and
their status polls with them, while the tab stays visible the whole time. The
Are You Still Watching modal can then sit there for hours, so the auth hash
baked into the monitor srcs is just as dead as after an overnight sleep, and
a hidden-time gate would wave it straight through.
Track the age of the credential itself instead. ZMAuth.update() stamps it
whenever a reply carries auth_relay or auth - even when the value is
unchanged, since the server has still just confirmed it - and revalidateAuth()
stamps it too, so authentication being off doesn't leave every caller
revalidating once an hour forever. authHiddenTooLong(hiddenAt, now) becomes
authIsStale(freshAt, now); onAuthVisible() no longer keeps a timestamp.
That subsumes montage's refreshAuthAndStartMonitors(), which duplicated
revalidateAuth()'s navBar probe and fired a second one on every refocus.
Both call sites are now whenAuthFresh(startVisibleMonitors), which shares
the in-flight probe rather than racing it.
Tested: node tests/js/auth-helpers.test.js (47 passed), node
tests/js/table-helpers.test.js (8 passed), npx eslint on the changed files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkE45gkMjnkTiJiUySbe7V
Hidden tabs have their timers throttled, and frozen/slept ones stopped
outright, so nothing refreshes the credential while the tab is in the
background. The server rotates the auth hash at half of AUTH_HASH_TTL
(generateAuthHash()), so the hash baked into a stream <img> src or a
deferred table url is usually dead by the time the tab is refocused. Every
restarted stream and table poll then fires a request that 403s and logs an
auth error before anything gets around to revalidating.
Record when the tab goes hidden and treat the credential as stale once more
than an hour has passed. Add whenAuthFresh(cb), which runs cb straight away
after a short alt-tab but queues it behind a revalidation after a long
sleep, so nothing makes an authenticated request on the expired hash.
revalidateAuth() now queues callbacks rather than dropping them when a probe
is already in flight, clears the hidden timestamp only once a reply actually
arrives, and still runs the queued callbacks on a transient failure - a
network blip is no reason to leave the page's streams stopped. A 401 still
redirects to login and drops the queue.
Gate the two visibility-driven callers on it: watch.js startPage() and
table-helpers.js refreshTablesPendingVisibility(). montage.js already
refreshes auth before restarting its monitors.
Tested: node tests/js/auth-helpers.test.js (48 passed), node
tests/js/table-helpers.test.js (8 passed), npx eslint on the changed files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkE45gkMjnkTiJiUySbe7V
Auto-appending wherever a database client was found widens who can
decide what the daemons execute, and perl will not catch it: its taint
check refuses a world writable directory on PATH but permits a group
writable one.
world writable 0777 -> Insecure directory in $ENV{PATH}
group writable 0775 -> accepted
That is not theoretical. Homebrew's bin is 0775 owned by group admin, so
the previous commit would have silently handed every admin user the
ability to place a mysql binary that the ZoneMinder daemons run.
Check the directory before adding it, and when it is writable by group
or world, leave it out and say why. An explicit ZM_SCRIPT_PATH still
wins, so anyone who accepts that risk can still opt in - the difference
is that it is now a decision rather than a default.
Verified on macOS: the real Homebrew bin is refused with the warning, a
0755 directory is still appended, and -DZM_SCRIPT_PATH= overrides both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5KL9Xbi7K5aGsauLtd8tG
The scripts run under -T, so they cannot trust the caller's PATH and set
their own. Sixteen of them hardcoded /bin:/usr/bin:/usr/local/bin, which
assumes everything they shell out to lives under /usr or /usr/local.
ZoneMinder::General::findDbCommand looks for a database client on that
PATH, and zmupdate.pl and zmcamtool.pl run what it finds. So an install
whose client sits anywhere else cannot apply schema changes:
sh: mysql: command not found
Command 'mysql -u'zmuser' ... ' exited with status: 127
even with the client on the caller's PATH. Homebrew on Apple Silicon is
the case that surfaced it - the client is in /opt/homebrew/bin - but a
--prefix=/opt install on Linux has the same shape, as does anything that
keeps its database client outside the FHS locations.
Replaced the literal with @ZM_SCRIPT_PATH@, defaulting to the same three
directories plus wherever cmake actually found a client, and overridable
for packagers who want to pin it. Warn at configure time when no client
is found at all, since that failure otherwise appears much later and
says something unrelated.
Nothing changes for an install whose client is already under /usr/bin:
the directory is only appended when it is not in the list, so the
default stays exactly as it was.
Memory.pm is deliberately left alone. It has a narrower PATH of
/bin:/usr/bin, and the only command it runs is uname through an absolute
path from ZM_PATH_UNAME, so it does not need widening.
Verified on macOS across the three cases: with the client in
/opt/homebrew/bin the directory is appended; -DZM_SCRIPT_PATH= is
respected verbatim; and pointing detection at /usr/bin/mariadb leaves
the default untouched. Build clean, suite 146 cases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5KL9Xbi7K5aGsauLtd8tG
The guide was written from a verified build and install, but the steps
past that point were derived from the Debian packaging rather than run.
Working through them on Apple Silicon to a running system turned up
three things that were wrong or missing.
mysql -u root does not work. A Homebrew MariaDB authenticates root with
the unix_socket plugin, so only the operating system's root matches it;
your own account gets the administrative account, which is what plain
mariadb uses.
Nothing said ZoneMinder has to be started as ZM_WEB_USER. zmpkg.pl
compares the current user against it and tries sudo -u and two forms of
su to become it, all of which fail for an ordinary user, and stops with
"Unable to find valid su syntax". systemd hides this on Linux by
starting the unit as the web user. Now says so, and gives the
workstation alternative of running everything as one account.
The web server section did not mention that zms is a CGI binary. Apache
runs those with mod_cgi; nginx needs a wrapper such as fcgiwrap, which
Homebrew does not package, so with nginx alone the console works and
live streams do not. Also noted that the generated samples reference
/etc/pki, /etc/nginx and /run/fcgiwrap.sock, none of which exist here.
Added the hardcoded taint-safe PATH to the known gaps, since it is what
breaks zmupdate.pl on Apple Silicon: seventeen scripts pin
/bin:/usr/bin:/usr/local/bin, so the database client in the Homebrew
prefix cannot be found. Worth making configurable - it would equally
affect a --prefix=/opt install on Linux. Sys::CPU is listed too; it has
been removed from CPAN and only zmtelemetry.pl uses it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5KL9Xbi7K5aGsauLtd8tG
