devs/profanity
2
0
Fork 0
mirror of https://git.jabber.space/devs/cproof.git synced 2026-07-20 17:06:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(review): address PR #94 reviewer comments

Browse Source 
Blockers:
- SPDX/CProof copyright headers (7 files)
- stanza-id non-uniqueness: warn-only, don't skip (matches SQLite)
- fopen → open(O_EXCL|O_NOFOLLOW, 0600) + fdopen in export

Medium:
- log_info → log_warning for SQLite fallback
- remove prefs_save() auto-save from backend switch
- vtable null → log_warning in 4 dispatch functions
- log_debug → log_warning + null error handling in export
- rename failure → cons_show_error to user
- remove unused total_skipped variable
- LMC respects PREF_CORRECTION_ALLOW

Low nits:
- man page .SS fix + jabber.space URL
- profrc.example simplified
- show flatfile path on /logging flatfile
- EXPORT_PROGRESS_INTERVAL constant (magic 500)
- FF_META_PREFIX_ID/AID constants + FF_INDEX_STEP comment
- buffer overflow guard on close[1]/close[2]
- for-loop → memchr for JID resource scan
- BOM check extracted to ff_skip_bom() (DRY)
- index building extracted to _ff_maybe_index_line()
- O_APPEND+O_EXCL comment rewritten
- Makefile.am spaces → tabs
This commit is contained in:
jabber.developer2
2026-03-28 12:38:39 +03:00
parent 58002409ff
commit a96a4ad41e
12 changed files with 192 additions and 196 deletions
Download Patch File Download Diff File Expand all files Collapse all files

152
src/database_flatfile.c
View File

