- ui/window.c win_show_subwin (#1144): unconditional newpad without
freeing the previous pad leaks the ncurses subwin if the function
is invoked twice without an intervening win_hide_subwin. Added a
delwin guard.
- omemo/omemo.c omemo_on_disconnect (#1126): hash tables and
libsignal handles are destroyed unconditionally even when a full
init never ran (e.g. shutdown after a failed load), and neither
g_hash_table_destroy nor signal_*_destroy are documented as
NULL-safe. NULL-check each handle before tearing it down.
- xmpp/roster_list.c (#1140 / #1141 / #1160): null the caller's
pointer immediately after resource_destroy() so a stale reference
cannot trigger a double-free; resource_destroy itself only zeroes
the struct fields it owns and cannot reach back to the caller.
Cleanup of the conversion-safety warnings exposed by enabling
-Wconversion / -Wsign-compare in the previous commit, plus guard
clauses at the few places where unsigned arithmetic could actually
misbehave.
Build:
- configure.ac drops -Wno-error=conversion and
-Wno-error=float-conversion. Only -Wno-error=sign-conversion and
-Wno-error=sign-compare remain, gating the ~230 sign warnings
inherited from upstream that will be cleaned up in follow-ups.
Type / conversion fixes (no behaviour change):
- Length-like locals in command/cmd_ac.c, command/cmd_funcs.c,
pgp/gpg.c, tools/autocomplete.c, tools/parser.c and ui/mucwin.c
switched from int to size_t / glong (matching strlen /
g_utf8_strlen return type) so we no longer need an (int) cast and
loop counters / array sizes stay in their natural unsigned domain.
- g_timer_elapsed / GTimeSpan -> int casts in session.c, iq.c,
core.c, server_events.c, window.c.
- _win_print_wrapped: indent parameter and local curx/maxx switched
from size_t to int to match _win_indent / getcurx / getmaxx.
- Port casts (int -> unsigned short) at the libstrophe boundary in
connection.c and session.c, each preceded by
g_assert(port >= 0 && port <= UINT16_MAX) so the truncation is
documented at the call-site.
- curl_off_t / fread size_t results cast at usage in http_upload.c,
http_download.c, omemo/crypto.c.
- strtoul results cast to uint32_t in xmpp/omemo.c and omemo/omemo.c
where device/prekey IDs are genuinely 32-bit.
- config/color.c: fg/bg/palette indices switched to `short`
end-to-end (find_col, color_hash, find_closest_col,
_color_pair_cache_get, cache.pairs), so the ncurses init_pair
boundary needs at most one (short)i cast for the cache index. Also
TODO-noted: init_extended_pair is needed for >15-bit palettes.
- xmpp/avatar.c: float arithmetic explicitly casts its int operands.
- tests/functionaltests/proftest.c: read() result handling uses
size_t for the accumulator, _read_output returns ssize_t, and the
buffer-shift check happens before space subtraction so the
expression cannot underflow.
Real-risk guard clauses (the part that actually fixes bugs):
- src/ui/statusbar.c _tabs_width: `end > opened_tabs - 1` rewritten
as `end < opened_tabs` so opened_tabs == 0 no longer underflows.
- src/ui/statusbar.c _status_bar_draw_extended_tabs: the mirror
comparison rewritten as `end >= opened_tabs`.
- src/ui/statusbar.c status_bar_draw: replaced
`MAX(0, getmaxx - (int)_tabs_width)` with an explicit precheck
before subtraction.
- src/omemo/omemo.c prekey selection: prekey_index is now uint32_t
and randomized into an unsigned buffer, so modulo with prekeys_len
cannot yield a negative index for g_list_nth_data.
- src/omemo/crypto.c omemo_decrypt_func: PKCS#5/PKCS#7 unpadding
reads `plaintext[plaintext_len - 1]`, which would underflow on a
malformed empty ciphertext and read past the heap buffer. Reject
plaintext_len == 0 before the padding peek and validate the
padding byte against the buffer length before the unpad loop.
Initialise plaintext = NULL so the early `goto out` cannot free
uninitialised memory.
- src/ui/inputwin.c (4 mbrlen sites) and src/ui/window.c
_win_print_wrapped: mbrlen() returns 0 for the null wide
character. The existing checks rejected (size_t)-1 / -2 but
treated 0 as a valid step, so the surrounding loops would either
advance by SIZE_MAX (i += ch_len - 1) or spin in place
(word_pos += 0 forever). Add `|| ch_len == 0` to each guard;
inside the spell-check word-emission loop also fall back to a
one-byte advance.
- Defensive `len > 0 ? len - 1 : 0` prechecks at the strlen-based
g_strndup / loop sites in ui/console.c, plugins/c_api.c and
plugins/python_plugins.c.
Turn on the stricter conversion-safety warnings and wire in an optional
sanitizer build mode. Nothing is promoted to a hard error yet: the
warnings become part of the build output and can be cleaned up
incrementally without blocking development.
- Drop -Wno-sign-compare and add -Wsign-compare + -Wconversion to the
GCC-specific warnings loop (probed for compiler support).
- Add --enable-sanitizers flag that enables ASan + UBSan +
-fsanitize=unsigned-integer-overflow with -fno-sanitize-recover=all.
Off by default; intended for a dedicated CI job.
- Under PACKAGE_STATUS=development (-Werror), opt out of turning the
new conversion/sign-compare warnings into hard errors via
-Wno-error=sign-conversion/-conversion/-float-conversion/-sign-compare.
Existing codebase has ~240 sign-conversion, ~87 other narrowing and a
handful of sign-compare warnings (mostly inherited from upstream);
they stay visible but don't block development builds.
- Route glib/gio CFLAGS through -isystem so macros like
GPOINTER_TO_UINT in glibconfig.h don't generate noise.
Arch Linux ships a stripped ld-linux-x86-64.so.2, so Valgrind cannot
find the memcmp/memcpy/strlen symbols it must redirect and exits at
startup with "Fatal error at startup: a function redirection which is
mandatory for this platform-tool combination cannot be set up".
debuginfod is already installed in the image but was never wired up.
Set DEBUGINFOD_URLS to the official Arch debuginfod server so Valgrind
fetches the missing glibc debug symbols on demand and caches them.
Apply fixes, refactors and test additions requested by reviewer on PR #105.
Fixes:
- database: warn and notify user on duplicate archive_id instead of
silently debug-logging it (R05).
- database: add missing '[' in "[DB Migration]" log prefix (R06).
- xmpp/resource: NULL-out name/status after g_free to avoid double-free
via roster_list.c cleanup path (R09, R23).
- common: widen strtoi_range internal storage from int to long so that
values in (INT_MAX, LONG_MAX] are rejected as out-of-range instead of
being silently truncated on 64-bit platforms (R25).
Refactors:
- xmpp/message: extract _receive_omemo helper, removing three copies of
the OMEMO receive block in groupchat / MUC-PM / chat handlers (R04).
- omemo: flatten deeply nested device-list processing via guard-clause
continues (R11).
- tools/autocomplete: merge two nested ifs into a single && condition
(R13).
- ui/titlebar: extract _show_trust_indicator and inline _wprintw_withattr
wrapper, collapsing three near-identical trust-indicator blocks (R22).
- config/tlscerts: drop _checked_g_strdup wrapper; g_strdup is
NULL-safe per glib documentation (R19).
- ui/inputwin: use auto_gchar for spellcheck word instead of manual
g_free (R20).
- tools/editor: drop outdated "Deprecated synchronous" comment that
no longer matches the callback-based implementation (R28).
Tests:
- tests/command/cmd_ac: rename segfaults_when_empty ->
no_segfault_when_empty; expand cycling coverage to three files plus
backward SHIFT-TAB traversal (R16, R17).
- tests/common: add strtoi_range overflow/underflow and strtol-parsing
consistency tests (R25).
- tests/xmpp/jid: add test for '@' inside resourcepart per RFC 6122
section 2.4 (R26).
Misc:
- xmpp/omemo: change omemo_error_to_string return type from char* to
gchar* for glib consistency (R01).
- subprojects/libstrophe: point wrap-git at our fork at
git.jabber.space/devs/libstrophe-gh (R24).
- RELEASE_GUIDE: drop "Updating website" section referring to an
upstream site that is not ours (R18).
Inject system flags from /etc/makepkg.conf into the CI environment to
detect build collisions caused by Pikaur's configuration bug.
Pikaur's cascading logic causes flags from /etc/makepkg.conf to be
merged into the build environment. This creates collisions with flags
defined in the project's Makefile.am (e.g., duplicate -D_FORTIFY_SOURCE
definitions), which can cause builds to fail for users.
By exporting these flags in the CI environment, we ensure that any
code change that is sensitive to flag duplication will trigger a
failure in our Arch Linux CI matrix, preventing broken builds from
reaching users.
Implementation details:
- Detects Arch Linux via /etc/os-release.
- Uses a sed-based flattener to handle multi-line variables and
trailing backslashes in makepkg.conf.
- Exports the flags to the shell environment so that 'configure'
and 'make' inherit them naturally, maintaining parity with a
real Pikaur session.
Updated tooling builds supported XEP list from profanity.doap file
The profanity.doap file is a git submodule in the website repository.
It is used as an input to create the supported XEPs webpage.
3cd439d172Fixes: #2154
Signed-off-by: Benson Muite <benson_muite@emailplus.org>
We recently made the jid validation and creation functions more strict.
So this is a sideffect of it. We need to use `jid_fulljid_or_barejid`
instead of jidp->fulljid here since it now treats just the barejid not
as a fulljid.
Ref: 09757da5df
Ref: f251cc8bb3
Ref: fa64b135df
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
If we build Profanity without GTK we dont iterate the context so we will
never know when the editor exited. Calling `g_main_context_iteration()`
or finally switching to GMainLoop fixes this.
So far calling `tray_update()` does this for us in GTK builds.
So this went unnoticed.
The editor will inherit ignored signal handlers (SIGINT, SIGTSTP, SIGPIPE)
from us. Neovim uses libuv which seems to check SIGINT on startup to
determine whether the environment is interactive or not.
We now reset these signals to SIG_DFL in the child process before execvp.
Profanity uses readline which still competed for terminal input with the
editor. We only took care about other Profanity UI processes in our
previous commit. This led to misordered characters in the editor.
In my tests with vim everything worked fine and these bugs were
discovered when a user used neovim.
Fixes: https://github.com/profanity-im/profanity/issues/2148
Ref: 36ec2b0ae1
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
For key transport messages, Monal at least doesn't append the
authentication tag to the plaintext key contents as that only makes
sense if the key was used to encrypt something. This causes the key
length check to fail and show the
OMEMO message received but decryption failed.
error to the user which is confusing because there is no user-originated
message involved.
Skip the length check for key transport messages as profanity only uses
these to advance the ratchet and makes no use of the decrypted contents.
Signed-off-by: Karel Balej <balejk@matfyz.cz>
ASan and Valgrind both intercept memory allocations and management at
runtime. Running them simultaneously might lead to conflicts in memory
tracking.
This change ensures that during the Valgrind phase of the build matrix,
only Valgrind is responsible for memory analysis, avoiding redundant
overhead and ensuring more reliable results.
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Remove IDE specific entries, they belong in the global conf of the user.
Remove autotools specific files.
Ref: bc777c56b2
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Move from blocking fork/wait logic to nonblocking fork and
g_child_watch_add.
This ensures that the Profanity main loop continues to run while an
external editor is open. So we don't loose connection and react to
pings.
We change editor handling also in vcard and muc subject editing.
In the new implementation we are launching the editor and passing a
callback which we will use once the editor exited.
We use the recently added ui_susped() and ui_resume().
To not clutter the UI we need to check whether Profanity UI is suspended
and omit drawing in this case.
Fixes: https://github.com/profanity-im/profanity/issues/1888
Ref: 9b112904a9bc7250dc013d901187ca8622580d98
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
There was way too much repetition here.
This is in preparation for future changes regarding the editor.
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Commit 81f92f1fe introduced a selective update mechanism to
_accounts_save() to prevent overwrites when running multiple instances
by only modifying the specific account being saved.
Commit 5c484c fixed a problem with that commit regarding empty accounts.
It turns there was another bug introduced:
The new implementation only set the keys that existed in memory.
So removal of keys was not possible anymore.
Fixes: 81f92f1fed
Ref: 5c484c26ed
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Replace g_spawn_command_line_sync with g_spawn_sync with the
G_SPAWN_CHILD_INHERITS_STDIN flag.
This is actually needed to so that interactive commands can access the
terminal.
Otherwise they cannot ask the user for the passphrase.
Ref: f5787fb31f
Fixes: https://github.com/profanity-im/profanity/issues/2143
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
I think this bug happens when a user scrolls up a window, triggering
iq_mam_request_older() to load older history, in the case when the
database is empty for that contact.
The initial MAM fetch is triggered when the window is opened which
should get the latest history.
Fixes: https://github.com/profanity-im/profanity/issues/2142
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Allow building the latest libstrophe from upstream Git automatically when
-Dforce_fallback_for=libstrophe is passed.
Default to find and link against system-provided libstrophe stays the same.
Signed-off-by: Paul Fertser <fercerpav@gmail.com>
Use XSetIOErrorExitHandler on X11 to detect connection loss and
disable GTK features (tray, clipboard, notifications).
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Signed-off-by: Paul Fertser <fercerpav@gmail.com>
Co-authored-by: Paul Fertser <fercerpav@gmail.com>
Replace 'INSERT OR IGNORE' with 'INSERT ... ON CONFLICT(`archive_id`) DO
NOTHING RETURNING id'. This is only available since sqlite 3.35.0.
So deduplication only happens for `archive_id` and we don't silently
ignore other errors or constraints (like not null).
We can now detect if an insertion was skipped due to duplication by
checking the result of 'RETURNING id'.
We don't print out when we don't insert duplicated messages since this
will happen often and will be too noisy. So we match the behaviour of
what Dino is doing.
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Unescape any character following a backslash. This fixes autocompletion
for names with escaped spaces.
For example: `/msg Thor\ Odinson hello`
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
