Revert "Make `Dictionary::Remove(Iterator)` return the next iterator" and
change the one location that now made use of that return value to increment the
iterator itself beforehand. I don't see anything that would be wrong with the
previous code and I'm pretty certain this is a compiler bug. While at it, I've
also removed a stray semicolon and changed the increment in the other branch to
the prefix operator as the value is discarded.
[35/151] Building CXX object lib/base/CMakeFiles/base.dir/base_unity.cpp.o
FAILED: lib/base/CMakeFiles/base.dir/base_unity.cpp.o
/usr/bin/c++ -DBOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT -DBOOST_COROUTINES_NO_DEPRECATION_WARNING -DBOOST_FILESYSTEM_NO_DEPRECATED -DOPENSSL_SUPPRESS_DEPRECATED -D_GNU_SOURCE -I/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f -I/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/nlohmann_json -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/utf8cpp/source -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/mmatch -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/socketpair -O2 -g -Wsuggest-override -g -pthread -Winvalid-pch -O2 -g -DNDEBUG -std=c++1z -MD -MT lib/base/CMakeFiles/base.dir/base_unity.cpp.o -MF lib/base/CMakeFiles/base.dir/base_unity.cpp.o.d -o lib/base/CMakeFiles/base.dir/base_unity.cpp.o -c /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/configobject.cpp:8:0,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:9:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/serializer.hpp:20:14: warning: 'virtual const char* icinga::CircularReferenceError::what() const' can be marked override [-Wsuggest-override]
const char *what(void) const throw() final;
^~~~
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/filelogger-ti.hpp:12:0,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/filelogger.hpp:8,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/filelogger.cpp:4,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:25:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/streamlogger.hpp:33:7: warning: 'virtual void icinga::StreamLogger::ProcessLogEntry(const icinga::LogEntry&)' can be marked override [-Wsuggest-override]
void ProcessLogEntry(const LogEntry& entry) final;
^~~~~~~~~~~~~~~
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/streamlogger.hpp:34:7: warning: 'virtual void icinga::StreamLogger::Flush()' can be marked override [-Wsuggest-override]
void Flush() final;
^~~~~
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/config/configitem.hpp:8:0,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/scriptutils.cpp:17,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:56:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/config/expression.hpp:264:19: warning: 'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]
const DebugInfo& GetDebugInfo() const final;
^~~~~~~~~~~~
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:78:0:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/workqueue.cpp:80:1: internal compiler error: Segmentation fault
}
^
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://bugs.opensuse.org/> for instructions.
This reverts commit 3cb6e5a152.
to fix a build error against OpenSSL 4.0.
OpenSSL 4.0 made `ASN1_INTEGER` an opaque type.
Also, `X509_get0_serialNumber()` must be used now,
which returns `const ASN1_INTEGER*`.
`DiagnosticInformation()` wasn't able to take a `std::exception_ptr` due
to the missing conversion on older boost versions, so now everything uses
the std::exception_ptr instead. There are still a few reasons to use
`boost::exception` in some places, but for exception pointers, the standard
one should be better in most cases and almost never requires to include an
extra header.
There is some race condition when the `async_write()`/`async_flush()` operation
for the `icinga::Hello` message fails (connection reset by peer for example)
around the same time the connect timeout fires and calls `cancel()` on the
stream, the following call to `async_shutdown()` may block indefinitely. If
that happens, the endpoint remains in the connecting state and no new
connection attemps are initiated.
This commit fixes the issue by removing the `Defer` containing the
`async_shutdown()`. The purpose of `async_shutdown()` is to signal a clean
termination of the connection to the peer, which really isn't something that
makes sense to to in a `Defer` block that is also executed in case of errors.
For the one situation where doing a clean TLS shutdown makes some sense
(closing anonymous client connections), a call to GracefulShutdown() is added
to that specific code path.
A large part of the change is just changing the indentation of the code, given
that a now unnecessary `try`/`catch` block is removed.
The following Go code creates a TLS server that can be used to demonstrate the
issue. Note that given that a race condition is involved, this is not reliable
and the sleep duration may need some fine-tuning. For this to work,
`ApiListener.tls_handshake_timeout` needs to be set to a large-enough value
like 60s to disable the timeout for `async_handshake()` itself so that the
overall connect timeout is the one that fires. However, changing the timeout is
not a prerequisite for the problem, it just makes it easier to reproduce. The
error can also happen with the default timeouts if the TCP connect takes long
enough so that the handshake is started late enough that its timeout expires
after the connect timeout.
package main
import (
"crypto/tls"
"log"
"net"
"time"
)
func main() {
cert, err := tls.LoadX509KeyPair("bad-agent.crt", "bad-agent.key")
if err != nil {
panic(err)
}
listener, err := tls.Listen("tcp", ":1337", &tls.Config{
Certificates: []tls.Certificate{cert},
})
if err != nil {
panic(err)
}
log.Println("Listening on", listener.Addr())
for {
conn, err := listener.Accept()
if err != nil {
panic(err)
}
go handle(conn.(*tls.Conn))
}
}
func handle(conn *tls.Conn) {
addr := conn.RemoteAddr().String()
log.Println(addr, "new connection")
time.Sleep(15*time.Second - 10*time.Millisecond)
log.Println(addr, "SetLinger(0)", conn.NetConn().(*net.TCPConn).SetLinger(0))
log.Println(addr, "Handshake()", conn.Handshake())
log.Println(addr, "conn.NetConn().Close()", conn.NetConn().Close())
}
With additional logging in the `catch` block for `boost::system::system_error`
and `Defer shutdownSslConn` (both removed by this commit), this showed the
following. Note that in particular, `async_shutdown()` never returned,
indicating that it hangs in there.
[2026-04-24 17:32:56 +0200] information/ApiListener: Reconnecting to endpoint 'bad-agent' via host 'host.docker.internal' and port '1337'
[2026-04-24 17:33:11 +0200] critical/ApiListener: Timeout while reconnecting to endpoint 'bad-agent' via host 'host.docker.internal' and port '1337', cancelling attempt
[2026-04-24 17:33:11 +0200] information/ApiListener: New client connection for identity 'bad-agent' to [172.17.0.1]:1337
[2026-04-24 17:33:12 +0200] information/ApiListener: rethrowing for bad-agent: Error: Connection reset by peer [system:104 at /usr/include/boost/asio/detail/reactive_socket_send_op.hpp:137 in function 'do_complete']
[2026-04-24 17:33:12 +0200] information/ApiListener: doing async_shutdown for bad-agent
implicit conversions between signed `char` and `unsigned char` and
vice versa should be save and converting to unsigned char first
circumvents this warning on platforms where `char` is signed.
With the limit from the previous commit, if a JSON-RPC now message fails to
parse due to being nested to deep, it would have torn down the whole
connection. It is still possible to trigger that scenario from DSL config (for
example by returning nested structures from a lambda that is used in a check
with command_endpoint). In order to fail more gracefully, only discard the
single message and don't kill the whole connection.
Data structures parsed from JSON may be accessed recursively, so deeply nested
structures may wreak havoc by overflowing the stack. Thus, enforce a general
nesting depth limit of 24 by default (which should be more than enough for
reasonable use), with the ability to pass a different limit to JsonDecode() if
needed.
The log message on TLS handshake errors always stated that a client handshake
failed, even if if the connection was acting as the server. The commit changes
it so that the actual role is taken into account.
In Url::ParseUserinfo, after extracting the password, ValidateToken is
incorrectly called upon m_Username instead of m_Password. This commit
fixes this and actually verifies the password.
The bug was introduced with the surrounding code in 6571ffc2c8.
Luckily, this does not seem to have any security impact. However, as
being a bug, this commit now fixes the behavior.
This change just gives clear ownership over the CpuBoundWork to the
OutgoingHttpMessage, instead of the previous shared_ptr and weak_ptr
combination with an unclear purpose.
so that `/v1/events` doesn't have to use `IoBoundWorkSlot`.
`IoBoundWorkSlot#~IoBoundWorkSlot()` will wait for a free semaphore slot
which will be almost immediately released by `CpuBoundWork#~CpuBoundWork()`.
Just releasing the already aquired slot manually is more efficient.
If there are such requests, without this change, they would all be allowed and
processed, resulting in unsafe concurrent (write) access to these data
structures, which can ultimately crash the daemon or lead to other unintended
behavior.
Take a mutex to avoid race conditions in the map that lead to
segmentation faults. Move the ApiScriptFrame object back behind a
shared pointer to avoid holding the mutex for too long.
Fixes#10674
This commit refactors the ValueGenerator class to be a template that can
work with any container type. Previously, one has to manually take care
of the used container by lazily iterating over it within a lambda. Now,
the `ValueGenerator` class itself takes care of all the iteration,
making it easier to use and less error-prone. The new base `Generator`
class is required to allow the `JsonEncoder` to handle generators in a
type-erased manner.
This adds generalized IncomingHttpMessage and OutgoingHttpMessage templates
that support different types of streams (via a std::variant) and can both
be used for either requests or responses.
The tacked on metadata from the old HttpRequest and server connection from
the old HttpServerConnection have been moved to HttpApi(Request|Response)
classes that derive from the above generalized message types.
