Add "+DEBUG" to the version string only when configured with --enable-debug
The debug log messages are always available and a runtime option (since
commit c7de505c), but the assert()'s are only active when ngIRCd was
configured with the "--enable-debug" option.
So only add "+DEBUG" to the version string when the latter is the case.
michi [Sun, 1 Jan 2023 12:12:54 +0000 (13:12 +0100)]
Make the debug loglevel always available
This basically means to unifdef DEBUG in (almost) all places.
We keep it in src/portab/portab.h so DEBUG stays available to
enable assert(). Also add a comment about this.
Better validate MODE +k & +l parameters and return errors
Implement new numeric ERR_INVALIDMODEPARAM_MSG(696) and:
- Reject channel keys with spaces and return ERR_INVALIDMODEPARAM_MSG;
This was possible until now and resulted in garbled IRC commands later.
- Reject empty channel keys and return ERR_INVALIDMODEPARAM_MSG;
This was possible until now and resulted in garbled IRC commands later.
- Return ERR_INVALIDMODEPARAM_MSG when user limit is out of bounds;
This was silently ignored until now.
Channel modes +k & +l: Always report an error when a parameter is missing
This relates to #290 and considerations which errors to show when: and I
think it is the better approach to give feedback instead of silently
failing.
Note that this code path is also used when handling modes of channels
defined in "[Channel]" blocks in configuration files: in this case the
client is the local server and we can't send messages to it, because it
has no socket connection! Therefore we need those "is_machine" checks
and log an error im this case.
Alexander Barton [Mon, 26 Dec 2022 16:32:59 +0000 (17:32 +0100)]
Channel mode setting: The local server is allowed to work on local channels
Don't forbid the local server to change modes on local channels: this
happens when overriding modes on local (&) channels in the server
configuration file, for example, and is perfectly fine.
Without this patch, the server worked as expected but showed critical
error messages for each local channel in its configuration file:
"Got remote MODE command for local channel!? Ignored."
Alexander Barton [Tue, 29 Dec 2020 20:12:43 +0000 (21:12 +0100)]
Don't set AI_ADDRCONFIG, even when it exists
Basically, the issue described in #281 is that the test suite uses the
IPv4 address 127.0.0.1 on an IPv6-only host. But this is the "safest"
thing to do in (almost) all other setups: relaying on DNS host names
makes things even more complex, as different systems map 127.0.0.1
differently (including the reverse lookup; that's why we switched to
127.0.0.1 back in 2014, see commit 3f807e10457).
But with AI_ADDRCONFIG set, on an IPv6-only host, we prevent 127.0.0.1
to get translated properly, even when the loopback interface has this
address configured! So don't set it any more.
The drawback is that the resolver possibly returns more addresses now,
even of an unsupported/not connected address family; but this shouldn't
do much harm in practice, as ngIRCd iterates over all returned addresses
while trying to establish an outgoing connection.
Revert "Show allowed channel types in ISUPPORT(005) numeric only"
The ISUPPORT(005) numeric lists only channel prefixes which are listed
in the "AllowedChannelTypes" configuration option. And if this is the
empty string ("") for example, this now results in IRC clients assuming
"oh, no channel prefix characters at all, so no channels at all, so no
PRIVMSG can go to any channel" -- which is not the case when there are
pre-defined channel set up or other servers still having channels!
So "allowed channel types" != "supported channel types", and we always
have to list all supported ones in the ISUPPORT(005) numeric!
Test suite: Wait 2 seconds after reloading the daemon
On reload, all listening ports are closed, configuration updated, and
then opened again. Which leads to subsequent tests running while the
daemon isn't listening on any ports, and that's why the tests fail.
The "proper" way whould be to loop and check for open ports, but waiting
is what the start-server.sh script does right now, so stick with this in
reload-server.sh for now as well.
This fixes the issue, at least on my RaspberryPi ...
Alexander Barton [Thu, 11 Jun 2020 14:45:30 +0000 (16:45 +0200)]
Don't send invalid CHANINFO commands when no key is set
It can happen that a channel is +k, but no key is set: for example by
misconfiguring a pre-defined channel. In this case, ngIRCd sent an
invalud CHANINFO command ("CHANINFO #test +Pk 0 :'", note the unset
key represented by the two spaces) to its peers.
Alexander Barton [Mon, 25 May 2020 21:43:29 +0000 (23:43 +0200)]
IRC_SERVER: Make sure that the client sent a prefix
The SERVER command is only valid with a prefix when received from other
servers, so make sure that there is one and disconnect the peer if not
(instead of crashing ...).
This obsoletes PR #275.
Thanks Hilko Bengen (hillu) for finding & reporting this as well for the
patch & pull request! But I think this is the "more correct" fix.
Alexander Barton [Mon, 25 May 2020 17:02:16 +0000 (19:02 +0200)]
Fix PING-PONG handling when processing backlog in read buffers
Prior to this commit, the PONG wasn't registered correctly, becauuse the
"last ping" time was set to time(NULL), which could be bigger than the
"last data" time stamp, for example when handling the read buffer took
more than 1 second -- and this resulted in the PONG time out kicking in
effectively disconnecting a newly linked server for example, because
ngIRCd thought it was still waiting for a PONG: last data < last ping.
Now the "last ping" value has three possible values:
0: new connection, no PING, no PONG so far.
1: got a PONG, no longer waiting for a PONG.
<t>: time stamp of last sent out PING command.
Alexander Barton [Sun, 24 May 2020 21:24:51 +0000 (23:24 +0200)]
Revert "Set the "last data" time to "last ping" time when updating the latter"
This patch completely broke the PING-PONG logic: now ngIRCd never
disconnects any stale peers but keeps sending out PINGs over and over
again ...
The real issue (server disconnects right after connect) will be fixed in
the next commit, but let's revert to the somewhat "half-broken but
'known' state" first ...
Allow more characters per line in MOTD and help text files
Change the line buffer in the Read_TextFile() function from 127 to
COMMAND_LEN (=512) bytes. Lines can't even get that long, because they
have to be prefixed before being sent to the client, so this is a sane
maximum.
This allows for even more "fancy" and "wider" MOTDs :-)
Not sure about the portability of strpbrk() in really ancient OS, and
this was the only place where it became used recently in ngIRCd ...
So let's play it safe! ;-)
Don't wait for the network when read buffers possibly hold commands
There is no point in waiting up to one second for the network receiving
new data when there is still a read buffer holding at least one command;
we shouldn't waste time but handle it immediately!
Handle commands in the read buffer before reading more data
If there are more bytes in the read buffer already than a single valid
IRC command can get long (513 bytes, COMMAND_LEN), wait for this/those
command(s) to be handled first and don't try to read even more data from
the network (which most probably would overflow the read buffer of this
connection soon).
This sounded like the right approach at first, but I'm not that sure
that it really makes sense to have different sizes of read buffers: the
per-connection read buffer only needs to keep data that is needed to
parse one full command, be it plain text, encrypted and/or compressed.
Then ngIRCd should handle this one command, move leftover data to the
beginning of the buffer and read the next chunk from the network that is
missing to get the next complete command (512 bytes at max).
So I revert this for now and try to fix the logic in Read_Request(),
which is broken nevertheless, as it results in servers becoming
disconnected during "server burst" when "big" lists are transferred.
Correctly use Config_Error() instead of Log() in Read_Config()
The name of the Config_Error() function is misleading: it is not only
used to show configuraton errors, but all messages shown during normal
operation as well as for "config testing": it takes care of the correct
formatting of the messages (syslog, forground logging, config testing).
Set the "last data" time to "last ping" time when updating the latter
This is required because the PING can be received quite a bit earlier
than it is actually handled, for example during "server burst" or other
heavy operations:
So the times won't match and PING-PONG logic would become garbled,
because we test for "last ping > last data" to determine if a PING
already was sent or not.
This applies the same logic we have for write buffers to distinguish
between server and client connections and sets the maximum buffer size
accordingly. As a result peering with servers with many GLINE/KLINEs
does not kill the connecting server connection anymore.
Depending on the stack size, too many clients on the same channel
quitting at the same time would trigger a crash due to too many
recursive calls to Conn_Close().