refactor: extract clipboard MIME helper for testability#971
Conversation
|
CodeAnt AI is reviewing your PR. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 10 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughExtracted clipboard MIME construction into a new helper Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
codeant-ai
Bot
added
the
size:M
|
CodeAnt AI finished reviewing your PR. |
|
coverage: 20.923% (+0.08%) from 20.84% — refactor/clipboard-mime-helper into main |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 81-83: Add a macOS test slot declaration consistent with the
existing ones: declare void buildClipboardMimeDataMac(); alongside
buildClipboardMimeDataLinux(), buildClipboardMimeDataWindows(), and
buildClipboardMimeDataDword() so the macOS-specific MIME type set by
buildClipboardMimeData is covered by a corresponding test slot.
- Around line 1131-1145: The test calls the internally-linked helper
dwordBytes(), which is defined as static/inline in the implementation and thus
not visible to the test; either make dwordBytes a public non-static function by
moving its full definition into the implementation file without static/inline
and adding a matching declaration to the header, or change the test to exercise
the behavior via the public API (e.g., call buildClipboardMimeData() and inspect
the produced QByteArray) so you no longer rely on the internal dwordBytes()
symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0e3fd34-e375-4bbf-a400-864fff9d5ec0
📒 Files selected for processing (2)
src/qtpass.htests/auto/util/tst_util.cpp
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/qtpass.h`:
- Around line 120-125: The public helper buildClipboardMimeData(const QString
&text) currently returns a raw QMimeData* without documenting ownership; update
the function comment to explicitly state that ownership is transferred to the
caller (caller is responsible for deleting the pointer or transferring it to a
QObject owner such as QClipboard::setMimeData which takes ownership), or
alternatively change the API to return a smart pointer (e.g.,
std::unique_ptr<QMimeData>) to express ownership — reference
buildClipboardMimeData in your change so callers know who must free or accept
ownership.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 57e3d4fb-15a2-4102-900e-b2391ca1608c
📒 Files selected for processing (2)
src/qtpass.cppsrc/qtpass.h
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 1117-1126: The test tst_util::buildClipboardMimeDataWindows only
asserts ExcludeClipboardContentFromMonitorProcessing; update it to also verify
the two other Windows hardening flags set by buildClipboardMimeData: check that
mime->hasFormat("CanIncludeInClipboardHistory") and
mime->data("CanIncludeInClipboardHistory") equals dwordBytes(0), and similarly
assert mime->hasFormat("CanUploadToCloudClipboard") and
mime->data("CanUploadToCloudClipboard") equals dwordBytes(0); ensure these
assertions use QVERIFY2 with clear messages and keep the existing cleanup
(delete mime).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: aa370501-e46a-463e-b4f4-bb87a46e40f1
📒 Files selected for processing (1)
tests/auto/util/tst_util.cpp
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #971 +/- ##
==========================================
+ Coverage 26.86% 26.99% +0.12%
==========================================
Files 39 39
Lines 3283 3286 +3
==========================================
+ Hits 882 887 +5
+ Misses 2401 2399 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 1117-1136: The function tst_util::buildClipboardMimeDataWindows
contains a duplicated preprocessor tail: remove the orphaned second
`#else`/`#endif` and extra closing brace that repeat the non-Windows skip block
so there is only one `#ifdef Q_OS_WIN ... `#else` QSKIP("Windows-only test");
`#endif`` and a single closing `}` for the function; ensure the remaining block
keeps the Windows assertions (calls to buildClipboardMimeData, QVERIFY/QCOMPARE
checks) and the non-Windows QSKIP branch intact so the file compiles without
`#else without `#if`` errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8acc0d88-b9f8-415e-8e5f-3896708a774c
📒 Files selected for processing (2)
src/qtpass.htests/auto/util/tst_util.cpp
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 1123-1125: The test currently only checks excl.size() and
excl.at(0); instead, assert the entire 4-byte DWORD payload matches the expected
bytes by comparing excl to dwordBytes(1). Replace the single-byte check on
excl.at(0) with a full-buffer equality check (e.g., QVERIFY2(excl ==
dwordBytes(1), ...)) referencing the excl variable and the dwordBytes(1) helper
so malformed DWORD encodings are caught.
- Around line 1160-1162: Add an explicit format existence check before asserting
the payload: call mime->hasFormat("application/x-nspasteboard-concealed-type")
and assert it is true (using QVERIFY or QVERIFY2) to ensure the format key is
present, then keep the existing QVERIFY2 comparing
mime->data("application/x-nspasteboard-concealed-type") to QByteArray() to
verify the value; update the assertion messages accordingly to reflect the two
checks (format exists, then value is empty).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0ce5d49b-3fca-480b-93c0-38e3fc7f1def
📒 Files selected for processing (2)
src/qtpass.htests/auto/util/tst_util.cpp
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/auto/util/tst_util.cpp (1)
1127-1128:⚠️ Potential issue | 🟠 MajorAssert Windows flag payload values, not only format presence.
Line 1127 and Line 1128 only check
hasFormat()forCanIncludeInClipboardHistoryandCanUploadToCloudClipboard. A wrong DWORD payload would still pass this test, weakening the security regression coverage added by this PR.🔧 Suggested test strengthening
QVERIFY(mime->hasFormat("ExcludeClipboardContentFromMonitorProcessing")); QVERIFY(mime->hasFormat("CanIncludeInClipboardHistory")); QVERIFY(mime->hasFormat("CanUploadToCloudClipboard")); + QCOMPARE(mime->data("CanIncludeInClipboardHistory"), dwordBytes(0)); + QCOMPARE(mime->data("CanUploadToCloudClipboard"), dwordBytes(0)); delete mime;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auto/util/tst_util.cpp` around lines 1127 - 1128, The test currently only checks presence of formats; instead read the payload bytes for "CanIncludeInClipboardHistory" and "CanUploadToCloudClipboard" from the mime object (e.g., call mime->data("CanIncludeInClipboardHistory") / mime->data("CanUploadToCloudClipboard")), assert the QByteArray size is 4 (DWORD) and decode it as a little-endian 32-bit unsigned integer (using QDataStream or manual byte unpacking) and QCOMPARE that the decoded value equals the expected DWORD flag (e.g., 1 for enabled or the correct constant your product uses) for both formats to ensure the actual flag values are validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 1127-1128: The test currently only checks presence of formats;
instead read the payload bytes for "CanIncludeInClipboardHistory" and
"CanUploadToCloudClipboard" from the mime object (e.g., call
mime->data("CanIncludeInClipboardHistory") /
mime->data("CanUploadToCloudClipboard")), assert the QByteArray size is 4
(DWORD) and decode it as a little-endian 32-bit unsigned integer (using
QDataStream or manual byte unpacking) and QCOMPARE that the decoded value equals
the expected DWORD flag (e.g., 1 for enabled or the correct constant your
product uses) for both formats to ensure the actual flag values are validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4a48eac5-6b70-4eba-9e5b-7f85f26e3d94
📒 Files selected for processing (1)
tests/auto/util/tst_util.cpp
3 participants
User description
Summary
Changes
Testing
All existing tests pass.
Summary by CodeRabbit
Refactor
Tests
CodeAnt-AI Description
Copying text to the clipboard now applies the right privacy hints on Linux, macOS, and Windows
What Changed
Impact
✅ Fewer passwords saved in clipboard history✅ Better clipboard privacy on Linux, macOS, and Windows✅ More reliable password copying across platforms💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.