fix: enforce max clipboard size by ericli3690 · Pull Request #21782 · ankidroid/Anki-Android

ericli3690 · 2026-09-08T00:42:10Z

Note

Assisted-by: Claude Opus 5

Purpose / Description

Clipboard size is limited by the Binder IPC buffer size. If too much is copied, the app may throw an error (TransactionTooLarge). Previously, we protected against copying too much only at callsites (SyncMediaWorker) or not at all.

Fixes

Fixes the possibility of copying too much from card templates and crashing the app, etc., all other usages of the clipboard in the app.

Approach

This commit enforces a 200KB maximum copy size (100K chars) at the clipboard copying function so that callers no longer need to worry about it. This is purposefully much lower than 1MB because the 1MB limit is shared between all ongoing transactions for the entire device.

Adds unit tests for copyToClipboard. Deletes a useless string util function that was only used at a clipboard calling site. Removed an unused clipboard util function.

How Has This Been Tested?

Tested on a physical Samsung S23, API 36.
Copying debug info works.
Unit tests.

Learning

Split off from feat(reminders): persistent logs #21396

Checklist

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code

david-allison

LGTM, my thoughts almost certainly want to be split out (or ignored) unless they're really easy.

david-allison · 2026-09-08T00:48:40Z

I'm wondering whether we should have a value class to enforce the invariant/trimming, rather than doing it silently here

Good idea. I've taken a shot at this in the form of TruncatedString, could you take a look and let me know if this was what you were thinking?

david-allison · 2026-09-08T00:50:34Z

-                .getApplicationContext<Context>()
-                .getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager
+        context = ApplicationProvider.getApplicationContext()
+        clipboardManager = context.getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager


nit: there's getSystemService<T>

(Maybe a requireSystemService<T> would improve it further)

david-allison · 2026-09-08T00:52:53Z