@@ -1,24 +1,12 @@
// SPDX-License-Identifier: GPL-3.0-or-later
//
// This file is part of CProof.
// See LICENSE for the full GPLv3 text and the special OpenSSL linking exception.
/*
* database_flatfile.c
* vim: expandtab:ts=4:sts=4:sw=4
*
* Copyright (C) 2026 Profanity Contributors
*
* This file is part of Profanity.
*
* Profanity is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Profanity is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Profanity. If not, see <https://www.gnu.org/licenses/>.
*
* Flat-file database backend: init/close, message write, message read,
* LMC correction logic, and the backend vtable.
*
@@ -110,10 +98,10 @@ _ff_cache_line_ids(ff_contact_state_t* state, const char* line)
if (parts) {
for (int i = 0; parts[i]; i++) {
if (g_str_has_prefix(parts[i], "id:") && !stanza_id) {
stanza_id = ff_unescape_meta_value(parts[i] + 3);
} else if (g_str_has_prefix(parts[i], "aid:") && !archive_id) {
archive_id = ff_unescape_meta_value(parts[i] + 4);
if (g_str_has_prefix(parts[i], FF_META_PREFIX_ID) && !stanza_id) {
stanza_id = ff_unescape_meta_value(parts[i] + strlen(FF_META_PREFIX_ID));
} else if (g_str_has_prefix(parts[i], FF_META_PREFIX_AID) && !archive_id) {
archive_id = ff_unescape_meta_value(parts[i] + strlen(FF_META_PREFIX_AID));
}
}
g_strfreev(parts);
@@ -121,17 +109,12 @@ _ff_cache_line_ids(ff_contact_state_t* state, const char* line)
// Extract from_jid (needed for stanza_senders map): after "] " before "/" or ": "
char* from_jid = NULL;
if (stanza_id && *(close + 1) == ' ') {
if (stanza_id && close[1] != '\0' && close[1] == ' ' && close[2] != '\0') {
const char* sender_start = close + 2;
const char* colonspace = ff_find_unescaped_colonspace(sender_start);
if (colonspace) {
const char* jid_end = colonspace;
for (const char* p = sender_start; p < colonspace; p++) {
if (*p == '/') {
jid_end = p;
break;
}
}
const char* slash = memchr(sender_start, '/', colonspace - sender_start);
const char* jid_end = slash ? slash : colonspace;
from_jid = g_strndup(sender_start, jid_end - sender_start);
}
}
@@ -150,6 +133,36 @@ _ff_cache_line_ids(ff_contact_state_t* state, const char* line)
}
}
// Attempt to add an index entry for the current line.
// Called every FF_INDEX_STEP messages during build/extend.
static void
_ff_maybe_index_line(ff_contact_state_t* state, const char* buf, off_t pos)
{
char* space = strchr(buf, ' ');
if (!space)
return;
char* ts_str = g_strndup(buf, space - buf);
GDateTime* dt = g_date_time_new_from_iso8601(ts_str, NULL);
if (!dt) {
log_warning("flatfile: unparseable timestamp in %s at offset %ld: %s",
state->filepath, (long)pos, ts_str);
g_free(ts_str);
return;
}
g_free(ts_str);
if (state->n_entries >= state->cap_entries) {
state->cap_entries = state->cap_entries ? state->cap_entries * 2 : 64;
state->entries = g_realloc(state->entries,
state->cap_entries * sizeof(ff_index_entry_t));
}
state->entries[state->n_entries].byte_offset = pos;
state->entries[state->n_entries].timestamp_epoch = g_date_time_to_unix(dt);
state->n_entries++;
g_date_time_unref(dt);
}
static gboolean
_ff_state_build(ff_contact_state_t* state)
{
@@ -157,15 +170,7 @@ _ff_state_build(ff_contact_state_t* state)
if (!fp)
return FALSE;
int c1 = fgetc(fp);
int c2 = fgetc(fp);
int c3 = fgetc(fp);
if (c1 == 0xEF && c2 == 0xBB && c3 == 0xBF) {
state->bom_len = 3;
} else {
state->bom_len = 0;
fseek(fp, 0, SEEK_SET);
}
state->bom_len = ff_skip_bom(fp);
state->n_entries = 0;
state->total_lines = 0;
@@ -197,23 +202,7 @@ _ff_state_build(ff_contact_state_t* state)
_ff_cache_line_ids(state, buf);
if (msg_count % FF_INDEX_STEP == 0) {
char* space = strchr(buf, ' ');
if (space) {
char* ts_str = g_strndup(buf, space - buf);
GDateTime* dt = g_date_time_new_from_iso8601(ts_str, NULL);
if (dt) {
if (state->n_entries >= state->cap_entries) {
state->cap_entries = state->cap_entries ? state->cap_entries * 2 : 64;
state->entries = g_realloc(state->entries,
state->cap_entries * sizeof(ff_index_entry_t));
}
state->entries[state->n_entries].byte_offset = pos;
state->entries[state->n_entries].timestamp_epoch = g_date_time_to_unix(dt);
state->n_entries++;
g_date_time_unref(dt);
}
g_free(ts_str);
}
_ff_maybe_index_line(state, buf, pos);
}
msg_count++;
free(buf);
@@ -284,23 +273,7 @@ _ff_state_extend(ff_contact_state_t* state)
_ff_cache_line_ids(state, buf);
if (new_count % FF_INDEX_STEP == 0) {
char* space = strchr(buf, ' ');
if (space) {
char* ts_str = g_strndup(buf, space - buf);
GDateTime* dt = g_date_time_new_from_iso8601(ts_str, NULL);
if (dt) {
if (state->n_entries >= state->cap_entries) {
state->cap_entries = state->cap_entries ? state->cap_entries * 2 : 64;
state->entries = g_realloc(state->entries,
state->cap_entries * sizeof(ff_index_entry_t));
}
state->entries[state->n_entries].byte_offset = pos;
state->entries[state->n_entries].timestamp_epoch = g_date_time_to_unix(dt);
state->n_entries++;
g_date_time_unref(dt);
}
g_free(ts_str);
}
_ff_maybe_index_line(state, buf, pos);
}
new_count++;
free(buf);
@@ -445,9 +418,10 @@ _ff_add_message(const char* type, const char* stanza_id, const char* archive_id,
return;
}
// Open the file with O_NOFOLLOW to prevent symlink attacks,
// and O_APPEND for safe concurrent appends.
// Open the file with O_NOFOLLOW to prevent symlink attacks.
// Try O_CREAT|O_EXCL first to detect new files atomically (no TOCTOU).
// O_APPEND is included for consistency — it has no effect on a new file
// but ensures append semantics if the fd is inherited after the fallback.
int fd = open(log_path, O_WRONLY | O_APPEND | O_CREAT | O_EXCL | O_NOFOLLOW,
S_IRUSR | S_IWUSR);
gboolean is_new = (fd >= 0);
@@ -601,14 +575,15 @@ _flatfile_add_incoming(ProfMessage* message)
} else {
contact = message->from_jid->barejid;
}
// MAM dedup: skip if this stanza-id already exists in the log (XEP-0313 replay)
// Stanza-id duplicate warning (non-blocking).
// XEP-0359 stanza-ids SHOULD be unique per server, but older clients
// (Pidgin, Adium, old Profanity) may use incremental IDs that servers
// echo back, causing false positives. Log but do NOT skip — matches
// SQLite backend behavior. See https://github.com/profanity-im/profanity/issues/1899
if (message->stanzaid && !message->is_mam) {
if (_ff_has_archive_id(contact, message->stanzaid)) {
log_error("flatfile: duplicate stanza-id '%s' from %s, skipping",
message->stanzaid, message->from_jid->barejid);
cons_show_error("Got a message with duplicate (server-generated) stanza-id from %s.",
message->from_jid->fulljid);
return;
log_warning("flatfile: duplicate stanza-id '%s' from %s (may be non-unique client ID)",
message->stanzaid, message->from_jid->barejid);
}
}
@@ -903,9 +878,22 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta
ff_parsed_line_t* oldest = all_parsed->data;
state->cursor_offset = oldest->file_offset;
// Apply LMC corrections and build ProfMessage list
// Apply LMC corrections (if enabled) and build ProfMessage list
GSList* pre_result = NULL;
_ff_apply_lmc(all_parsed, &pre_result);
if (prefs_get_boolean(PREF_CORRECTION_ALLOW)) {
_ff_apply_lmc(all_parsed, &pre_result);
} else {
// LMC disabled — convert all lines to ProfMessage without correction
for (GSList* cur = all_parsed; cur; cur = cur->next) {
ff_parsed_line_t* pl = cur->data;
if (pl) {
ProfMessage* msg = ff_parsed_to_profmessage(pl);
if (msg)
pre_result = g_slist_prepend(pre_result, msg);
}
}
pre_result = g_slist_reverse(pre_result);
}
g_slist_free_full(all_parsed, (GDestroyNotify)ff_parsed_line_free);
if (!pre_result)