diff --git a/Command.cc b/Command.cc index 3742ce2..a6e5cd6 100644 --- a/Command.cc +++ b/Command.cc @@ -1169,7 +1169,7 @@ search_count(const Buffer &buf, bool regex, const std::string &q, const RegexMat else c.reached = true; ++c.total; - p += q.size(); + p += 1; // overlapping, as Next/Previous visit them } if (p != std::string_view::npos && p >= cur_off) c.reached = true; @@ -1238,7 +1238,9 @@ run_search(Editor &ed, Buffer &buf, bool regex, SearchStep step, bool incrementa x = ed.SearchOrigX(); } if (step == SearchStep::Next && have_cur) { - const std::size_t len = std::max(ed.SearchMatchLen(), 1); + // Plain search visits overlapping matches ("aa" in "aaa" at 0 and 1), + // as the backward search does; a regex continues after its match. + const std::size_t len = regex ? std::max(ed.SearchMatchLen(), 1) : 1; x += len; if (x > line_text_view(buf, y).size() && y + 1 < buf.Nrows()) { ++y; diff --git a/Editor.cc b/Editor.cc index ad532ac..1ae79e6 100644 --- a/Editor.cc +++ b/Editor.cc @@ -397,6 +397,7 @@ Editor::CancelRecoveryPrompt() pending_recovery_replay_err_.clear(); std::string().swap(pending_recovery_content_); pending_recovery_valid_bytes_ = 0; + pending_recovery_swap_size_ = 0; pending_recovery_have_content_ = false; } @@ -413,6 +414,7 @@ Editor::ResolveRecoveryPrompt(const bool yes) std::string content = std::move(pending_recovery_content_); const bool have_content = pending_recovery_have_content_; std::uint64_t valid_bytes = pending_recovery_valid_bytes_; + const std::uint64_t swap_size_then = pending_recovery_swap_size_; CancelRecoveryPrompt(); std::string err; @@ -427,8 +429,13 @@ Editor::ResolveRecoveryPrompt(const bool yes) SetStatus("Recovery failed: no buffer"); return false; } - if (have_content) { - // The prompt already replayed the journal; install that. + std::uint64_t swap_size_now = 0; + try { + swap_size_now = std::filesystem::file_size(swp); + } catch (...) { + } + if (have_content && swap_size_now == swap_size_then) { + // The prompt already replayed the journal (unchanged since); install that. b->replace_all_bytes(content); std::string().swap(content); } else { @@ -607,6 +614,11 @@ Editor::process_pending_opens_() pending_recovery_content_ = std::move(rec); pending_recovery_valid_bytes_ = valid; pending_recovery_have_content_ = true; + try { + pending_recovery_swap_size_ = std::filesystem::file_size(swp); + } catch (...) { + pending_recovery_have_content_ = false; // replay on "y" + } StartPrompt(PromptKind::Confirm, "Recover", ""); SetStatus("Recover swap edits for " + req.path + "? (y/n, C-g cancel)"); return opened_any; diff --git a/Editor.h b/Editor.h index 3f70dc3..805881a 100644 --- a/Editor.h +++ b/Editor.h @@ -721,6 +721,7 @@ private: // produced it: "y" installs it instead of replaying the journal again. std::string pending_recovery_content_; std::uint64_t pending_recovery_valid_bytes_ = 0; + std::uint64_t pending_recovery_swap_size_ = 0; // the journal's size then bool pending_recovery_have_content_ = false; // GUI-only state (safe no-op in terminal builds) diff --git a/RegexEngine.cc b/RegexEngine.cc index c1fa32f..6b911b7 100644 --- a/RegexEngine.cc +++ b/RegexEngine.cc @@ -38,24 +38,20 @@ append_format(std::string &out, std::string_view fmt, std::string_view subject, out.append(subject.substr(me)); ++i; } else if (n >= '0' && n <= '9') { - // $nn if that group exists, else $n; $0 is not a reference. - std::size_t idx = static_cast(n - '0'); - std::size_t used = 1; - if (i + 2 < fmt.size() && fmt[i + 2] >= '0' && fmt[i + 2] <= '9') { - const std::size_t two = idx * 10 + static_cast(fmt[i + 2] - '0'); - if (two >= 1 && two < ngroups) { - idx = two; - used = 2; - } - } - if (idx == 0 || idx >= ngroups) { - out.push_back(c); // not a group reference: literal - continue; + // As libstdc++'s match_results::format: up to two digits, read + // greedily; $0 is the whole match; a group the pattern does not + // have expands to nothing. + std::size_t idx = static_cast(n - '0'); + ++i; + if (i + 1 < fmt.size() && fmt[i + 1] >= '0' && fmt[i + 1] <= '9') { + idx = idx * 10 + static_cast(fmt[i + 1] - '0'); + ++i; } std::size_t b = 0, e = 0; - if (group(idx, b, e)) + if (idx == 0) + out.append(subject.substr(ms, me - ms)); + else if (idx < ngroups && group(idx, b, e)) out.append(subject.substr(b, e - b)); - i += used; } else { out.push_back(c); } @@ -121,8 +117,16 @@ Regex::Compile(std::string_view pattern, std::string &err) int errcode = 0; PCRE2_SIZE erroff = 0; static const char empty = '\0'; + // As in ECMAScript (std::regex): '.' matches neither CR nor LF, and '$' + // matches only at the end (PCRE2's default newline is LF alone, so '.' + // matched the CR of a CRLF line and replace-all deleted it). + pcre2_compile_context *cctx = pcre2_compile_context_create(nullptr); + if (cctx) + pcre2_set_newline(cctx, PCRE2_NEWLINE_ANYCRLF); impl->code = pcre2_compile(reinterpret_cast(pattern.data() ? pattern.data() : &empty), - pattern.size(), 0, &errcode, &erroff, nullptr); + pattern.size(), PCRE2_DOLLAR_ENDONLY, &errcode, &erroff, cctx); + if (cctx) + pcre2_compile_context_free(cctx); if (!impl->code) { PCRE2_UCHAR msg[256]; pcre2_get_error_message(errcode, msg, sizeof(msg)); diff --git a/RegexEngine.h b/RegexEngine.h index 0c18b19..a40ab85 100644 --- a/RegexEngine.h +++ b/RegexEngine.h @@ -11,8 +11,11 @@ * PCRE2 additionally has lookbehind, possessive quantifiers and the like. * * Replacement strings use ECMAScript syntax with either engine, as - * std::regex_replace reads it: $& (the match), $1..$99 (groups), $` (the - * text since the previous match), $' (the rest of the line), $$ (a dollar). + * std::regex_replace (libstdc++) reads it: $& or $0 (the match), $1..$99 + * (groups, two digits read greedily; a group the pattern lacks expands to + * nothing), $` (the text since the previous match), $' (the rest of the + * line), $$ (a dollar sign). As in ECMAScript, '.' matches neither CR nor + * LF and '$' matches only at the end of the line's text. */ #pragma once diff --git a/Swap.cc b/Swap.cc index 08419cf..c1e6ddc 100644 --- a/Swap.cc +++ b/Swap.cc @@ -845,6 +845,7 @@ SwapManager::open_ctx(JournalCtx &ctx, const std::string &path, std::string &err if (st.st_size == 0) { ctx.header_ok = write_header(fd, ctx); ctx.approx_size_bytes = ctx.header_ok ? 64 : 0; + ctx.header_version = VERSION; if (!ctx.header_ok) { err = "Failed to write swap file header: " + path; // Drop any partial header and close, so the next attempt starts a @@ -856,6 +857,20 @@ SwapManager::open_ctx(JournalCtx &ctx, const std::string &path, std::string &err } else { ctx.header_ok = true; // stage 1: trust existing header ctx.approx_size_bytes = static_cast(st.st_size); + // An existing journal (continued after recovery) may be version 1, + // which must not receive chunked checkpoints (see process_one). + // (The journal fd is write-only; read through another.) + std::uint8_t ver[4] = {0, 0, 0, 0}; + ctx.header_version = 0; + const int rfd = kte::syscall::Open(path.c_str(), O_RDONLY | O_CLOEXEC, 0); + if (rfd >= 0) { + if (::pread(rfd, ver, sizeof(ver), 8) == static_cast(sizeof(ver))) + ctx.header_version = static_cast(ver[0]) | + (static_cast(ver[1]) << 8) | + (static_cast(ver[2]) << 16) | + (static_cast(ver[3]) << 24); + kte::syscall::Close(rfd); + } } return ctx.header_ok; } @@ -1023,8 +1038,9 @@ SwapManager::compact_to_checkpoint(JournalCtx &ctx, const RecordFrames &chkpt_re const int fl = ::fcntl(tfd, F_GETFL); if (fl >= 0) (void) ::fcntl(tfd, F_SETFL, fl | O_APPEND); - ctx.fd = tfd; - ctx.header_ok = true; + ctx.fd = tfd; + ctx.header_ok = true; + ctx.header_version = VERSION; // Best-effort: fsync parent dir to persist the rename. try { @@ -1556,8 +1572,14 @@ SwapManager::process_one(const Pending &p) // For a large buffer the thresholds scale with its size, so the cost // of compacting stays proportional to what was journaled since. const std::uint64_t compact_at = std::max(compact_bytes, 2 * p.chkpt.size()); - if (p.type == SwapRecType::CHKPT && compact_bytes > 0 && - ctxp->approx_size_bytes + rec.total >= compact_at) { + // A chunked checkpoint may not be appended to a version 1 journal + // (an older kte would skip it and misapply what follows): write it + // as a compacted version 2 journal instead. + const bool chunked = p.type == SwapRecType::CHKPT && p.chkpt.size() > kMaxRecordPayload - 5; + const bool v1_journal = ctxp->header_version != 0 && ctxp->header_version < 2; + if (p.type == SwapRecType::CHKPT && + ((chunked && v1_journal) || + (compact_bytes > 0 && ctxp->approx_size_bytes + rec.total >= compact_at))) { std::string compact_err; if (compact_to_checkpoint(*ctxp, rec, compact_err)) { std::lock_guard lg(mtx_); @@ -1566,7 +1588,10 @@ SwapManager::process_one(const Pending &p) circuit_breaker_.RecordSuccess(); return; } - report_error(compact_err, p.buf); // fall back to appending + report_error(compact_err, p.buf); + if (chunked && v1_journal) + return; // not appended; the records around it stay valid + // otherwise fall back to appending } // Remember where this record starts so a partial write can be undone; diff --git a/Swap.h b/Swap.h index 7df7425..61ce73a 100644 --- a/Swap.h +++ b/Swap.h @@ -237,6 +237,7 @@ private: bool gap{false}; bool gap_notified{false}; // the user was told about the current gap bool chkpt_queued{false}; // a checkpoint is queued and not yet being written + std::uint32_t header_version{0}; // of the open journal file (0: none yet) bool lockout_notified{false}; // told about the lock; kept across ResetJournal std::uint64_t gap_chkpt_request_ns{0}; bool gap_unfixable_reported{false}; // gap on a buffer too large to checkpoint diff --git a/docs/audits/claude-findings-20260923.md b/docs/audits/claude-findings-20260923.md index e98dcb4..75c5115 100644 --- a/docs/audits/claude-findings-20260923.md +++ b/docs/audits/claude-findings-20260923.md @@ -558,6 +558,15 @@ before the fix (unless noted). 1.55 s -> 0.10 s per keystroke. - Only one checkpoint per journal is queued at a time; whole-file edits in a row on a large buffer each queued a full copy of it. +- Review of this work found, and these are fixed: with PCRE2, '.' + matched the CR of CRLF lines (regex replace-all deleted it) — PCRE2 + now uses the ANYCRLF newline convention and $ matches only at the end; + $0, out-of-range $n and $nn in replacements now follow libstdc++; + plain search visits overlapping matches in both directions (the count + could read "2/1"); the recovery prompt replays again if the journal + changed while it was open; a version 1 journal continued after + recovery gets a chunked checkpoint only by being rewritten as + version 2. **Known limitations (not fixed)** - Without PCRE2 (std::regex builds), catastrophic regex backtracking diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index 8f66e35..c1a0864 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -17,6 +17,7 @@ #include "syntax/LanguageHighlighter.h" #include +#include #include #include #include @@ -2184,16 +2185,20 @@ TEST(Swap_DeferredBaseCrc) } -// kte::Regex replacements follow std::regex_replace (ECMAScript), whichever -// engine is built in. (References to groups the pattern does not have, like -// $3 with two groups, are implementation-defined and left out.) +// kte::Regex replacements follow std::regex_replace (ECMAScript as libstdc++ +// reads it), whichever engine is built in. TEST(RegexEngine_ReplaceMatchesStdRegex) { const char *subjects[] = {"", "baaac", "one two three", "a-b-c", "xyz", "aaa", " lead", "tail "}; const std::pair cases[] = { {"a*", "X"}, {"a", "[$&]"}, {"(\\w+)", "<$1>"}, {"(o)(n)?", "$2$1"}, {"-", "$`|$'"}, {"\\s+", "$$"}, {"^", ">"}, {"$", "<"}, {"b|", "_"}, {"\\b", "|"}, - {"([a-z])([a-z])", "$2$1"}, {"(a)(x)?", "[$2]"} + {"([a-z])([a-z])", "$2$1"}, {"(a)(x)?", "[$2]"}, +#if defined(__GLIBCXX__) + // Read as libstdc++ reads them (libc++ differs): $0 is the match, + // two digits are read greedily, missing groups expand to nothing. + {"b", "[$0]"}, {"(b?)", "<$10>"}, {"(a)", "[$2]"}, {"(a)(b)?", "$12"}, +#endif }; for (const char *subj: subjects) { for (const auto &[pat, fmt]: cases) { @@ -2228,3 +2233,94 @@ TEST(RegexEngine_CatastrophicPatternStops) ASSERT_TRUE(h.EditorRef().Status().find("too complex") != std::string::npos); ASSERT_TRUE(h.Exec(CommandId::Refresh)); } + + +// '.' does not match the CR of a CRLF line (PCRE2's default newline +// convention let it, and regex replace-all deleted the CR). +TEST(RegexEngine_DotDoesNotMatchCR) +{ + TestHarness h; + h.Buf().insert_text(0, 0, "abc\r\ndef\r\n"); + regex_replace_all(h, "b.*", "X"); + ASSERT_EQ(h.Text(), std::string("aX\r\ndef\r\n")); + kte::Regex rx; + std::string err; + ASSERT_TRUE(rx.Compile("c$", err)); + std::size_t pos = 0, len = 0; + ASSERT_TRUE(!rx.Search("abc\r", 0, pos, len)); // '$' only at the very end + ASSERT_TRUE(rx.Search("abc", 0, pos, len)); +} + + +// Plain search visits overlapping matches in both directions, and the count +// agrees (Previous found the overlapping one while Next and the count +// skipped it: "2/1"). +TEST(Search_OverlappingMatchesConsistent) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + h.Buf().insert_text(0, 0, "aaa"); + h.Buf().SetCursor(0, 0); + ASSERT_TRUE(h.Exec(CommandId::FindStart)); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "aa")); + ASSERT_EQ(h.Buf().Curx(), (std::size_t) 0); + ASSERT_TRUE(ed.Status().find("1/2") != std::string::npos); + ASSERT_TRUE(h.Exec(CommandId::MoveRight)); + ASSERT_EQ(h.Buf().Curx(), (std::size_t) 1); + ASSERT_TRUE(ed.Status().find("2/2") != std::string::npos); + ASSERT_TRUE(h.Exec(CommandId::MoveLeft)); + ASSERT_EQ(h.Buf().Curx(), (std::size_t) 0); + ASSERT_TRUE(h.Exec(CommandId::MoveLeft)); // wraps + ASSERT_EQ(h.Buf().Curx(), (std::size_t) 1); + ASSERT_TRUE(ed.Status().find("2/2") != std::string::npos); + ASSERT_TRUE(h.Exec(CommandId::Refresh)); +} + + +// A version 1 journal (continued after recovery) never receives chunked +// checkpoint records, which an older kte would skip: the checkpoint is +// written as a compacted version 2 journal instead. +TEST(Swap_V1JournalGetsNoChunkedRecords) +{ + TempDir d("v1_chunked"); + const std::string path = (d.path / "big.txt").string(); + const std::string text = big_text(); + { + std::ofstream o(path, std::ios::binary); + o << text; + } + Buffer b; + std::string err; + ASSERT_TRUE(b.OpenFromFile(path, err)); + const std::string swp = kte::SwapManager::ComputeSwapPathForTests(b); + { + // A bare version 1 header (no base identity recorded). + std::string hdr(64, '\0'); + std::memcpy(hdr.data(), "KTE_SWP", 8); + hdr[8] = 1; + std::ofstream o(swp, std::ios::binary | std::ios::trunc); + o << hdr; + } + kte::SwapManager sm; + sm.Attach(&b); + b.SetSwapRecorder(sm.RecorderFor(&b)); + b.insert_text(0, 0, "X"); + sm.Flush(&b); + sm.Checkpoint(&b); + sm.Flush(&b); + b.insert_text(0, 1, "Y"); + sm.Flush(&b); + const std::string j = slurp(swp); + ASSERT_EQ(static_cast(j[8]), 2u); // rewritten as version 2 + const std::string copy = (d.path / "copy.swp").string(); + { + std::ofstream o(copy, std::ios::binary | std::ios::trunc); + o << j; + } + Buffer r; + ASSERT_TRUE(r.OpenFromFile(path, err)); + ASSERT_TRUE(kte::SwapManager::ReplayFile(r, copy, err)); + ASSERT_TRUE(r.BytesForTests() == b.BytesForTests()); + b.SetSwapRecorder(nullptr); + sm.Detach(&b, true); +}