- SearchCache::get: if gcb returns false, call dcb(false, {}) instead
of silently proceeding with dcb(true, {}).
- DhtProxyClient push refresh: skip expiration of old values when the
get() completion reports failure (ok == false), preventing spurious
expires on transient errors.
In pushNotificationReceived, track the set of value IDs received
in the refresh GET and only expire values whose IDs are not in the
new set. Previously all old values were expired on refresh,
incorrectly expiring edited values.
ValueCache emitted any content change of a known value as an edition
while OpValueCache only accepts a higher seq. A node reporting an older
version made ValueCache replace its copy and re-emit versions that were
then dropped or, with source counting, double counted downstream. Use
the same rule at both levels.
ValueCache::addValues no longer emits an expire callback for the
old version when a value is edited; only the add callback for the
new version is emitted.
Add a grace period in expireValues when the cache is synced, using
the value type's expiration as an extra margin before emitting
expirations.
OpValueCache::onValuesAdded: on edit, expire the old value and add
the new one without incrementing refCount.
SearchCache::get: only serve from cache when synced.
Add testListenValueEdit to DhtRunnerTester: verifies that editing
a value triggers an add callback (not an expire+add), and that
the edited value eventually expires.
Add testListenValueEdit and testPushListenValueEdit to
DhtProxyTester: same semantics tested over the long-poll and push
notification proxy paths.
Fix proxy tearDown order to destroy DhtProxyServer before shutting
down the proxy DhtRunner, preventing use-after-free on cancel.
Add unit tests for OpValueCache and ValueCache in isolation.
OpCacheTester covers add/edit/expire flows, refcount management,
and value synchronization lifecycle.
ValueCacheTester covers edition handling and value expiration.
asio sockets are not thread-safe. http::Connection::close() used to close
the socket synchronously on the calling thread. Request::cancel() and
Request::~Request() (via terminate()) are called from arbitrary threads
(e.g. DhtProxyClient::stop(), cancelAllListeners(), cancelListen()),
so the close raced with in-flight operations on the io_context thread.
In particular, asio::async_connect's range_connect_op closes and
re-opens the socket when trying the next endpoint (common on mobile
where the IPv6 endpoint fails first). A concurrent close() from another
thread corrupts the epoll reactor's per-descriptor state, leading to
crashes in epoll_reactor::deregister_descriptor / register_descriptor
called from range_connect_op::process.
Dispatch the close to the io_context thread when not already on it.
Also keep the connection alive during the TLS handshake, like every
other asynchronous operation, so that the destructor can never run
while an operation is pending.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Track subscribe/permanent-put expiration with std::chrono::system_clock
(wall clock) instead of steady_clock in both the proxy client and server,
so persisted expirations survive restarts and map directly to epoch time.
- DhtProxyServer::clock is now system_clock; expireTimer/expireNotifyTimer
use asio::system_timer.
- DhtProxyClient::refreshSubscriberTimer uses asio::system_timer and
system_clock::now() for subscriber refresh.
to_system_time computed its saturation bounds by subtracting a
steady_clock duration from system_clock::time_point::max(). When the
two clocks have different periods (microseconds for system_clock and
nanoseconds for steady_clock on libc++), the common type promotes the
bound to nanoseconds and overflows, so every future time point was
converted to system_clock::time_point::max().
Cast the delta to the target duration before comparing against the
bounds, and saturate from_system_time before the reverse cast for
symmetry.
An error array was read at index 0 without checking its size, so a packet
carrying "e":[] dereferenced a null element pointer and crashed the process
on unauthenticated UDP input.
When a message had an error array but no "a", "r" or "u" map, the same object
was then walked through the map view of the msgpack union. A two-element array
appeared to hold two key/value pairs, reading past its allocation. A request
field that was not a map was never type-checked either.
Validate the error array size, restrict request parsing to "a", "r" and "u",
and require those to be maps. Add parser regressions for the empty error
array, the error-only message and the non-map request; the first one
segfaults without this change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Key partial messages by source and transaction ID, buffer bounded fragments received before their header, and avoid charging reordered requests twice against rate limits. Cover source collisions, early fragments, empty data and large values.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reject field-compressed responses with no selected fields before computing the value count, avoiding a division by zero on malformed network input. Add a parser regression test.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dht::onDisconnected() only starts a bootstrap when bootstrapJob is
unset, reading a set handle as "a bootstrap is already scheduled".
That reading is wrong: Scheduler::run() erases the job from its timers
and calls it, but never clears the shared handle the caller holds, and
Scheduler::cancel() leaves the handle alone too when the job is no
longer in the timers. The handle therefore stays set for as long as the
Dht lives, once a bootstrap job has run.
That is harmless while bootstrap() reschedules itself, which it does as
long as the node is disconnected. It is not harmless on the early
return: a job that fires while a ping is still in flight, or while a
dubious contact keeps the status at Connecting, returns without
rescheduling. From then on the chain is dead and onDisconnected() will
not restart it, so a node that goes on to lose every contact stays
disconnected for good. Only an explicit addBootstrap() or a
connectivityChanged() can revive it, and a node whose network simply
went away gets neither.
Clearing the handle on the early return restores the meaning
onDisconnected() relies on.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
gnutls_pubkey_export reports the size of the exported PEM including its
terminating null byte, and PublicKey::toString hands that size straight
back to the caller. The returned string therefore ends with a null byte
that is not part of the PEM text.
Certificate::toString does not have the problem because
gnutls_x509_crt_export excludes the terminator, so the two exports
disagree on what a PEM string is.
The stray byte travels: Value::toJson stores owner->toString() in the
"owner" field, so a value published through the proxy carries a null
byte inside a JSON string, which is valid JSON but a surprise for any
consumer that treats the field as text.
Resize to the reported size and drop the terminator when it is there.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RevocationList::sign() always overwrote the CRL Number extension: random
on the first signature, incremented afterwards. This leaves no way for a
certificate authority replicated over several devices to keep numbers
unique, since every device branching from the same list mints the same
number for a different content.
Add an optional `number` parameter, used verbatim when non-zero, so
callers can partition the number space (RFC 5280 5.2.3 requires
monotonicity for a given CRL scope and issuer, not globally).
Also fix the default increment path: the extension holds a DER integer
with leading zeros stripped, so reading it straight into an uint64_t only
worked for numbers wide enough to occupy all 8 bytes. Signing a list
numbered 5 produced 0x0500000000000001 instead of 6. Read and write the
number as a big-endian byte string instead, which makes the `endian()`
helper unnecessary.
Add tests covering the default numbering, explicit numbers (including
values with leading zero bytes and the largest positive 8-byte integer),
pack/unpack round-trips, and a device-partitioned numbering scheme.
In SearchCache::expire, if there is an op whose expiration is equal to
`now`, then it's not processed and the function returns `now` as the
next expiration timepoint. This can lead to the opExpirationJob in
Search::cancelListen getting rescheduled at the same time in an infinite
loop.
Since 15eb221c the node's bound port is persisted alongside the DHT
state and reused on the next run for a stable identity. If that port
is still held by another process - typically a fast application
restart racing the previous instance's shutdown - UdpSocket
constructor throws (both the IPv4 and IPv6 binds fail) and
DhtRunner::run() propagates the exception: the node never starts.
In Jami this left every account stuck in the connecting state after a
quick restart, with 'DhtException occurred: Can't bind socket' logged
per account, and no retry.
The persisted port is only a best-effort hint: when binding it fails,
retry with OS-assigned ports instead of throwing. The persisted file
is then refreshed with the newly bound ports as before.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover starting a node whose persisted port (persist_path + "_port.txt")
is already bound by another node, as happens when an application is
restarted while the previous instance still holds the socket. The node
is expected to start on a fallback port instead of failing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The pushType field (593e59e0) lets the DHT proxy server tag push
notifications with the connection type ("pt" in the FCM payload), so
mobile clients can tell incoming calls apart from background noise.
It was only serialized in msgpack. Clients using DhtProxyClient
send their puts as JSON (Value::toJson()), where the field was
silently dropped, and the proxy server parsed the incoming JSON with
Value(const Json::Value&), which never read it back. As a result any
value put through a proxy (e.g. a call request from a mobile device)
reached the recipient's device with an empty "pt", was classified as
noise, and did not wake up the callee: calls from proxied devices
never rang, while the same call from a full DHT node (desktop) did.
Serialize pushType as "pt" in Value::toJson() when set, and parse it
back in the JSON constructor, mirroring the msgpack behavior.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover Value::toJson() / Value(const Json::Value&) preservation of the
pushType field, and its absence from the JSON when empty. This is the
serialization path used by DhtProxyClient::put(), so mobile clients
behind a DHT proxy depend on it to propagate the push notification
type.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adrien Béraud
Cyrille BéraudandCopilot
dependabot[bot]
Cyrille
François-Simon Fauteux-Chapleau