@@ -155,7 +155,7 @@ class SyncMediaWorker(
    internal fun getCopyToClipboardIntent(text: String): PendingIntent {


Also here for the value class

There is already a Kotlin string function for this function with identical behaviour.

This helper function is no longer used anywhere. Introduced at `092f4f9f47`. Usage removed at `aabd99dc4e`.

Clipboard size is limited by the Binder IPC buffer size. If too much is copied, the app may throw an error (TransactionTooLarge). Previously, we protected against copying too much only at callsites (SyncMediaWorker) or not at all. This commit enforces a 200KB maximum copy size (100K chars) at the clipboard copying function so that callers no longer need to worry about it. This is purposefully much lower than 1MB because the 1MB limit is shared between all ongoing transactions for the entire device. Adds unit tests for copyToClipboard. Assisted-by: Claude Opus 5

ericli3690 · 2026-09-12T00:04:33Z

Changes:

Implemented TruncatedString inline value class.
Created getParcelableExtraCompat because the normal getParcelableExtra was bothering me.
Removed a hanging docstring that I found while doing the above.
Created CopyToClipboardReceiver.getIntent because the lack of it was bothering me.

Ready for review!

david-allison

All nitpicks, let's go! Thanks so much for the value class, didn't expect it to be that much of a pain.

Set whatever timebox for a second review you feel is appropriate, then merge at will

Suggested by David. Use an inline value class so that callers of copyToClipboard are made aware that the string they pass may be truncated if it is too long. Modifies unit tests accordingly. Assisted-by: Claude Opus 5

A new helper method for getting parcelable extras, modeled after Bundle.getParcelableCompat in BundleUtils. Assisted-by: Claude Opus 5

Hanging docstring from 95a1a87. Moved to NoteEditorMultimediaController already, so this should be safe to delete. It is currently annotating a completely different method. Discovered as part of a refactor involving getParcelableExtraCompat.

Following the getIntent pattern for this codebase. Assisted-by: Claude Opus 5

ericli3690 · 2026-09-12T00:33:48Z

Changes:

Added TODO comment about emoji / grapheme splitting, as advised by David.

criticalAY

LGTM! once the concerns from David are handled, feel free to add to queue
@david-allison fyi

criticalAY · 2026-09-12T19:57:13Z

+            text: TruncatedString,
+        ): Intent =
+            Intent(context, CopyToClipboardReceiver::class.java).apply {
+                putExtra(EXTRA_SYNC_ERROR_LOG, text)


non-blocking: Could the extra stay a String

Interesting! I assumed value classes would be handled as a 0-cost abstraction.

I'll check when I'm next at my PC

Ah, as in have the getIntent method take a TruncatedString, turn it into a raw string, send it, then have the onReceive method pack it back into a TruncatedString?

I think I've already handled all of David's feedback. Though, I am now realizing that for some reason this is a BroadcastReceiver, not an AnkiBroadcastReceiver. I'll probably stack on a new commit to turn it into an AnkiBroadcastReceiver, too.

criticalAY · 2026-09-12T19:57:53Z

+        private const val EXTRA_SYNC_ERROR_LOG = "syncErrorLog"
+
+        /**
+         * Method for getting an intent for this service.


nit:

Method for getting an intent for this receiver....

ericli3690 self-assigned this Sep 8, 2026

ericli3690 added the Needs Review label Sep 8, 2026

ericli3690 requested a review from david-allison September 8, 2026 00:44

david-allison approved these changes Sep 8, 2026

View reviewed changes

david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Sep 8, 2026

ericli3690 added 3 commits September 11, 2026 14:29

refactor: replace trimToLength with take

62ca66f

There is already a Kotlin string function for this function with identical behaviour.

chore: remove unused clipboard helper

b1f87fb

This helper function is no longer used anywhere. Introduced at `092f4f9f47`. Usage removed at `aabd99dc4e`.

ericli3690 force-pushed the ericli3690-clipboard branch from d0d6885 to df07b89 Compare September 12, 2026 00:02

ericli3690 added Needs Review and removed Needs Second Approval Has one approval, one more approval to merge labels Sep 12, 2026

ericli3690 requested a review from david-allison September 12, 2026 00:03

david-allison approved these changes Sep 12, 2026

View reviewed changes

Comment thread AnkiDroid/src/main/java/com/ichi2/utils/ClipboardUtil.kt

Comment thread AnkiDroid/src/test/java/com/ichi2/utils/ClipboardUtilTest.kt

Comment thread common/android/src/main/java/com/ichi2/anki/common/utils/ext/Intent.kt

david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Sep 12, 2026

ericli3690 added 4 commits September 11, 2026 17:32

feat: use TruncatedString for copyToClipboard

23ff4c1

Suggested by David. Use an inline value class so that callers of copyToClipboard are made aware that the string they pass may be truncated if it is too long. Modifies unit tests accordingly. Assisted-by: Claude Opus 5

refactor: getParcelableExtraCompat

41ff423

A new helper method for getting parcelable extras, modeled after Bundle.getParcelableCompat in BundleUtils. Assisted-by: Claude Opus 5

refactor: CopyToClipboardReceiver.getIntent

49ae922

Following the getIntent pattern for this codebase. Assisted-by: Claude Opus 5

ericli3690 force-pushed the ericli3690-clipboard branch from df07b89 to 49ae922 Compare September 12, 2026 00:32

criticalAY self-requested a review September 12, 2026 18:47

criticalAY approved these changes Sep 12, 2026

View reviewed changes

ericli3690 added the Needs reviewer reply Waiting for a reply from another reviewer label Sep 12, 2026

		@@ -155,7 +155,7 @@ class SyncMediaWorker(
		internal fun getCopyToClipboardIntent(text: String): PendingIntent {

Uh oh!

Uh oh!

Conversation

ericli3690 commented Sep 8, 2026

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning

Checklist

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericli3690 commented Sep 12, 2026

Uh oh!

david-allison left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ericli3690 commented Sep 12, 2026

Uh oh!

criticalAY left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

david-allison left a comment •

edited

Loading