NVSocket is removed in this release and Libvlc is deprecated. Both
reach cameras ffmpeg reaches, so existing monitors are converted rather
than left to fail at capture time. The migration is 1.39.38.
Libvlc only changes type; Path already holds the full url, and User
and Pass are merged by FfmpegCamera the same way.
NVSocket has no url to convert -- it spoke a bespoke socket protocol to
one vendor's nvr -- so those monitors are disabled rather than left with
a type the daemon no longer recognises, which is Fatal and would take
zmc down rather than skip the monitor. The rows are kept so their zones,
groups and history survive.
The value is then dropped from the Type enum in all three tables that
carry it, and from zm_create.sql.in so a fresh install matches.
Verified against a scratch database built with the live schema, covering
Libvlc, NVSocket, and existing Ffmpeg and Local monitors. Re-running the file is a no-op.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Both reach cameras that ffmpeg reaches. Libvlc does it through a second
streaming library with its own build dependency, threading and bugs; NVSocket
speaks a bespoke socket protocol for one vendor's nvr that we have no way to
test. Neither earns the maintenance, and the same monitors work as Type Ffmpeg
with the camera's rtsp or http url.
Deprecated, not removed. A monitor using either still captures; it logs a
warning at startup naming what to change it to, and the form marks the type and
explains. That follows what Remote/RTSP did before it was removed, which is the
shape that gives people a release to move in.
The warnings were verified firing rather than assumed: switching a monitor to
each type in turn produces the expected message naming that monitor.
php -l and eslint clean; the three pre-existing duplicate lang keys are
unchanged. Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
FfmpegCamera opened a decoder for the audio stream, stored it, exposed it
through Camera::getAudioCodecContext() and Monitor::GetAudioCodecContext(), and
freed it in Close(). Nothing decoded a single frame with it.
Both things that decode audio build their own from the stream's codecpar:
VideoStore::open() for transcoding to aac, and AudioDetector::Open() for
levels. The previous commit removed the last place the camera's context was
even passed, and it had been ignored there too.
So there is no audio decoder to move to the monitor, only one to stop opening.
The camera keeps finding the audio stream -- that is discovery, and packet
routing and timestamps need it -- and the secondary-input branch still adopts
mAudioStream from FFmpeg_Input, just not the context that came with it, which
FFmpeg_Input owns and uses itself.
Camera now holds no codec context of any kind. Between this and the video move,
a camera finds streams and produces packets, and whoever needs pixels or
samples opens their own decoder.
Verified end to end rather than by reading, on a camera that really does carry
audio: with RecordAudio on and the encoder transcoding, the recorded file has
both an hevc video track and an aac audio track over 848 frames. Suite 147
cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
VideoStore took an AVCodecContext for the input audio and ignored it: the
member was initialised to nullptr and open() builds its own from
audio_in_stream's codecpar, with its own codec lookup, time_base and
avcodec_open2, and frees it in the destructor.
So the parameter described a relationship that did not exist, and following it
backwards -- through Monitor::GetAudioCodecContext() to the camera -- suggested
the camera's audio decoder was feeding recording. It was not.
Drop the parameter and say beside the member where the context actually comes
from.
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
The hand-written rtsp client is gone, so a monitor still typed Remote
with Protocol rtsp has nothing left to capture with. Ffmpeg reaches every
camera that client did, so 1.39.37 converts those monitors rather than
leaving them to fail at capture time.
The monitor gains an equivalent url built from Host, Port and Path.
Credentials are deliberately left in the User and Pass columns instead of
being written into the url: FfmpegCamera merges them into the path itself
and percent-encodes them through the Url class, and only when the path
does not already carry credentials. Building the url here instead would
break any password containing @ : / or #. A Host that already embeds
user:pass@ keeps working too, because the resulting path then carries
credentials and the runtime merge leaves it alone.
Remote/http monitors are untouched. The statement was verified against a
scratch database covering rtsp with and without a port, relative, empty
and NULL paths, a Host with embedded credentials, and Remote/http.
Re-running the file is a no-op.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the rtsp client gone, Type=Remote with Protocol=rtsp cannot capture, so
the monitor form no longer offers it and the camera probes no longer suggest
it.
A monitor already saved as rtsp still sees the option, because dropping it
outright would mean opening such a monitor to read the warning silently
re-saved it as http -- swapping one broken configuration for another. For those
the option is labelled removed and the warning now says the monitor will not
capture until it is changed, rather than that it will stop working in a future
release.
The three probes that suggested Remote/rtsp -- ACTI in both probe
implementations, and Vivotek -- now suggest Type Ffmpeg with the equivalent
rtsp:// Source Path, which is the same stream by a maintained route.
Type=Remote keeps http. mjpeg and snapshot-jpeg cameras are what it is for.
php -l clean on all four files; eslint clean; the three pre-existing duplicate
lang keys are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
rtsp_describe was passed to RemoteCameraRtsp and nowhere else, so with that
camera gone it was loaded from the database on every monitor read and then
ignored.
The SELECT column and its positional read come out together. They have to: the
row is walked by index, so removing one without the other silently shifts every
field after it rather than failing to build.
The Monitors.RTSPDescribe column stays. Dropping it is a migration and a web
change, which is a separate piece of work from no longer reading it.
Verified against the live database rather than by inspection, since a
misalignment here compiles perfectly: the monitor loads its name, 2560x1440
geometry, hevc codec and vaapi setting correctly, and captured 668 frames with
no errors.
Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
RemoteCameraRtsp was the last camera that opened one, so Camera's
mVideoCodecContext and getVideoCodecContext() are now written by nobody and
read only by the fallback that existed for it.
Drop both. Monitor::GetVideoCodecContext() returns the monitor's context and
nothing else, OpenDecoder() no longer has to ask whether the camera brought its
own, and FfmpegCamera::Close() stops freeing a pointer it cannot have set. The
audio decoder stays with the camera; only video moved.
This is what the previous commits were working towards: one owner for the
decoder, with the hw device reference and the hw pix fmt storage beside it, and
no path where a camera can free a context the decoder thread is using.
Verified on hardware after the removal: vaapi negotiated, 646 frames into a
recorded event, no errors. Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
ZoneMinder carried two rtsp implementations: ffmpeg, which every Type=Ffmpeg
monitor uses, and a hand-written one behind Type=Remote with Protocol=rtsp --
its own rtsp state machine, rtp transport, rtcp control and sdp parser, some
3200 lines. ffmpeg handles rtsp well enough now that a second implementation
earns nothing, and it was already marked deprecated in the interface.
A monitor still configured that way now logs an error naming the exact Source
Path to move to, built by RtspUrlFromRemote with the credentials stripped, so
the message is actionable rather than only a refusal. That helper and its tests
stay; they describe the migration, which still needs describing.
Type=Remote keeps its http protocol. mjpeg streams and snapshot-jpeg cameras
are what that capture method is for and they are unaffected.
Removed: zm_remote_camera_rtsp, zm_rtsp, zm_rtp, zm_rtp_ctrl, zm_rtp_data,
zm_rtp_source and zm_sdp. zm_rtsp_auth stays -- it is included by
zm_remote_camera.h and so is shared with the http camera. Everything named
zm_rtsp_server_* is the restreaming server and is unrelated.
Verified nothing outside the cluster referenced any of it: RtspThread,
RtpSource, RtpCtrlThread, RtpDataThread and SessionDescriptor had no callers
elsewhere in the tree, and RemoteCameraRtsp was named only by zm_monitor.
Build clean; suite 147 cases / 12531 assertions. An rtsp-over-ffmpeg monitor
still captures on hardware: vaapi negotiated, 646 frames into a recorded event,
no errors.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Pause() releases the decoder, so Play() has to open another. It did not, and
an on-demand monitor would have resumed with a null context: Play() re-primes
the camera and restarts the decoder thread without going back through
PrimeCapture(), which is where the only OpenDecoder() call was.
This was introduced by moving ownership to the monitor. Before that the camera
rebuilt its own context inside camera->PrimeCapture(), which Play() does call,
so the gap did not exist.
Neither monitor on the machine this was developed against is on-demand, so the
earlier hardware runs exercised the startup path only and could not have shown
it. Found by following who calls what rather than by testing.
Verified by running an on-demand monitor through a full cycle: hardware
decoding is negotiated twice, once at prime and again at resume after the
pause, with the decoder and analysis threads restarting in between and no
errors. Without the fix the second open is absent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The comment above PrimeCapture() still described the decoder thread holding a
context it got from camera->getVideoCodecContext(), which the camera would free
and rebuild. The monitor owns that context now, so the first half no longer
described what happens. The second half still does: the decoder thread has to
be joined so it releases the in-flight packet locks in decoder_queue, or stale
entries survive the reconnect and the packetqueue fills and stays full. Kept
that and corrected the rest.
RemoteCameraRtsp declared get_VideoStream, get_AudioStream,
get_VideoCodecContext and get_AudioCodecContext -- four accessors duplicating
the base class's under a different naming convention, none of them called from
anywhere in the tree. Removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
With the monitor opening the decoder, the camera's hwaccel_name,
hwaccel_device, hw_device_ctx and hw_pix_fmt are written and never read, and
the two hwaccel arguments to its constructor are passed only to be stored.
Remove them rather than leave fields that look like they still configure
something -- the monitor reads the same settings straight from its own members.
The re-arm comment in Close() described setting use_hwaccel back to true so a
transient hardware failure would not pin the monitor to software for the life
of the process. That flag is gone, but the behaviour is not: OpenDecoder()
builds a fresh decoder on every PrimeCapture(), so a reconnect retries hardware
decoding by construction. The reasoning moved there with it.
Close() keeps freeing mVideoCodecContext. It is a base-class member that an
Ffmpeg camera no longer sets, but RemoteCameraRtsp does, and the base is where
that belongs.
Verified on hardware again after the removal: vaapi still negotiates and 652
frames decoded into a recorded event with no errors. Suite 147 cases / 12531
assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The camera's job is to produce packets; decoding belongs with the thread that
does it. FfmpegCamera opened the video decoder and handed the context out, so
Monitor::Decode() ran against a pointer whose lifetime the camera controlled.
PrimeCapture() already carried a comment about the consequence: the decoder
thread had to be stopped and joined before re-priming, because
camera->PrimeCapture() frees that context and builds another, and a decoder
running against the dying one is unsafe.
Monitor::OpenDecoder() now opens it, after camera->PrimeCapture() has produced
the stream to decode from, and Monitor::CloseDecoder() releases it where the
decoder thread has already been joined -- at the top of PrimeCapture and in
Pause(). The monitor holds the hw device reference and the hw pix fmt storage
alongside it, which is what the context's opaque pointer refers to, so the
lifetime is now one object's business rather than split across two.
Two cases deliberately do nothing:
Cameras that hand over images rather than packets -- local, vlc, vnc -- have
no video stream, so there is nothing to open.
RemoteCameraRtsp opens its own decoder and decodes through it.
GetVideoCodecContext() falls back to the camera's for that one rather than
running a second decoder over the same stream. That capture method is
deprecated and goes in 1.41, and this branch cannot be tested here, so it is
left as it is rather than changed blind.
Verified on hardware: an rtsp hevc monitor with DecoderHWAccelName=vaapi still
negotiates through the monitor-owned path ("Trying hwdevice vaapi", "Using
vaapi hardware decoding for hevc") and decoded 752 frames into a recorded
event with no errors.
Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The mismatch Debug compared the monitor's configured size against the decoder
context's. Both hold the same numbers -- the context is filled from the
stream's codecpar a few lines earlier -- but the stream is the thing actually
describing what the camera sends, and reading it here means the check no longer
needs a decoder to have been opened.
That matters for the next step: this was the camera's last use of the decoder
context for anything other than handing it out.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Second piece of groundwork for letting the decoder thread own decoding. The
whole decoder setup -- codec selection, context allocation, thread count,
option defaults and overrides, hwaccel negotiation, avcodec_open2 and the
fallback -- sat as 184 lines inside FfmpegCamera::OpenFfmpeg(), reachable only
by a camera. Monitor cannot call it there, so the move would have meant
copying it.
Lift it to open_video_decoder() in zm_ffmpeg.cpp, leaving the camera a call
and the null check. No behaviour change: the body is the same code with camera
members replaced by parameters.
hw_pix_fmt and hw_device_ctx are taken by reference and keep their names, which
is not only convenience. The returned context stores &hw_pix_fmt in ctx->opaque
and get_hw_format dereferences it on every format negotiation, so the storage
has to outlive the context. A reference parameter says that at the signature
and makes a temporary impossible; the header says why.
Two things the extraction had to account for:
opts was declared once for the whole of OpenFfmpeg and shared with the input
open. It is freed unconditionally before the decoder block and nothing
repopulates it in between, so a fresh local is equivalent. The
AVDictionaryEntry it scanned with was shared the same way and is now local
too.
use_hwaccel was a camera member but only ever read at the gate inside this
block, cleared when a candidate's hwaccel failed so the next codec attempt
would not retry it. That is entirely internal to the open, so it is a local
now and the member is gone rather than left looking like it still gates
something.
Verified on hardware rather than by reading: an rtsp hevc monitor with
DecoderHWAccelName=vaapi still negotiates ("Decoder hevc supports type vaapi",
"Using vaapi hardware decoding for hevc", now logged from zm_ffmpeg.cpp) and
decoded 749 frames into a recorded event with no errors.
Build clean with no new warnings; suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Groundwork for moving decoding out of the camera and into the decoder thread.
The parsing of DecoderHWAccelName sat inline in FfmpegCamera::OpenFfmpeg(),
where nothing could test it, and the ai_server branch has a second copy of the
same logic in its Monitor-side decoder open. Moving the decoder without pulling
this out first would carry the duplication across rather than end it.
No behaviour change: same iteration over the types this build offers, same
comma-separated priority list, same warn-and-skip for a name libavutil does not
know, same order.
Being a free function it can now be tested, which it could not be before. The
cases covered are the ones an operator's setting actually runs into -- a single
name, a priority list, spaces around the commas, a typo among good entries,
empty entries -- asserted against whatever device types the host reports rather
than against a fixed list, so the test travels. This host offers seven, so no
section skipped for want of data.
Confirmed non-vacuous by mutation: dropping TrimSpaces fails one case, and
accepting an unknown name instead of skipping it fails two.
Build clean with no warnings; suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
With ZM_LOG_DEBUG off, Logger::initialise() clamped every target level
(file, database, syslog, terminal) to INFO as well as the overall level.
SIGUSR1 raises the overall level, but the effective level is the lower of
that and the highest target level, so it stayed at INFO and the signal
did nothing.
Only cap the overall level. Targets keep their configured levels, so
SIGUSR1 behaves as if ZM_LOG_DEBUG_LEVEL had been incremented: targets
configured for debug start receiving it, targets at INFO stay at INFO.
Startup behaviour is unchanged since the effective level is still INFO.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit af970628c3fe5fe76d163a30747cf534babbb8a6)
A blank line after the zoneminder package's Depends ended that stanza
early, so Recommends and Description formed a stanza of their own with
no Package field. dpkg rejects the whole file ("stanza lacks the
'Package' field"), so nothing could read its build depends either.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ubuntu2004 control file (the one the deb package CI builds from),
the redhat spec and the opensuse spec already build-depend on
nlohmann-json, so the packages we ship are built with
ZM_HAS_NLOHMANN_JSON and carry AI detection results. distros/debian and
distros/beowulf did not, so a package built from either silently lost
them: CMake treats the library as optional and only warns.
It is header-only and in Debian since buster, which Beowulf is based on,
so it costs nothing at runtime and needs no Depends entry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RemoteCameraHttp read the camera's Content-Length with atoi() and then asked
ReadData() for that many bytes, which passes the count straight to
Buffer::read_into(). Buffer::expand() worked in int: once the allocation passed
INT_MAX the spare space went negative, it grew again, and mAllocation + count
wrapped to a small value, so the following memcpy or read() wrote the camera's
data past a small heap block. Whoever answers the camera's HTTP request, often
on a plain-HTTP link, could crash zmc in a loop or corrupt its heap.
- RemoteCameraHttp::ParseContentLength() replaces the four atoi() calls. It
refuses a length that is not a number or is larger than twice the monitor's
raw frame size plus one network buffer (capped at 1 GiB), and GetResponse()
fails the response instead of buffering it.
- Buffer::expand() does its arithmetic in size_t, grows to exactly what is
needed, and refuses to hold more than INT_MAX bytes, since read_into()
returns the count as an int.
- Buffer::read_into() refuses a read that does not fit after expand() rather
than reading past the allocation.
tests/zm_buffer.cpp covers reads through a pipe, an oversized read, data kept
across a shift and a grow, and the Content-Length bounds.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
Filter::canView(), canEdit() and canDelete() ask the user object
$u->canEdit('System'), $u->canView('Events') and so on, but ZM\User had no such
methods. The calls fell through to ZM_Object::__call(), which stored the argument
as a property and returned it, so every check returned a truthy area name. Any
account with Events=View passed as a System editor and could save and run a
filter with AutoExecute, an OS command run by zmfilter.pl. The same fall-through
made the filter ownership and filterdebug checks pass for everyone.
Add canView(), canEdit() and canCreate() to ZM\User. They take the level from the
user's own field and from their role, the way the global functions in auth.php do
for the logged-in user, and refuse anything that is not a permission area.
Monitor::commandFieldChangesNotAllowed() now uses canEdit('System') so a role's
System=Edit counts there too.
In the filter action, require edit on a stored filter before applying the
submitted fields when saving over it, since the fields can name a new owner;
stop after a refused save instead of starting the filter in the background; and
require edit on the filter for start/stop/restart, which had no check.
tests/php/test_user_permissions.php drives the real ZM\User, ZM\User_Role and
ZM\Filter classes. The older filter tests stubbed the user with its own
canView/canEdit, which is why they passed while the real class had neither.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Annotate() clamped the label position to [0, width - text_width] in unsigned
arithmetic. A label wider (or taller) than the frame wrapped that bound to about
four billion, so the requested LabelX/LabelY was used unchanged and glyph rows were
written past the end of the frame buffer on every captured frame. The label text,
position and size are monitor settings any editor of that monitor can change.
Compute the bounds in signed arithmetic and floor them at zero. Draw a glyph only
when its whole cell fits in the row and a line only when it fits in the frame,
rather than checking after the glyph is already drawn, and skip foreground pixels
that fall outside the row (the shipped font reaches one column past the cell).
LabelSize indexed the font variants with size - 1 through at(), so 0 or anything
above 4 threw out_of_range in the capture thread. Clamp it to the valid sizes in
Annotate() and centreCoord().
Add tests that draw into a buffer followed by a guard region, covering
oversized, overlong, overtall and out-of-range label sizes in each pixel format.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Montage review wrote two values into single-quoted JavaScript strings
unencoded, inside the page's nonce-bearing script:
- minTime/maxTime, which can come from the date terms of a filter in
the request, so a crafted link ran script for the Events viewer who
opened it (GHSA-qm44-862r-fmq3). Stored cookies fed them the same way.
- Monitor.WebColour, which any editor of that monitor can set through
the API, so it ran script for whoever viewed montage review
(GHSA-8xqq-932v-9h3j).
Keep only range values that parse as a time, in canonical form, and
write minTime, maxTime and monitorColour with json_encode(). The canvas
style attribute took WebColour raw as well; escape it for HTML.
The same unencoded pattern ran through the other page-script templates,
where earlier fixes had covered named fields one at a time. Pass every
value echoed directly into a quoted string in web/skins/classic/views/js
through validJsStr(), 68 in all, including the timeline view's own
minTime/maxTime and request-derived values such as rate, scale, mode and
streamMode. Numeric and date values are unchanged by it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Removed z-index property from #frames img.
I'm not sure if this will have any other adverse effects, but I haven't found any.
When there are many tags in the dropdown list, the previews bleed through the list and make the tags unselectable where the previews are. This change allows the tags dropdown to display over the previews as they should.
zmc runs a monitor's EventStartCommand and EventEndCommand at the start
and end of each event. Both the classic save and bulk edit actions and
the API monitor add and edit authorized only on the monitor, so a user
with edit on one monitor and no System permission could set a command
for zmc to run as the ZoneMinder service user.
Changing either field now needs System=Edit, as Filter::canEdit()
already requires for AutoExecuteCmd. Monitor::commandFieldChangesNotAllowed()
reports the command fields a change would alter; submitting the current
value is not a change, so other users can still save the rest of the
monitor. The monitor form shows the fields read-only to them. Also
close the EventEndCommand list item.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Options page admits System=View and wrote every Config value into
its inputs, only disabling them for users who cannot edit. The Private
flag was never consulted, so a read-only administrator received
ZM_AUTH_HASH_SECRET, which is enough to sign a token for any user, and
the other Private credentials on their tabs.
For a Private option, show its value only to System editors; others see
that it is hidden. The note about a value overridden in zm.conf, which
printed the overriding value, is skipped for those options too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since b2861d5cf loadConfig() returns the Config table merged with the
zm.conf and conf.d values that have no Config row. Those carry no
Private flag, and skin.js.php writes every config entry not marked
Private into each page as a JavaScript const, for any logged-in user
whatever their permissions. ZM_DB_USER and ZM_DB_PASS were in every
page, the permission error page included. The API's Configs listing
redacts by the same flag.
Mark values that exist only in the config files Private, apart from
ZM_DIR_SOUNDS, the one the browser uses (for the alarm sound).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Event::Event() wrote the event's Cause and Notes into single-quoted
literals of the Events INSERT without escaping them. Both come from
outside the daemon: Cause includes the names of the alarmed zones, which
any Monitors=Edit user can set, and the external trigger's cause and
text, which zmtrigger.pl takes unauthenticated from TCP port 6802. A
zone name could close the literal and fill the rest of the row with a
subquery, copying any value the ZoneMinder database user can read, such
as password hashes, into an event shown in the web UI.
An ordinary apostrophe was enough to break it: the INSERT failed, and
the constructor retries until it succeeds, so it logged the failure
several times a second and the monitor recorded no further events until
zmc was restarted. A zone named "Bob's drive" stopped motion recording.
Escape both, as updateNotes() already does for the same column. While
here, escape the other SQL values built from strings in src/: the
Event_Data detections JSON, the DefaultVideo update, both Tags queries,
the Servers lookup by ZM_SERVER_NAME and the device, path, protocol,
host and port that the zmc monitor-loading queries match on.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Fix: JPEG snapshot generation when requested frame is unavailable in event video
* Improve frame extraction fallback for unavailable timestamps
- Limit ffprobe search to a small time range around the requested position
- Validate the generated JPEG before considering extraction successful
- Fix timestamp formatting to be locale-independent
- Remove obsolete command/output handling from callers
-Handle non-monotonic packet PTS order
-- Do not assume ffprobe returns packets sorted by PTS
-- Scan all packets in the selected time range
-- Select the closest packet before and the closest packet after the requested timestamp
-- Avoid selecting an incorrect fallback frame when PTS values are reordered
* Use temporary files for frame extraction
- Write FFmpeg output to a unique temporary file for each extraction attempt
- Atomically rename the validated frame to the target path on success
- Prevent failed concurrent requests from deleting a frame created by another request
* Fix video frame extraction near event end
* Remove frame_time condition from lookback loop
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Isaac Connor <isaac@zoneminder.com>
Co-authored-by: Isaac Connor <iconnor@connortechnology.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: free a stream's monitor and playback buffer when nph-zms exits
Every nph-zms exited with ~1 MB still allocated, reported by LeakSanitizer
on every stream:
- The monitor's zones each hold a shared_ptr back to it, so releasing the
stream's reference never freed the Monitor, its zones (a full-frame zone
image each), camera or links. zmc breaks the cycle with disconnect() on
exit; zms never did. ~StreamBase now calls it.
- The buffered-playback SwapImage array (connkey + buffer) was never
deleted. MonitorStream now frees it in its destructor.
Freed at process exit either way, so nothing grows, but it shows up in
every leak report and hides real ones.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: check a stream frees its monitor and zones on destruction refs #5186
A stream holds a monitor with one zone; once the stream is destroyed, a
weak_ptr to the monitor must have expired. Each Zone holds a shared_ptr
back to its Monitor, so without ~StreamBase breaking the cycle the monitor,
its zones and their images outlive the stream. Fails on 82901ac1f
(REQUIRE( watched.expired() )), passes with the fix. Needs no database or
sanitizer, so ci-cpp-tests runs it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* refactor: hold the buffered-playback array in a unique_ptr refs #5186
temp_image_buffer is now a std::unique_ptr<SwapImage[]>, so it is freed
with the stream without a hand-written destructor, and the leak cannot
come back by dropping that line. The array is private and only allocated
in runStream(), so a unit test cannot see it freed without a sanitizer;
under LeakSanitizer, buffered nph-zms streams leaked 48,008 bytes from
runStream() on 82901ac1f and nothing with this branch.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* fix: free decoded frames and images once no consumer can read them
Packets stay in the packet queue for passthrough's keyframe alignment and
for pre-event frames, but each also kept its decoded AVFrame until it left
the queue, and with SaveJPEGs set (the default) its Image too: ~15 MB per
packet at 5MP. The frames also pinned libavcodec's buffer pool at the
queue's depth. With on-demand decoding and viewers, or a keyframe interval
longer than MaxImageBufferCount, zmc grew to several GB.
- Analyse() releases in_frame, and the y_image view into it, once the
packet has been analysed; later readers use packet->image. It is kept
when an encoder has no image to work from.
- PacketQueue::releaseDecoded() frees every decoded copy of packets that
all iterators have passed and that lie before where an event could still
start (openEvent's pre-event walk back, to a keyframe for passthrough).
The compressed packets stay queued.
Event output (capture/analysis JPEGs, snapshot, alarm, video) is unchanged.
refs #4860
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: say which reload can widen the window under releaseDecoded refs #4860
The comment said the release boundary can't move back because events only
open on the analysis thread. That holds for events, but a RELOAD action
(zmu --reload) runs Monitor::Reload() on zmc's main loop and can widen
pre_event_video_packet_count or set keep_keyframes without clearing the
queue. Say so, and what an event right after it loses: capture JPEGs for
released passthrough frames; on encode those packets are already gone.
Raised in review of #5185.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: never start an event on packets whose decoded data was released refs #4860
releaseDecoded() frees the images and decoder frames of packets before the
earliest point an event could start, computed with the current pre-event
window. A RELOAD action (zmu --reload) can widen that window, or turn on
keep_keyframes, without clearing the queue. On passthrough the queue still
holds the previous GOP until the next keyframe trims it, so the next event
could walk back into packets already released: its video was complete, but
those frames had no capture JPEGs. On a 15 fps camera with a 30-frame GOP,
widening PreEventCount 5 -> 100 left frames 1-30 without JPEGs in about a
third of the events right after the reload.
get_event_start_packet_it() now treats the first packet at or after
released_before_ as the front of the queue and warns when it stops there.
For passthrough that packet is the keyframe releaseDecoded() stopped on;
after a reload from encode to passthrough it may not be, so the start
moves forward to the next keyframe. With an unchanged window the walk
never reaches the boundary. After a widening reload the next event gets up
to one GOP fewer pre-event frames than before #4860's change, instead of
frames without images.
Raised in review of #5185.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
With its player left on Auto, a monitor with go2rtc enabled had
selectNextPlayer() pick go2rtc_webrtc first, and with ZM_GO2RTC_PATH
empty select_go2rtc() then alerted "ZM_GO2RTC_PATH is empty", on the
montage for every such monitor, although nobody had chosen go2rtc.
The fallback chain already skipped go2rtc for a monitor without it
enabled; skip it when ZM_GO2RTC_PATH is empty too. A monitor or browser
that chose go2rtc reaches select_go2rtc() directly and still gets the
alert.
A node test drives selectPlayer() through the restart chain for Auto
without a path, Auto with one, and go2rtc chosen explicitly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zmupdate.pl applied every zm_update-*.sql at or above the database's
version, with no upper bound. Migration files from a newer release left
in the update directory, after a downgrade or with a newer build's files
installed over an older one, were applied to a schema they were not
written for. Installing a 1.38 build over a 1.39 one ran the 1.39.0 to
1.39.5 migrations against the 1.38 database before one failed.
Skip, and say so, any migration newer than ZM_VERSION.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZoneMinder's RTSP server only starts when ZM_MIN_RTSP_PORT is set, and it
defaults to empty. The go2rtc and Janus managers decided to use the
restream from the monitor's RTSP Server flag (Janus: its restream flag)
alone, so with the port unset they built rtsp://127.0.0.1:0/<name>.
go2rtc was given that as the monitor's primary stream and as
<id>_ZoneMinderPrimary, neither of which can play; Janus pointed its
mountpoint at it.
Use the restream only when the port is set as well, and warn when a
monitor asks for it without one, so the camera's own stream is used.
MonitorStream applies the same test before choosing the
_ZoneMinderPrimary stream, which is no longer registered in that case.
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>
The patch loop writes ZM_DYN_DB_VERSION after each file so that a run
that dies partway leaves a record of how far it got. It wrote $version,
the version the database started at, which the loop never changes, so
the write was a no-op: a failed upgrade left the original version in
place and the rerun started again from the beginning, re-running every
patch that had already applied.
Write $v, the version of the patch that was just applied. A run that
completes still ends up at ZM_VERSION from the cascade write below.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KLYswrguKc6bBYL8JZbpmZ
The unbatched self-join DELETE takes a row lock on everything it scans,
so on a large Frames table it exhausts the lock table and dies with
ERROR 1206 whether or not there is anything to delete, leaving the
upgrade stopped before the primary key is rebuilt.
Look for duplicates first with a plain SELECT ... INTO, which is a
non-locking read, and only when there are any delete them 100 event ids
at a time so each statement locks just that range.
Tested on a scratch database seeded with duplicates at event ids 1, 50,
250 and 999: all duplicates removed, the lowest Id of each
(EventId, FrameId) kept, and the no-duplicates path runs clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KLYswrguKc6bBYL8JZbpmZ
EventStream receives the stream into a DOM <img> and copies it to the
canvas from a rAF loop. Gecko hands canvas drawImage() the frame the
img held when it was first decoded and never refreshes that snapshot,
so a multipart/x-mixed-replace img drawn to a canvas repaints one frame
forever while the stream keeps arriving. Montage review showed a single
frozen frame in Firefox while the timeline advanced; Chrome, whose
drawImage() tracks the element, played the same streams normally.
createImageBitmap() reads the frame the element is showing now. It is
async, so a tick is skipped while a request is outstanding rather than
queueing requests up behind the stream rate, and the pending flag is
cleared on teardown along with the img.
The img is left offscreen: measured against a reference
multipart/x-mixed-replace server, an img at left:-9999px, 1x1,
visibility:hidden still yields a fresh frame on every
createImageBitmap(), so Gecko is not culling the decode.
Verified against montage review on Firefox 154 and Chrome 154, driving
both from a script that hashes each monitor's canvas 40 times at 250ms:
before, Firefox saw 1 distinct frame per canvas and Chrome 40; after,
both see 40.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KLYswrguKc6bBYL8JZbpmZ
* fix: schedule ONVIF subscription renewal for short and unreported lifetimes refs #5179
The Beward camera from #5165 grants a 60 second subscription and its
RenewResponse carries no TerminationTime. Three things combined so that
ZM renewed after every PullMessages that returned messages (61 renewals
in 74 seconds of the reporter's log):
- The renewal was scheduled a fixed ONVIF_RENEWAL_ADVANCE_SECONDS (60)
before termination, which for a 60 second subscription is its creation
time. ONVIFNextRenewalTime() now uses the smaller of that advance and
half the remaining lifetime.
- Renew() only logged a missing TerminationTime and left
next_renewal_time in the past. assume_renewal_times() now schedules
from the lifetime we asked for, capped at what the camera last granted
(ONVIFAssumedLifetime()), so this camera renews every 30 seconds.
- IsRenewalNeeded() was only checked after a response carrying messages,
so a camera quiet for longer than its subscription was never renewed.
It is now also checked after an empty (SOAP_EOF) poll.
Since renewal is now checked after every poll, a camera that answers
Renew with ActionNotSupported has renewal disabled rather than being
asked again on each poll, matching the existing handling of unusable
TerminationTimes.
Tests cover the renewal time for long, short and very short
subscriptions, and the assumed lifetime with no, smaller and larger
previously granted lifetimes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: only disable ONVIF renewal for ActionNotSupported, keep sub-second grants refs #5179
Review follow-ups on the renewal scheduling change:
- Renew() treated soap->error == 12 as ActionNotSupported, but 12 is
gSOAP's generic SOAP_FAULT. With renewal now disabled on that path, a
NotAuthorized or InvalidArgVal fault from Renew would have stopped
renewal for good while marking the subscription healthy.
ONVIFIsActionNotSupported() checks the fault subcode and string for
ActionNotSupported (wsa: or ter:); any other fault now takes the
existing cleanup and re-subscribe path.
- granted_lifetime truncated the remaining time to whole seconds. A
termination less than a second away stored 0, which reads as "never
reported", so the next Renew without a TerminationTime assumed the full
requested 300 seconds. ONVIFGrantedLifetime() rounds up instead.
Tests cover ActionNotSupported subcodes and strings against other faults
and non-fault results, and the granted lifetime for whole, fractional and
sub-second remainders.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: count an assumed ONVIF renewal lifetime from the request, not the response refs #5179
When a RenewResponse has no TerminationTime, assume_renewal_times()
started the assumed lifetime when the response arrived, but the camera
counts it from when it received the request. A slow response pushed the
deadline and the next renewal later by the response time: with
subscription_timeout=10 and a 6 second response, the camera's deadline
was request+10 but the renewal was scheduled for request+11. In absolute
renewal mode it also ignored the exact deadline that was sent.
Renew() now records the time just before building the request and the
deadline it asks for: request time plus subscription_timeout, or the
absolute whole-second time it sends. ONVIFAssumedTermination() replaces
ONVIFAssumedLifetime() and returns that deadline, capped at request time
plus the lifetime the camera last granted. The renewal is scheduled from
the request time; if the response arrived after it, it is already due and
the next poll renews.
Tests cover no, smaller and larger previous grants, an absolute deadline
kept exactly, and the slow-response case from review.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Under a threaded MPM (event/worker) Apache blocks signals in its worker
threads and children started by exec() inherit that mask. Where /bin/sh
is bash (Gentoo, Fedora/RHEL, Arch) the mask reaches intel_gpu_top, which
never sees the SIGTERM from "timeout 2", so exec() never returned, the
page never loaded and every visit left another intel_gpu_top running.
dash, Debian's /bin/sh, clears the mask, which is why it doesn't show
there.
- Run the sample with -n 2 where intel_gpu_top supports it, so it exits
by itself after one full period. Support is detected from -h, because
intel-gpu-tools 1.26/1.27 (Ubuntu 22.04, Debian 12) lack -n.
- Without -n, keep "timeout 2" but add -k 3, so a blocked SIGTERM is
followed by an unblockable SIGKILL. The samples printed before the kill
are still parsed; only when there are none does the view explain the
kill instead of showing a raw-output error.
- Give the -h and -L calls a SIGKILL timeout too.
- Report the driver bound to the Intel DRM device and the kernel version
instead of /sys/module/i915/version or modinfo, neither of which gives
a version for i915 or xe, and neither of which exists when the driver
is built in, so Driver always showed Unknown.
- Add dark theme styles for the view's cards, card headers, footer row
and progress bars, which kept Bootstrap's light backgrounds behind the
dark theme's light text.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZM asks the camera to hold each PullMessages for up to pull_timeout
seconds (default 1). The Beward camera from #5165 answers in 10-30ms with
no messages, which gSOAP reports as SOAP_EOF, and Run() issued the next
request at once: ~2,500 PullMessages in 74 seconds of the reporter's log
(~34/s), each with a fresh UsernameToken digest.
Time each PullMessages. When it comes back with no messages (SOAP_EOF, or
a SOAP_OK response with no NotificationMessage) before the Timeout,
WaitForMessage() now waits out the remainder in 100ms steps, staying
responsive to terminate_ and zm_terminate. A camera that ignores the
long-poll is then polled at most once per pull_timeout; one that holds it
is polled again immediately as before. Events raised during the wait stay
queued in the camera's pull point and arrive with the next request, so
the added latency is at most pull_timeout.
ONVIFEarlyPollWait() computes the remainder and is tested for an
immediate answer, a full hold, a partial hold and sub-millisecond
elapsed times.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Per-topic alarm expiry takes each alarm's deadline from the
PullMessagesResponse TerminationTime. Beward cameras send their
CurrentTime as the TerminationTime in every response, so an alarm was
stored with a deadline that had already passed and the sweep at the end
of the same WaitForMessage() pass removed it about 25us later. The
analysis thread checks onvif->isAlarmed() once per frame, never saw the
monitor alarmed, and no event was recorded even though the log showed
"ONVIF Triggered Start Event".
Move the TerminationTime handling into a free function,
ONVIFAlarmTermination(), which applies the camera clock offset as before
and returns false when the adjusted time is not after now. The alarm then
gets no expiry and is cleared by the camera's explicit State=false
message, as it was before per-topic expiry was added. The Debug line now
also prints CurrentTime so this camera behaviour is visible in logs.
Tests cover TerminationTime equal to and before CurrentTime, a future
TerminationTime, a skewed camera clock, a missing CurrentTime keeping the
previous offset, and a missing TerminationTime.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
view_video.php took the end of the range straight from the request and never
checked it against the file:
if (!empty($matches[2])) $end = intval($matches[2]);
$length = $end - $begin + 1;
so on a 1000 byte file:
bytes=0-99999 206, Content-Length 100000, body 1000 bytes
bytes=5000-6000 206, Content-Range naming bytes that do not exist
bytes=500-100 206, Content-Length -399
bytes=1000- 206, Content-Length 0
bytes=-100 200 with the whole file, not the last 100 bytes
A client that is told to expect 100000 bytes and gets 1000 does not see a bad
request, it sees a truncated file, and reports the video as broken. The suffix
form was not recognised at all because the pattern required a digit before the
dash.
This is reached once per fragment by the byte-range HLS manifest VideoStore
writes, every fragment being a Range against the one mp4, so a player that
asks for anything the file cannot supply gets a body that does not match its
own Content-Length rather than an answer it can act on.
Parse the header properly: clamp a range that runs past the end, because a
client may ask for more than is there and is entitled to what is there;
answer 416 with "Content-Range: bytes */size" when the range cannot be
satisfied at all, so the client learns the real length; and read "-N" as the
last N bytes. Length is now derived from the range being served rather than
the one requested, and the send loop counts down by the bytes it actually
read, so Content-Length and the body cannot disagree.
Only the first range of a multi-range request is served, as before. A
multipart/byteranges body is not worth building for this, and falling back to
sending the whole representation is not an option when these are event videos
of hundreds of megabytes; Content-Range names exactly what was sent.
The parsing is its own dependency-free include so it can be tested without a
database, and tests/php/test_http_range.php covers each case above plus a
sweep asserting that every range it ever returns lies inside the file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#5174 reports an event's m3u8 as invalid and proposes moving every fragment's
byte range 8 bytes further into the file, from @17203 to @17211.
That reading comes from ffprobe's trace, whose second column is the offset of
the box *body* -- the start plus the 8 byte box header -- not the start. In
the manifest quoted there the first fragment is 1434876@17203, and the trace
has the moof body at 17211 and the mdat ending at 1452079, so the range spans
exactly moof(264) + mdat(1434612) = 1434876 from the start of the moof. The
proposed change would cut the moof header off every segment.
The manifest is not contiguous, which is what draws the eye: the init range
is ftyp+moov and the fragments start 16KB later, because reserve_region puts
the leading sidx in between and the sidx has to END where the fragments
BEGIN. Nothing fetches those bytes and HLS does not require byte ranges to
abut.
So assert it rather than argue it. Against sidx-moof.mp4, using the box
walker the sidx tests already keep for the purpose, check that a fragment
starts at its moof box rather than 8 bytes in, that fragments run back to
back, that the init range is the header boxes and nothing else, and that the
gap between them is exactly the reserved index.
This says nothing about the browser error that prompted the report, which is
not quoted in the issue; it only rules the byte ranges in or out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8c35190b0 moved the Servers lookup into a foreach over $s but kept
define('ZM_SERVER_ID', $Server->Id()), so every request on a node with
ZM_SERVER_NAME in zm.conf died with 'Call to a member function Id() on
null'. Use $s like the ZM_SERVER_HOST branch.
Also make the 'no corresponding entry in Servers table' errors reachable
($thisServer starts as an empty Server object, which is always truthy,
so test its Id) and concatenate the ZM_SERVER_ID error with '.' instead
of '+', which throws a TypeError on PHP 8.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
