Both cameras found on this network kept accurate UTC with no timezone set, so
their clocks were five hours out on the overlay while the recordings
themselves were correctly stamped. The factory daylight saving rule was also
nonsense everywhere: thirty minutes, March to September.
set_time points the camera at an ntp server and sets its timezone, with
optional summer time rules. The timezone is derived from a UTC offset rather
than hardcoded: the vendor's list starts at GMT-12:00 and steps every thirty
minutes, so -6 becomes index 12, which its own language pack labels
"(GMT-06:00) Central Time (US&Canada)". An offset with no index, a quarter
hour zone say, is refused instead of silently rounded.
These settings live in flash and set_time is meant to be safe to re-run, so
nothing is written unless it differs. Verified against a camera: an unchanged
call issues no write at all, a changed server issues exactly one, and
restoring it issues one more. The comparison recurses into the nested start
and end rules, which would otherwise always differ as hash references, and
compares as strings because the camera returns numbers and JSON booleans
interchangeably.
dstType is deliberately not sent. Supplying week based rules makes the camera
set it itself, and it rewrote the value we sent, so sending one would mean a
read back that never matches what was written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
Replaces the ONVIF subclass with a standalone protocol. ONVIF stays as it is
and remains the right choice for these cameras when only pan, tilt, zoom and
imaging are wanted; this exists for what ONVIF cannot reach on them.
The full ptz surface now goes through /api/image/ptz-ctrl, whose optType
values are taken from the web ui's PTZ_CONTROL_CMD_* constants: the eight
move directions and stop, zoom, focus including one-key autofocus, iris, auto
pan, lens initialisation, and presets by index and name. /api/image/ptz-info
adds pan, tilt and zoom position read-back, which ONVIF does not offer here.
The white supplement floodlight is dayNightMode.ledMode in
/api/image/image-param, switched between Warm Light and IR Light. ONVIF
exposes no light at all on this hardware - both GetRelayOutputs and the PTZ
auxiliary Wiper command are accepted and neither does anything measurable.
Login is the camera's five round handshake, with the session cookie returned
in the response body rather than a Set-Cookie header. The rounds are pure
functions and a captured exchange is replayed by a test: each one hashes the
raw bytes the previous hex digest names rather than its text, and getting
that wrong produces an equally plausible digest whose only symptom is a
refused login. Code 101 means the session expired, so every call re-logins
once and retries rather than dropping an operator's keypress.
Speed is clamped: the device advertises no range anywhere in its api or ui.
Preset indexes are validated before being sent for the same reason.
Standing alone also means no imports are inherited. Missing ones are invisible
to perl -c and only fail on the path that uses them, so the tests assert the
required modules are actually loaded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
These identify themselves as Manufacturer "NONE", Model "IPCamera" over
ONVIF; the real name is in the ONVIF scope (name/L5TW), in the vendor api's
/system/device-info, and in the 2C:6F:51 OUI.
PTZ, presets and imaging are inherited from the ONVIF module, which the
camera implements well - absolute, relative and continuous move, 256 presets,
brightness through sharpness, and motion and tamper events.
What ONVIF does not expose is the white supplement floodlight the deterrence
feature uses. GetRelayOutputs and the PTZ auxiliary Wiper command are both
accepted by the camera and both do nothing measurable. The light lives in the
vendor REST api at /api/image/image-param, as dayNightMode.ledMode, switched
between Warm Light and IR Light.
Reaching that api needs its login handshake: sha256 over
username+salt+base64(timestamp)+password, rehashed over the raw digest bytes
with the challenge appended, optionally iterated, with the session cookie
returned in the response body rather than a Set-Cookie header. The rounds are
pure functions so the captured exchange is replayed by a test; the raw-bytes
versus hex-text distinction in particular produces an equally plausible digest
when wrong, so it is pinned rather than trusted.
The section write echoes every field back, since a partial write makes the
camera drop what it was not sent.
Note the supplement light only engages while the camera is in night mode, so
setting it during the day reads back correctly but produces no light.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
zmcontrol dispatches straight onto the control object with
$control->$command(...). A monitor's actions and ui buttons are configured
independently of its control protocol, so nothing stops a LightOn action
pointing at a protocol with no lightOn, and Perl kills the control server with
"Can't locate object method".
Swapping a camera for a different make produces exactly that. A monitor whose
AMLink camera was replaced by an ONVIF one kept its LightOn and LightOff
actions, and ONVIF has neither, so the first alarm after the protocol change
would have taken the control server down for that monitor.
Check can() before dispatching, log which monitor and command need attention,
and answer a caller that asked for a response rather than leaving it hanging.
Direct protocol mode gets the same check, as a Fatal, since there the command
came from the command line and is a typo worth reporting.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
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>
zmconfgen.pl wrote zm_create.sql's Config rows without the Private and System
columns, so a database created from it had both at 0 for every option until
zmupdate.pl -f rewrote the table from ConfigData. The web UI relies on Private
to keep a secret out of the browser: skin.js.php exports every option that
isn't Private as a JavaScript constant on every page, the login page
included, and the Options page shows a Private value only to users who can
edit System settings (GHSA-7g3f-93q8-fhwj). So on such a database
ZM_AUTH_HASH_SECRET, which is enough to sign an admin token, was sent to
anyone who loaded the login page. A fresh RPM install is one: the spec runs
zmupdate.pl -f only on upgrade. Debian packages run -f on install.
- zmconfgen.pl writes Private and System like saveConfigToDB() does.
- ZoneMinder::Config::syncConfigFlags() sets each row's Private and System
flags from ConfigData without touching its value, and zmupdate.pl calls it
on every update, so an existing database is repaired the next time it is
updated without needing -f.
- skin.js.php never exports a password option, whatever its flag says.
Tested with zm_create.sql generated from this tree, loaded into a scratch
database: the fresh install has the 3 Private options and 1 System option
ConfigData marks. With every flag zeroed, syncConfigFlags() set those 4 rows
and on a second run changed none.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add ZM_PASSWORD_MIN_LENGTH (Options, System; default 8, 0 turns it off).
passwordProblem() refuses a shorter password, so the user page, the API
and both login-page forms apply it alike. Existing passwords keep
working; only a password being set is checked. The length counts
characters, not bytes.
There is no setting for character classes: they push people to
predictable variations (Password1!) and add less than length does.
A database whose Config table doesn't have the option yet (an upgrade
not run with zmupdate.pl -f) gets no minimum. The password inputs carry
minlength so the browser says so before the form is sent; the server
check is the one that counts. The API answers with the English text, as
for the other reasons.
Tests: tests/php/test_default_password.php.
ZM_OPT_USE_AUTH shipped as 0, so a new install accepted every request as
the default user with System=Edit. That includes requests a browser sends
on behalf of any web page a user on the same network opens, so a server
that is never exposed to the internet could still be driven from it.
Default it to 1. zm_create.sql (generated from ConfigData) gives new
installs authentication on. Existing installs keep their value:
zmupdate.pl -f loads Value from the database and writes it back, and only
DefaultValue changes. zm_update-1.39.41.sql also sets DefaultValue to 1
for the row, so that an upgrade run without -f doesn't leave
Value = DefaultValue = 0, which the C++ config loader would skip in favour
of its compiled-in default (on) while the web UI read off.
Update the option help and the user guide (getting started, FAQ, system
options) to say authentication is on for new installs and kept on
upgrade. tests/perl/test_auth_default.pl checks the default, the stored
value and that no update script sets the row's Value; tests/zm_config.cpp
now expects the compiled-in default (opt_use_auth) to be on.
A boolean option set in zm.conf or conf.d wins over the Config table, and a
person editing a file writes yes or off rather than 1 or 0. The C++ daemons
now read those words (ConfigFileValue, 4d7609d36), but PHP and Perl took the
value as written, and a non-empty string is true in both. So
ZM_STRICT_VIDEO_CONFIG=off in conf.d left the option on in the web UI and the
Perl scripts while zmc turned it off.
Read a file value for a boolean option the same way in all three: yes, true
and on are 1; no, false and off are 0; anything else stays as written. Only
booleans are read this way, so a string option such as a title keeps "Yes".
PHP defines the file values as constants before it reads the Config table,
since it needs the database settings to connect, and only the table says
which options are booleans. So a word now waits until the table has been
read, apart from ZM_DB_* settings, whose words database.php reads itself.
Perl reads the type with each row.
Tested on 1.38's zm138 database with a conf.d file setting
ZM_OPT_FAST_DELETE=yes (0 in the table), ZM_STRICT_VIDEO_CONFIG=off (1)
and ZM_WEB_TITLE=Yes. The installed PHP and Perl gave 'yes', 'off' and
'Yes'; these give '1', '0' and 'Yes'. tests/php/test_config_file_booleans.php
checks the rule.
refs #5289
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Event::Close() renamed the event to <prefix><Id>(r) every time. That
was fine while only zmeventtool.pl close and zmaudit called it, by hand
or a day later; with zmwatch closing events left open by a zmc that
died, it would overwrite a name the user set while the event recorded.
Rename only when the name is still the placeholder 'New Event', as
zmc's own close does (~Event: Name = IF(Name='New Event', ...)). The
'Recovered.' note still marks the event.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reolink_HTTP.pm and Uniview.pm said `use JSON`, and
ZoneMinder::General::jsonLoad required JSON. The Debian and Ubuntu
packages depend on libjson-maybexs-perl, not libjson-perl, so on a
packaged install neither control module loads ("Can't locate JSON.pm in
@INC") and jsonLoad, which reads MacVendors.json for the network scan,
logs that error and returns nothing.
Use JSON::MaybeXS as the other control modules (AMLink, Dahua_RPC,
IPSpeaker) and the rest of ZoneMinder::General already do. encode_json
and decode_json behave the same; JSON::MaybeXS picks Cpanel::JSON::XS,
JSON::XS or the core JSON::PP.
tests/perl/test_json_maybexs.pl hides JSON.pm from @INC and loads both
modules and runs jsonLoad.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Event::Close() (zmeventtool.pl close) took the end of an event only from
its Frames rows and refused events that had none. zmc writes Frames rows
in batches and updates the event row's Length, Frames and scores with
each batch, so an event whose zmc was killed in its first seconds has no
Frames rows, and either side can be ahead of the other.
Take Length, Frames, AlarmFrames, TotScore and MaxScore as the larger of
the event row and the Frames rows; EndDateTime follows the Length that
was used. The name and Notes are written as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The skin and css, the language and locale, the date, time and datetime
formats, the default bandwidth and the snapshots feature switch all
decide how the interface looks or what it offers, which is what that tab
is for, but they sat under System among the shutdown command, the shm
key and the audit intervals. System drops from 42 entries to 33 and
Display goes from 32 to 41.
The snapshots switch is read only by the skin: it gates a button in
montage and montage review, a menu item, and the snapshot views and
their permission checkboxes. Nothing in the daemons or the perl scripts
looks at it.
The category key is still 'web'. That is the tab now labelled Display;
the tab whose key is 'display' holds the per-session cookie settings and
stores no configuration at all, so a stored default does not belong
there. The hint setup for the moved entries -- the installed skin list,
the css list for the current skin, the bandwidth options, and the
language and locale lists -- moves to the same branch.
ZM_TIMEZONE stays under System. It reads like a presentation setting,
but Filter.pm uses it for date arithmetic when evaluating filters, so it
changes behaviour and not just what is displayed.
A migration, 1.39.40, is needed as well as the ConfigData change, because a
version upgrade does not rewrite Config: zmupdate only reloads it from
ConfigData under --freshen, which is mutually exclusive with an upgrade
run. Fresh installs take the new category from the generated
zm_create.sql.
Verified both paths converge on 33 system and 41 display entries with
identical membership: a fresh install of this branch, and a copy of a
real pre-change Config table with the migration applied. On the running
install the bandwidth, language and locale entries still render as their
pickers on the new tab, css as its list of choices, and the timezone
picker is still on System with its full list.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Controls.Type looks like the monitor type a control applies to, and has
been kept in step with the Monitors enum since 1.27, but nothing has
ever read it. The control dropdown on the monitor page lists every row
by Name, no query filters on it, and no code compares it to the
monitor's own Type. It was a field the edit form asked for and the
control list displayed back.
The column appears to be MonitorPresets.Type copied by analogy, where
the idea is real: that one decides what a preset creates. That also
explains why every migration altering one altered the other.
Removes the column from the schema, adds the 1.39.39 migration, the edit form row
and its type list, the control list column, the defaults in Control.php
and Control.pm, and the value from all 53 shipped rows in controls.sql.
This supersedes the 1.39.38 retyping of three libvlc controls to Ffmpeg:
the column they were being retyped into is gone.
zmcamtool needs no change -- its import regex rewrites the Id and
captures the Name, so it is not positional past that, and its export is
a mysqldump that follows the live schema.
Dropping Type left the two shipped Pelco rows byte-identical to their
twins, which existed only as a Local and an Ffmpeg copy of the same
control, so the redundant copies are removed: 53 shipped rows become
51, with both Pelco-D and Pelco-P still present once.
Existing installs are deliberately left with their duplicates. The same
pair collapses there too, but deleting rows a monitor's ControlId may
point at, to tidy a repeated name in a dropdown, is not a trade a
migration should make silently.
Verified by loading the old and rewritten controls.sql into scratch
databases and diffing every remaining column across all 53 rows: no
value shifted, which is the failure mode when a positional INSERT loses
a field. The file mixes positional and explicit-column INSERTs, so both
forms were handled. Fresh-install and migrated Controls schemas compare
equal, the shipped data loads into both, and Monitors and MonitorPresets
keep their Type. Build clean, 147 test cases pass, php -l and perl -c
clean on the edited files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Follows the daemons: no monitor can be type Libvlc any more, so the ui
should not offer it or carry text describing it.
Drops the source-type option and its deprecation notice, the Libvlc arm
of the Source Path and console source-column branches, the entry in the
control-capability type list, and the value from the api's Type
validation list. The OPTIONS_LIBVLC help block goes from all 25 language
files: monitor.php builds that key as OPTIONS_ plus the monitor type, so
it became unreachable the moment the type did. The Libvlc source-type
label goes from the 22 files that carried it.
SourceTypeDeprecated and Deprecated lose their last caller and are
removed. RemoteRtspDeprecated and Removed stay -- the rtsp protocol
option is still shown to monitors already set to it.
The WEB_FILTER_SOURCE help listed the affected source types; corrected
in ConfigData.pm.in and in the one translation that had localised it.
All 25 language files and the four edited php files parse clean under
php -l, and the config module parses under perl -c. No javascript
changed, so eslint has nothing to say here -- it cannot run in this
worktree anyway, which has no node_modules.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
RTSPDescribe chose whether to take the media url from the DESCRIBE
response rather than the configured path. That was a quirk of
ZoneMinder's own rtsp implementation; ffmpeg decides it itself. Removing
that client left the setting with nothing behind it -- the monitor form
still drew the checkbox and the save path still wrote the column, but no
code read it, so the only thing it could do was mislead whoever ticked
it.
Drops the form row and the Remote-only block that existed to hold it,
the javascript that showed and hid it as the protocol changed, the field
in the php and perl monitor models, the default in the save action, the
column from the schema, and the label and help text from all the
language files carrying them.
The 1.39.37 migration drops the column behind an information_schema
guard scoped to DATABASE(), so the file stays re-runnable.
Verified by building a pre-upgrade database from upstream/master's own
schema and applying the migration: the column goes, and Monitors matches a fresh install of this
branch column for column. Migration applied three times with no error.
php -l clean on the four edited files and all 28 language files, perl -c
clean, build clean, 154 test cases pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
FOSCAMR2C presetGoto and presetSet interpolated the preset into
"SELECT Label FROM ControlPresets WHERE Preset = $preset" and sent the selected
Label to the camera as name=. For presetGoto the preset came straight from the
web request, so any user who could see the monitor could run a SELECT against
the ZoneMinder database and have the value (the auth hash secret, password
hashes) delivered to the camera link, or use it as a boolean oracle.
- FOSCAMR2C: bind the preset in every statement, refuse a preset that is not a
number, and look ControlPresets up by MonitorId as Dahua and PSIA do.
- buildControlCommand(): refuse a non-numeric presetGoto preset, and a control
name that is not a plain method name. zmcontrol calls the name as a method and
the options are split back out of the command string on spaces.
- zmcontrol.pl: refuse a command that is not a plain method name, so a qualified
Package::sub name can't call a sub outside the control module.
- canView()/canEdit() with a monitor id returned only the monitor check and
ignored the area, so canView('Control', $mid) passed for anyone who could see
the monitor. Require the area permission as well; Monitors stays decided by
visibleMonitor()/editableMonitor().
tests/php/test_control_command.php covers buildControlCommand() and the
monitor-scoped canView()/canEdit(); tests/perl/test_foscamr2c_preset_sql.pl
checks the statements FOSCAMR2C prepares and what it sends to the camera.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
go2rtc serves its API under /api, and its websocket at /api/ws. zmc
appends /streams to ZM_GO2RTC_PATH and the browser appends /ws, so the
setting only worked if it ended in /api. The natural value to enter is
the server address, http://host:1984, and with that zmc reached
/streams, which is go2rtc's RTSPtoWeb compatibility endpoint, so no
stream was ever registered, and the browser's /ws was a 404. Where
another player was available the failure went unnoticed.
Normalise the setting where URLs are built from it: a trailing slash is
dropped and /api is added unless the path already ends in it, which also
covers go2rtc behind a reverse proxy at a sub-path.
- Go2RTCApiEndpoint() in zm_utils, used by zmc's Go2RTCManager.
- go2rtcApiUrl() in skin.js, used by MonitorStream and the console
thumbnail stream. It assigns the path once, since stripping the slash
from a bare host's "/" and appending gave "//api", as the old
pathname += '/ws' gave "//ws".
- The option's help says the suffix is optional.
Catch2 and node tests cover bare hosts, trailing slashes, an existing
/api, a proxy sub-path and credentials in the URL.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every query on Frames filters on EventId and orders or ranges by FrameId,
and nothing needs a frame by Id. The Id primary key was a clustered index
nothing read, and EventId_FrameId_idx was a secondary index every query
used. On a copy of a local table that secondary index was half the
table's size. With (EventId, FrameId) as the primary key the index is
gone, and an event's rows are stored together, so deleting an event is a
range delete.
zm_update-1.39.36.sql:
- converts AI_Detections.FrameId from Frames.Id to the per-event frame
number, drops its foreign key to Frames and indexes (EventId, FrameId).
A composite foreign key cannot replace it: ON DELETE SET NULL would
have to null the NOT NULL EventId, and Frames rows are written in
batches, so a detection can be recorded before its frame row.
- removes duplicate (EventId, FrameId) rows, keeping the earliest.
- rebuilds Frames with the new primary key.
- removes ON UPDATE CURRENT_TIMESTAMP from Frames.TimeStamp. Any UPDATE
of a frame row was overwriting its capture time.
Each step checks the current schema first, so the migration can be
re-run.
REST API: view, edit and delete take /frames/<action>/<EventId>/<FrameId>.json.
The old single-Id URLs return 404. CakePHP 2 has no composite keys, so
the model's primaryKey is EventId. That keeps Event's dependent cascade
delete limited to the event's own frames. The controller writes with
explicit (EventId, FrameId) conditions instead of save(), which would
match rows on EventId alone. Edit no longer changes EventId or FrameId.
view=image with fid but no eid used to look up Frames.Id. It now returns
404.
The Perl Frame class is identified by (EventId, FrameId).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZM_AUTH_HASH_SECRET signs the JWT access and refresh tokens, and its
default is a fixed string in the public source. Nothing generated a
per-install value and tokens signed with the default verified normally,
so on an install with auth on and the secret untouched anyone could sign
an admin token and be accepted by the web UI, the API and zms.
- ZoneMinder::Config::saveConfigToDB() now replaces an empty or default
secret with 32 random bytes from /dev/urandom, hex encoded. Package
installs and upgrades run zmupdate.pl -f, which saves the config, so
existing installs get a secret on upgrade. A secret the admin set is
left alone.
- validateToken() in PHP and zmLoadTokenUser() in C++ refuse to verify
tokens while the secret is empty or the default, and the API refuses to
issue them, as it already did for an empty secret.
- zmLoadTokenUser() no longer writes the signing key to the debug log.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GenerateVideo used DefaultVideo as the ffmpeg input relative to the event
directory and built the output file from Name with only whitespace
replaced, so an Events=Edit user could make zmvideo read or write any
path the account can reach. zmfilter's generateImage passed
event_path/DefaultVideo to ffmpeg -i, and the %EV% email tag attached
event_path/DefaultVideo, which could mail out any readable file.
Strip everything up to the last path separator from DefaultVideo in all
three places, and reduce Name to [-A-Za-z0-9_.] with no leading dot
before using it as the video filename.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 5904023f13448e557dc205edb9440972e4a6482d)
With ZM_AUTH_HASH_IPS on, the client address bound into the auth hash was
taken from the left-most X-Forwarded-For value whenever the header was
present, both when PHP generated the hash (getRemoteAddr()) and when PHP or
zms validated it (getAuthUser(), zmLoadAuthUser()). The header is client
controlled, so anyone holding a leaked hash could replay it from anywhere by
sending the address it was bound to.
Add ZM_AUTH_TRUSTED_PROXIES, a list of exact reverse proxy addresses.
X-Forwarded-For is now used only when REMOTE_ADDR is one of them, and is read
from the right, skipping hops that are themselves listed proxies, so values a
client prepends are never chosen. With the option empty, the default, the
header is ignored and REMOTE_ADDR is used.
PHP (web/includes/Network.php getRemoteAddr()) and C++ (ClientAddress() in
zm_utils, used by zmLoadAuthUser()) implement the same rule so generation and
validation continue to agree. Every PHP caller already routes through
getRemoteAddr(), so session.php and auth.php need no change.
Reverse proxy users who enable ZM_AUTH_HASH_IPS must list their proxy in the
new option; until they do, hashes bind to the proxy address, which still
validates but no longer distinguishes clients. This is the behaviour change
that the #4921 work avoided by trusting the header.
The option is added through ConfigData only, like other recent options;
zmupdate.pl --freshen inserts it, zms falls back to the compiled-in default and
PHP treats an undefined constant as empty, so no schema migration is needed.
Tests: ClientAddress Catch2 case; tests/php/test_remote_addr.php updated for
the trusted-proxy rule.
refs GHSA-72rf-54rm-798c
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit acea5889dec0826f595cb736147a5fdc9c94a2a9)
zmfilter.pl rebuilds SQL from the JSON the web tier stores verbatim in
Filters.Query_json, so a saved filter's attribute, operator and value all
arrive at the statement exactly as they were written. None of the three were
checked here, while web/includes/FilterTerm.php checks all three: values were
wrapped in quotes by hand, which a value containing one closes; the final
attribute branch concatenated 'E.'.$attr; and the generic operator branch
emitted the operator as written.
Any account that can save a filter could therefore have the daemon run SQL of
their choosing with its own credentials, against a table set that includes
Users. The daemon also deletes and moves events, so this is not only a read.
Confirmed against the module before the change, building SQL from stored terms
and checking whether the payload landed outside a string literal:
val = "x' OR 1=1 -- " -> outside, injected
attr = "Id=1 OR 1=1 -- " -> outside, injected
op = "= 1 OR 1=1 -- " -> outside, injected
Attributes now have to match what the PHP accepts, operators have to be one of
the sixteen the UI can produce, and a filter carrying anything else is refused
rather than run. Values go through DBI's quote().
Numbers are left unquoted so the statements the existing filters produce are
unchanged: a bare number cannot carry SQL, and quoting it would have altered
every numeric comparison in the file for no gain. Verified by generating the
SQL for seven representative filters before and after -- purge-when-full,
delete-after-a-day, LIKE on a name, IN on monitors, a monitor name, a score
range and an IS NULL -- and diffing: identical apart from the timestamp a
relative date resolves to.
After the change the three payloads above are contained in a literal, or the
filter is refused.
See GHSA-p8h3-4x5c-cv7p.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBHBB95RBX7D9p8ge2WDZb
(cherry picked from commit bc11642fea844bc86bac0080da42e4514d5a4d76)
Event::GenerateVideo built an ffmpeg command line as a string and ran it
through qx(), and two of the values in it are event fields an operator with
Events=Edit can set through the API. DefaultVideo was interpolated with no
quoting at all. The name behind the output filename only has its whitespace
replaced, so a single quote in it closed the quoting that was there. Either one
gave arbitrary command execution as the account the daemons run as.
Both confirmed against the module before the change, driving GenerateVideo
directly:
DefaultVideo = "x.mp4; touch <marker>; echo" -> marker created
Name = "evt';touch${IFS}<marker>;echo'" -> marker created
The second needs ${IFS} rather than spaces because the name has whitespace
substituted before it is used, which is the whole of the sanitising that was
being relied on.
The command is now a list passed to exec, so there is no shell to escape from
whatever those fields hold; ffmpeg's own output still lands in the event's
ffmpeg.log. The two configured option strings are admin-set and hold several
options each, so they are split on whitespace to become separate arguments;
shell quoting inside them is no longer honoured.
After the change both payloads run ffmpeg with the injection as a literal
argument and create no marker.
See GHSA-pfph-4j9j-7cv7.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBHBB95RBX7D9p8ge2WDZb
(cherry picked from commit 5fc491728c679bc7a8ff4b51183983e7fe184200)
The long options --shell/--command are util-linux only. OpenBSD su takes
-s for the shell and passes arguments after the login name to it, and
util-linux su accepts the same form.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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
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
The audio_fifo_path field was retired alongside video_fifo_path when the
media FIFOs were replaced by the stream socket. video_fifo_path was
repurposed as stream_socket_path, but a single socket carries both
streams so there is no separate audio path to publish; reserved_path2
stays reserved.
Spell out the convention where the field is defined, so a future reader
does not remove or reorder it (which would shift every later field for
out-of-tree shm readers) and knows the slot is free to reuse at the same
64-byte width. Align the PHP Monitor object and ZoneMinder::Memory notes.
refs #5143
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4UcdJLt1bxwdpcigGxZRD
Address monitor-side stream socket review findings:
- zmc starts the stream socket before the first connect attempt, and
SendStreamHealthEvent records the health state even when the socket is
not up yet, so connection/prime faults during startup are observable to
a consumer instead of being lost until the first successful prime.
- PrimeCapture announces audio whenever the camera has a decodable audio
stream (Capture forwards audio packets unconditionally), and clears a
previously announced stream when a re-prime no longer sees it.
- Publish the media stream socket path in the monitor shared-memory
block (reusing the retired video_fifo_path field as stream_socket_path,
same offset and size) so consumers discover it without hard-coding the
convention or reading the producer's zm.conf; the PHP Monitor object
and the ZoneMinder::Memory Perl module expose the renamed field.
- Move the wall-clock microseconds helper out of zm_monitor.cpp into
zm_time.h as SystemClockMicros(), where time helpers belong.
refs #5143
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4UcdJLt1bxwdpcigGxZRD
Rename video_fifo_path/audio_fifo_path to reserved_path1/2, keeping the
struct layout and 864-byte size unchanged so existing shared memory
mappers (and out-of-tree shm readers) keep working through the
transition. The fields are zeroed at startup as before, scrubbing any
stale path strings left by a pre-upgrade zmc.
The stream socket deliberately publishes nothing via shared memory: its
path is the documented convention PATH_SOCKS/stream_{monitor_id}.sock,
derivable from static config, and codec parameters travel in the HELLO
handshake instead of shm fields.
Perl (Memory.pm) and PHP shared memory maps updated to match by name;
offsets are unchanged.
Tests: full suite passes via ctest; php -l clean on Monitor.php.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
scripts/ZoneMinder/lib/ZoneMinder/Control/ held both ONVIF.pm and onvif.pm, the
only pair of paths in the tree differing solely in case. On a case insensitive
filesystem git can materialise just one of them, so a macOS clone reports the
loser as modified forever and git add -A commits one module's contents over the
other.
The runtime consequence is worse than the checkout noise. Every seeded Controls
row uses Protocol='ONVIF', and zmcontrol.pl builds the module name from that
column. Where onvif.pm is the file that survived, require ZoneMinder::Control::ONVIF
still succeeds because the filename matches, but it defines the lowercase package,
so the bless lands in an empty ::ONVIF and the first method call dies.
ONVIF.pm has replaced onvif.pm since the unified module landed. Of the subs only
onvif.pm defines, all but horizontalPatrol and horizontalPatrolStop exist there
under underscore-prefixed names, and every ONVIF-protocol Controls row has
CanAutoScan=0, so nothing can reach those two.
Nothing ever moved existing installs onto the new protocol name, so add
zm_update-1.39.29.sql to do it. zm_update-1.35.23.sql sent Protocol='onvif' to
FoscamCGI, which was right in 2021 when onvif.pm held the Foscam CGI protocol and
was copied to FoscamCGI.pm, but onvif.pm was afterwards replaced with a real ONVIF
implementation and the seed row restored, so rows written since belong on ONVIF.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The session leak is fixed - no "too many connections" since the restart - but
decode failures continue, so there is a second cause. The old message named
only the symptom, which is why the first diagnosis chased the wrong thing.
The failure now reports the payload length, whether that length is a multiple
of four as base64 requires, whether the reply decodes without unmasking, and
the leading bytes. Those separate the plausible causes: a camera answering
unmasked, a truncated reply, and a reply masked with a key we no longer share.
If the reply does decode unmasked it is returned rather than discarded, since
an unmasked answer is still an answer.
What is already ruled out, so the next person does not repeat it: a stale
session still answers masked and decodes cleanly, just without a result field;
LWP's decoded_content is byte-identical to content here; and the masked payload
did not collide with the </cmd> terminator in 48 replies across six sessions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
The control daemon filled its log with
failed to decode the reply to CoaxialControlIO.control: malformed JSON
on every lightOn, while the light itself still worked. Three faults in this
module compounding, all mine.
Nothing ever logged out. close() was inherited from Dahua_RPC, which only sets
a flag, so every login abandoned a session. The camera counts connections and
reclaims them slowly, and zmcontrol keeps one object for the life of the daemon
while Dahua_RPC re-logins whenever the camera times the session out - the log
shows that happening every half hour. Each one leaked a slot until the camera
answered global.login with "too many connections!", which it is still doing
here hours later.
login() then made a transient failure permanent. It cleared session and
mask_key up front, so a login that failed left the object with no key. Several
Dahua_RPC callers re-login on error and carry on without checking the result,
so every later command was sent unmasked, came back masked, and failed to
base64-decode. The logged error therefore described the symptom two steps
downstream of the cause and never mentioned the session at all.
So:
- logout() gives the session back, and close() calls it
- login() releases the previous session before taking another
- rpc_call refuses to send on the masked channel with no key, and says the
session is gone rather than emitting an unreadable decode error
- a login refused for "too many connections" is reported as that
global.logout, with no parameters on the ordinary Request channel, confirmed
against the camera's own web bundle rather than guessed.
Tested: 5 new assertions that a Request with no key returns undef and puts
nothing on the wire, while Login, OutsideCmd and the key exchange - which
legitimately have no key yet - are still sent. Perl suite 15 files / 266
assertions.
Not yet verified against the camera: it is still refusing logins with "too many
connections" from the sessions already leaked, so the logout path could not be
exercised end to end. It needs a retry once the camera has reclaimed them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
Points the camera at an NTP server and sets how often it syncs. The cameras
were on time.windows.com once a day, or in one case at an address on the LAN
that does not answer NTP at all, so it was not syncing.
UpdatePeriod is in minutes and 1 is accepted - established by writing it and
reading it back, since the firmware has no getConfigCaps to ask. That is as
often as the camera will go, so it can be held within a minute of the server.
Only Address, Enable and UpdatePeriod are written. TimeZone and TimeZoneDesc
are deliberately left alone: the camera's clock already reads correctly and
the index is not a standard one, 26 for "Middletime" here against a factory
default of 25 "Easterntime". Changing a timezone we do not understand to fix a
sync interval would be a poor trade.
The write is a whole-section merge that echoes back every field the camera
returned, so a partial write cannot silently drop one, and set_time skips the
write entirely when the settings already match. The camera holds this in
flash and the method is meant to be safe to re-run - from cron, for instance -
so an unconditional write would spend erase cycles for nothing. Same reasoning
as FoscamHD::set_time.
Tested: 12 new assertions over the merge and the change detection, including
that untouched fields survive, that the caller's hash is not mutated, and that
1440 against "1440" does not read as a change - which would otherwise rewrite
flash on every run. Verified against both cameras: one syncing from
time.windows.com every 1440 minutes and one pointed at a LAN address that does
not answer NTP at all every 60, both now on the local server every 1, and
re-running was a no-op. Perl suite 15 files / 261 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
These cameras have a white light that ONVIF does not expose at all: imaging
offers only Brightness/ColorSaturation/Contrast/Sharpness, GetRelayOutputs is
empty with RelayOutputs="0", and there is no PTZ service and so no auxiliary
commands. The light lives behind a vendor JSON-RPC tunnel.
The vocabulary turns out to be Dahua's -- CoaxialControlIO for the light,
magicBox for reboot -- so this subclasses Dahua_RPC and replaces only open,
rpc_call and login. lightOn, lightOff, lightStatus and reboot are inherited
unchanged, and Dahua_RPC's lightStatus already parses the
params.status.WhiteLight these cameras return.
The transport is the whole of the work, and none of it is discoverable by
probing:
* The endpoint is /Onvif/device_service with a capital O. The lowercase
/onvif/device_service is the real ONVIF SOAP service; the capitalised
spelling is a separate vendor tunnel sharing the path. /RPC2, /RPC3,
/OutsideCmd and every /cgi-bin/* return 404 on this model, which is why
the API looked absent.
* Requests are <body><cmdType>T</cmdType><cmd>P</cmd></body>. Without
cmdType the camera drops the connection rather than returning an error.
* P is base64 of the JSON-RPC object, XOR-masked with a session key once one
exists. Masking is mandatory: an unmasked Request is refused even on a
freshly authenticated session, so the key exchange is not optional.
* That key needs a three step bootstrap -- log in, fetch the device RSA
public key over the unauthenticated OutsideCmd channel, then send an AES
key encrypted under it and decrypt the mask key from the reply. The salt
doubles as the AES key and must be 32 decimal digits; 16 is accepted by
the AES step and then rejected by getGeneralKey.
IO polarity matches Dahua_RPC: IO 1 is on, IO 2 is off. Worth stating because
the camera's own web UI is a toggle that does not reveal which is which, and
because a floodlight cannot be verified optically in daylight -- an RTSP
brightness comparison appeared to say the opposite and was exposure drift.
CoaxialControlIO.getStatus is the reliable witness.
Tested: 26 assertions over the pure transport helpers -- mask symmetry and
cycling, NUL bytes surviving the mask, the envelope, stripping the CRLF the
device pads its replies with, and the channel routing, which fails silently on
the wire if wrong. Verified live against both units: open negotiates a 32 byte
mask key and lightStatus reports Off -> On -> Off around lightOn/lightOff. Perl
suite 15 files / 249 assertions pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
ZoneMinder has carried audio for years without ever listening to it: the
packets go into the event and nothing reads them. AudioDetector decodes
them in the capture thread and reports a 0-100 level, which Analyse()
turns into score alongside motion, ONVIF and Amcrest.
The level is dBFS-derived, not a raw amplitude ratio. Linear RMS is
unusable as a setting: ordinary speech sits at 1-3% of full scale, so
every sound worth catching would be crammed into the bottom two points of
the range and no operator could tune it. Mapping -60..0 dBFS onto 0..100
puts speech around 43 instead.
Three columns on Monitors: AudioDetection to enable it, AudioThreshold
for the level to alarm at, and AudioAlarmScore for what it contributes.
AudioAlarmScore defaults to 9, matching what an ONVIF or Amcrest alarm
already adds. A threshold of 0 means off, so enabling detection without
choosing a threshold cannot alarm on silence -- a plain level >= threshold
test would alarm on every packet in that state.
The level and the alarm flag go in SharedData's two spare bytes, renamed
from reserved1/reserved2. Offsets and sizes are unchanged, so the
888-byte cross-process layout and the Memory.pm and Monitor.php offset
tables all still agree; the readers are renamed in the same commit so the
level is available to the web UI.
The decoder is opened lazily on the first audio packet rather than at
camera setup, so a monitor with detection off never carries one and a
stream that gains audio on reconnect still gets picked up. Scoring reads
the capture thread's most recent level rather than scoring per audio
packet, because the score belongs to a video frame and audio packets do
not arrive in step with them.
Tested: 242 assertions over the pure helpers - the RMS of both sample
formats, the dB scale's monotonicity and endpoints, full-scale clamping
of decoder overshoot, and the threshold-0 case. The repo's Catch2 harness
needs v3 and only v2 is installed here, so tests/zm_audio_detector.cpp
was compiled against a v2 shim to check it, and an identical set of
assertions was run standalone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
An IP speaker is controllable but has none of the capabilities the Controls
table models: nothing moves, focuses or lights up. It plays a sound file it
already holds, selected by a numeric id, and carries an output volume.
Add CanAudioPlay, MinAudioFile, MaxAudioFile and CanAudioVolume to Controls,
with migration zm_update-1.39.21.sql. CanAudioPlay renders one button per
sound id plus a stop button; MinAudioFile/MaxAudioFile bound that range;
CanAudioVolume renders the volume pair. The sound id travels in the command
name (audioPlay12), the way presets already do, because a control button has
no way to attach a separate parameter.
Add ZoneMinder::Control::IPSpeaker driving the device's own HTTP interface,
and a Controls entry for it. Playback goes over the vendor interface rather
than ONVIF because the ONVIF audio output service on this firmware only
describes the output and offers no way to start a stored file.
Measured against a device reporting ONVIF Manufacturer "IPSpeaker" and
firmware CS20-V3.3.45N:
- File ids fall in two windows, 10-14 built-in and 20-30 operator uploads.
Anything else is refused with result -2, and an id in a window with no file
uploaded with result -3, so valid_fileid refuses only the former: an empty
slot is a legitimate id the operator may fill later.
- config=audio.set replaces the whole audio section, so a partial write
reverts the microphone, codec list and echo-cancellation settings. Every
volume change reads the section and echoes it back with the new level.
audio.get also returns outmute, which audio.set rejects, so it is excluded
from the field list.
- The volume reads back and round-trips, but the device's RTSP audio is a
pre-volume tap of the playback signal: it is unchanged with outvolume at 0,
so it cannot confirm the loudspeaker's acoustic output. CanAudioVolume is
set on the strength of the setting persisting, and both the POD and the
seed row say the acoustic effect is unconfirmed.
The seed row covers the built-in window only, because a fresh device has
nothing uploaded and the firmware refuses an empty slot.
Adding columns to Controls extends the 43 positional INSERTs in
db/controls.sql, which carry no column list and so must match the column
count exactly.
Tests: scripts/ZoneMinder/t/ip_speaker.t, 32 assertions covering file id
windows, volume clamping and stepping, request building and the whole-section
write. All pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
(cherry picked from commit d07e552a3c58373f7f0ccebf9aa28adb4d51baa3)
The original fix replaced the hardcoded 000 token in getCamParams and
_setImaging, but get_config/set_config landed upstream afterwards and
carry three more imaging requests that still address VideoSource 000:
the ImagingSettings and ImagingOptions entries in %config_types, and the
SetImagingSettings body in set_config.
Cameras that number their sources differently reject all three. On an
AMLINK AL5M-T5171EW the video source is 00000 and a request for 000 comes
back as "The requested VideoSource does not exist.", so the imaging half
of the config API is unusable on those cameras even with the earlier fix
applied.
The query bodies now carry a __VIDEO_SOURCE_TOKEN__ placeholder, matching
how __PROFILE_TOKEN__ already works, and get_config substitutes it only
when the body contains it so unrelated categories do not pay for a
GetVideoSources round trip. set_config calls _video_source_token()
directly, which is cached after the first lookup.
The added tests assert on the module source because %config_types is a
file-scoped lexical. That is deliberate: the failure this guards against
is a new imaging call being written with a literal token again, which is
exactly how get_config/set_config reintroduced the bug.
Verified against both live AMLINK units: GetVideoSources returns 00000 on
each, and GetImagingSettings answers for 00000 while faulting for 000.
Perl suite 13 files / 191 assertions pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
getCamParams() and _setImaging() addressed the imaging service with a
literal VideoSourceToken of 000. That token is a different namespace from
the media ProfileToken carried in ControlDevice, so it could not be
configured around: on an AMLINK AL5M-T5171EW the media profiles are
MediaProfile00000 while the video source is 00000, and the camera answers
a request for 000 with "The requested VideoSource does not exist."
Brightness (Iris) and Contrast (White) control were unusable on any camera
that does not happen to number its first video source 000.
Read the token from GetVideoSources on first use and cache it. The
fallback to 000 is applied per call rather than cached, so a camera that
is unreachable when the control daemon starts gets another chance instead
of being pinned to the wrong token for the life of the daemon.
Parsing is split out as video_source_token_from_xml() so it can be tested
without a camera, matching the pattern used by the HikVision light
helpers.
Verified against a live AMLINK AL5M-T5171EW: token discovered as 00000,
irisAbsOpen(step 10) moved Brightness 50 -> 60, restored to 50.
(cherry picked from commit 7f1935d69d58b3e2bbf71a54fe11b1a6b4bc30af)
Object::save() returns the error string on failure and '' on success, so
`if ($manufacturer->save())` took the success branch only when the save had
failed. ManufacturerId and ModelId were therefore assigned from an unsaved
object and left unset whenever the save actually worked.
Invert the test, capture the error string, and log it. Model.pm needs
ZoneMinder::Logger imported for Error().
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
