From 5534846a5f63e72c818c690731e3e500abcad20d Mon Sep 17 00:00:00 2001 From: Anne Jan Brouwer Date: Wed, 28 Oct 2026 10:19:54 +0100 Subject: [PATCH 1/4] Content search preview: never half a surrogate pair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 200-character window around a match could start or end inside a surrogate pair (an emoji, CJK Extension B), leaving a lone surrogate next to the "…". A window that starts on a low surrogate takes the high one before it, and one that ends on a high surrogate takes the low one after. From Henk's review of #2075. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJygpMUf461yZgfAqwMJ6d Signed-off-by: Anne Jan Brouwer --- CHANGELOG.md | 2 +- src/nativegrep.cpp | 15 +++++++++++---- src/nativegrep.h | 3 ++- tests/auto/util/tst_util.cpp | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 045790cf8..a0e119b64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,7 +113,7 @@ umbrella [#908](https://github.com/IJHack/QtPass/issues/908)). - Bounded and non-blocking reads where store content or a pasted value could cost QtPass: an OTP configuration over 4096 characters is refused before it is parsed; content search keeps at most 200 characters of a - matching line; a `.gpg-id.sig` must be a regular file of at most 64 KiB + matching line, around the match and never half an emoji; a `.gpg-id.sig` must be a regular file of at most 64 KiB before `gpg` is given it (a FIFO blocked QtPass); a key file is opened without blocking on a FIFO; and the key generator refuses a name or passphrase with a line break, which would have become a batch directive. diff --git a/src/nativegrep.cpp b/src/nativegrep.cpp index 76fcbe1f8..6adc39d8e 100644 --- a/src/nativegrep.cpp +++ b/src/nativegrep.cpp @@ -77,12 +77,19 @@ auto NativeGrep::matchFile(const QProcessEnvironment &env, // it; "…" marks what was left out on either side. const qsizetype at = qMax(0, m.capturedStart() - (t.data() - line.data())); - const qsizetype start = qBound(0, at - kMaxPreviewLength / 4, - t.size() - kMaxPreviewLength); - QString preview = t.mid(start, kMaxPreviewLength).toString(); + qsizetype start = qBound(0, at - kMaxPreviewLength / 4, + t.size() - kMaxPreviewLength); + qsizetype end = start + kMaxPreviewLength; + // Never half a surrogate pair (an emoji, CJK Extension B) at either + // edge: the window takes the whole character. + if (start > 0 && t[start].isLowSurrogate()) + --start; + if (end < t.size() && t[end - 1].isHighSurrogate()) + ++end; + QString preview = t.sliced(start, end - start).toString(); if (start > 0) preview.prepend(QChar(0x2026)); - if (start + kMaxPreviewLength < t.size()) + if (end < t.size()) preview.append(QChar(0x2026)); matches << preview; } diff --git a/src/nativegrep.h b/src/nativegrep.h index 5600fd7a9..62f0e8d9f 100644 --- a/src/nativegrep.h +++ b/src/nativegrep.h @@ -54,7 +54,8 @@ class NativeGrep : public QObject { * A result shows where the pattern matched, not the line; a single line * can be most of an entry's 64 MiB plaintext, and 10000 such copies would * not fit in memory. A longer line is shown as a window of this many - * characters around the match, "…" where the line goes on. + * characters around the match (one more to keep a surrogate pair whole), + * "…" where the line goes on. */ static constexpr qsizetype kMaxPreviewLength = 200; diff --git a/tests/auto/util/tst_util.cpp b/tests/auto/util/tst_util.cpp index 0cf6affcf..66ccea10d 100644 --- a/tests/auto/util/tst_util.cpp +++ b/tests/auto/util/tst_util.cpp @@ -2967,6 +2967,38 @@ void tst_util::grepTakesABoundedNumberOfMatches() { qPrintable(far.first())); QVERIFY(far.first().startsWith(QChar(0x2026))); QVERIFY(far.first().endsWith(QChar(0x2026))); + + // Characters outside the BMP on both sides, the window's edges falling + // inside a surrogate pair: the preview keeps every pair whole. + const QString emoji = QString::fromUtf8("\xF0\x9F\x98\x80"); + // The "x" puts the match an odd number of code units from the pairs + // before it, so the window starts inside one and ends inside another. + const QString wide = QStringLiteral("ab") + emoji.repeated(1000) + + QStringLiteral("xhit") + emoji.repeated(1000); + const QString plain = tmp.filePath(QStringLiteral("wide.txt")); + { + QFile f(plain); + QVERIFY(f.open(QIODevice::WriteOnly)); + f.write(wide.toUtf8() + '\n'); + } + QVERIFY(script.open(QIODevice::WriteOnly | QIODevice::Truncate)); + script.write("#!/bin/sh\ncat '" + QFile::encodeName(plain) + "'\n"); + script.close(); + const QStringList pairs = NativeGrep::matchFile( + env, longGpg, QDir(store).filePath("e0000.gpg"), rx); + QCOMPARE(pairs.size(), 1); + const QString shown = pairs.first(); + QVERIFY2(shown.contains(QStringLiteral("hit")), qPrintable(shown)); + for (qsizetype i = 0; i < shown.size(); ++i) { + if (shown.at(i).isHighSurrogate()) { + QVERIFY2( + i + 1 < shown.size() && shown.at(i + 1).isLowSurrogate(), + qPrintable(QStringLiteral("lone high surrogate at %1").arg(i))); + } else if (shown.at(i).isLowSurrogate()) { + QVERIFY2(i > 0 && shown.at(i - 1).isHighSurrogate(), + qPrintable(QStringLiteral("lone low surrogate at %1").arg(i))); + } + } } // Exactly the budget, every entry whole: complete, no marker anywhere. From 6be12948f5005db2b570bee36b4163c0c14de54b Mon Sep 17 00:00:00 2001 From: Anne Jan Brouwer Date: Wed, 28 Oct 2026 10:33:54 +0100 Subject: [PATCH 2/4] Content search preview: say up to two more, rewrap the changelog line Henk's review of #2081: both edges can adjust, so the window can be two code units longer, not one; and the CHANGELOG line is wrapped at the width of its neighbours again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJygpMUf461yZgfAqwMJ6d Signed-off-by: Anne Jan Brouwer --- CHANGELOG.md | 9 +++++---- src/nativegrep.h | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0e119b64..48f9c1122 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,10 +113,11 @@ umbrella [#908](https://github.com/IJHack/QtPass/issues/908)). - Bounded and non-blocking reads where store content or a pasted value could cost QtPass: an OTP configuration over 4096 characters is refused before it is parsed; content search keeps at most 200 characters of a - matching line, around the match and never half an emoji; a `.gpg-id.sig` must be a regular file of at most 64 KiB - before `gpg` is given it (a FIFO blocked QtPass); a key file is opened - without blocking on a FIFO; and the key generator refuses a name or - passphrase with a line break, which would have become a batch directive. + matching line, around the match and never half an emoji; a + `.gpg-id.sig` must be a regular file of at most 64 KiB before `gpg` is + given it (a FIFO blocked QtPass); a key file is opened without blocking + on a FIFO; and the key generator refuses a name or passphrase with a + line break, which would have become a batch directive. - An entry that decompresses into millions of lines no longer exhausts memory below the 64 MiB plaintext limit. Showing one read every line into a list of its own (gigabytes for an entry of newlines) and made a row of diff --git a/src/nativegrep.h b/src/nativegrep.h index 62f0e8d9f..8d0ce21d7 100644 --- a/src/nativegrep.h +++ b/src/nativegrep.h @@ -54,8 +54,8 @@ class NativeGrep : public QObject { * A result shows where the pattern matched, not the line; a single line * can be most of an entry's 64 MiB plaintext, and 10000 such copies would * not fit in memory. A longer line is shown as a window of this many - * characters around the match (one more to keep a surrogate pair whole), - * "…" where the line goes on. + * characters around the match (up to two more, one per edge, to keep a + * surrogate pair whole), "…" where the line goes on. */ static constexpr qsizetype kMaxPreviewLength = 200; From 4d6a0d38c839c714a490101e7fd8df991e83ce7c Mon Sep 17 00:00:00 2001 From: Anne Jan Brouwer Date: Wed, 28 Oct 2026 11:07:50 +0100 Subject: [PATCH 3/4] Content search preview: say code units, claim only surrogate pairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on #2081: kMaxPreviewLength counts UTF-16 code units, not characters; what is kept whole is a surrogate pair, not every emoji (a ZWJ sequence or a flag can still be cut between its code points), and the CHANGELOG says so; the test also checks the expanded size (kMaxPreviewLength + 4: one code unit and one "…" per edge). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJygpMUf461yZgfAqwMJ6d Signed-off-by: Anne Jan Brouwer --- CHANGELOG.md | 2 +- src/nativegrep.h | 10 ++++++---- tests/auto/util/tst_util.cpp | 3 +++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48f9c1122..e08820529 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,7 +113,7 @@ umbrella [#908](https://github.com/IJHack/QtPass/issues/908)). - Bounded and non-blocking reads where store content or a pasted value could cost QtPass: an OTP configuration over 4096 characters is refused before it is parsed; content search keeps at most 200 characters of a - matching line, around the match and never half an emoji; a + matching line, around the match and never half a surrogate pair; a `.gpg-id.sig` must be a regular file of at most 64 KiB before `gpg` is given it (a FIFO blocked QtPass); a key file is opened without blocking on a FIFO; and the key generator refuses a name or passphrase with a diff --git a/src/nativegrep.h b/src/nativegrep.h index 8d0ce21d7..50cab67fa 100644 --- a/src/nativegrep.h +++ b/src/nativegrep.h @@ -49,13 +49,15 @@ class NativeGrep : public QObject { static constexpr int kMaxMatches = 10000; /** - * @brief Longest preview of a matching line, in characters. + * @brief Longest preview of a matching line, in UTF-16 code units. * * A result shows where the pattern matched, not the line; a single line * can be most of an entry's 64 MiB plaintext, and 10000 such copies would - * not fit in memory. A longer line is shown as a window of this many - * characters around the match (up to two more, one per edge, to keep a - * surrogate pair whole), "…" where the line goes on. + * not fit in memory. A longer line is shown as a window of this many code + * units around the match (up to two more, one per edge, to keep a + * surrogate pair whole), "…" where the line goes on. A sequence of + * several code points (a ZWJ emoji, a flag) can still be cut between + * them. */ static constexpr qsizetype kMaxPreviewLength = 200; diff --git a/tests/auto/util/tst_util.cpp b/tests/auto/util/tst_util.cpp index 66ccea10d..aa54e754e 100644 --- a/tests/auto/util/tst_util.cpp +++ b/tests/auto/util/tst_util.cpp @@ -2989,6 +2989,9 @@ void tst_util::grepTakesABoundedNumberOfMatches() { QCOMPARE(pairs.size(), 1); const QString shown = pairs.first(); QVERIFY2(shown.contains(QStringLiteral("hit")), qPrintable(shown)); + // Both edges fell inside a pair and took one more code unit, and "…" + // stands on either side. + QCOMPARE(shown.size(), NativeGrep::kMaxPreviewLength + 4); for (qsizetype i = 0; i < shown.size(); ++i) { if (shown.at(i).isHighSurrogate()) { QVERIFY2( From a2049cf2ef89b07a2afb701f4d97492edd6ee1c1 Mon Sep 17 00:00:00 2001 From: Anne Jan Brouwer Date: Wed, 28 Oct 2026 11:24:25 +0100 Subject: [PATCH 4/4] tst_util: the wide-line stand-in finds its file next to itself CodeRabbit on #2081: the script quoted the temporary path in '...', which a TMPDIR holding a quote breaks. It reads wide.txt from its own directory now, so no path is written into it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJygpMUf461yZgfAqwMJ6d Signed-off-by: Anne Jan Brouwer --- tests/auto/util/tst_util.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/auto/util/tst_util.cpp b/tests/auto/util/tst_util.cpp index aa54e754e..107609b14 100644 --- a/tests/auto/util/tst_util.cpp +++ b/tests/auto/util/tst_util.cpp @@ -2982,7 +2982,9 @@ void tst_util::grepTakesABoundedNumberOfMatches() { f.write(wide.toUtf8() + '\n'); } QVERIFY(script.open(QIODevice::WriteOnly | QIODevice::Truncate)); - script.write("#!/bin/sh\ncat '" + QFile::encodeName(plain) + "'\n"); + // Next to the script, so no path is quoted into it (TMPDIR may hold a + // quote). + script.write("#!/bin/sh\ncat \"$(dirname \"$0\")/wide.txt\"\n"); script.close(); const QStringList pairs = NativeGrep::matchFile( env, longGpg, QDir(store).filePath("e0000.gpg"), rx);