Skip to content

Commit 1affb70

Browse files
committed
Bug 1966443: Skip NUL characters in text that is copied to clipboard or dragged r=gstoll,dom-core,edgar
Windows and GTK do not allow NUL characters in text data types that appear on the clipboard or in DND payloads. Mac and Android do not specify if this is allowed but they use string types that permit it. This patch normalizes behavior across platforms by removing NUL ('\0') characters from text flavors in those cases. The test has exceptions for the behavior of different platforms when copying a string that only contains NUL characters. Android and Wayland behave slightly differently than others but it is not an important edge case. Differential Revision: https://phabricator.services.mozilla.com/D258092
1 parent a2ba53f commit 1affb70

File tree

6 files changed

+272
-0
lines changed

6 files changed

+272
-0
lines changed

dom/base/nsContentAreaDragDrop.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,8 @@ nsresult DragDataProducer::Produce(DataTransfer* aDataTransfer, bool* aCanDrag,
640640
data = do_QueryInterface(supports);
641641
if (NS_SUCCEEDED(rv)) {
642642
data->GetData(mHtmlString);
643+
// Do not add NULs to DND text.
644+
mHtmlString.StripChar(L'\0');
643645
}
644646
rv = transferable->GetTransferData(kHTMLContext, getter_AddRefs(supports));
645647
data = do_QueryInterface(supports);
@@ -655,6 +657,8 @@ nsresult DragDataProducer::Produce(DataTransfer* aDataTransfer, bool* aCanDrag,
655657
data = do_QueryInterface(supports);
656658
NS_ENSURE_SUCCESS(rv, rv); // require plain text at a minimum
657659
data->GetData(mTitleString);
660+
// Do not add NULs to DND text.
661+
mTitleString.StripChar(L'\0');
658662
}
659663

660664
// default text value is the URL

widget/nsBaseClipboard.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "nsFocusManager.h"
2525
#include "nsIClipboardOwner.h"
2626
#include "nsIPromptService.h"
27+
#include "nsISupportsPrimitives.h"
2728
#include "nsError.h"
2829
#include "nsXPCOM.h"
2930

@@ -389,6 +390,7 @@ NS_IMETHODIMP nsBaseClipboard::SetData(
389390
mIgnoreEmptyNotification = true;
390391
// Reject existing pending asyncSetData request if any.
391392
RejectPendingAsyncSetDataRequestIfAny(aWhichClipboard);
393+
SanitizeForClipboard(aTransferable);
392394
rv = SetNativeClipboardData(aTransferable, aWhichClipboard);
393395
mIgnoreEmptyNotification = false;
394396
}
@@ -988,6 +990,42 @@ void nsBaseClipboard::RequestUserConfirmation(
988990
promise->AppendNativeHandler(sUserConfirmationRequest);
989991
}
990992

993+
/* static */
994+
nsresult nsBaseClipboard::SanitizeForClipboard(nsITransferable* aTransferable) {
995+
NS_ENSURE_ARG(aTransferable);
996+
997+
nsTArray<nsCString> flavors;
998+
nsresult rv = aTransferable->FlavorsTransferableCanImport(flavors);
999+
NS_ENSURE_SUCCESS(rv, rv);
1000+
1001+
// Remove NULs from text flavors.
1002+
for (const auto& flavor : flavors) {
1003+
nsCOMPtr<nsISupports> data;
1004+
rv = aTransferable->GetTransferData(flavor.get(), getter_AddRefs(data));
1005+
NS_ENSURE_SUCCESS(rv, rv);
1006+
if (NS_WARN_IF(MOZ_UNLIKELY(!data))) {
1007+
continue;
1008+
}
1009+
nsCOMPtr<nsISupportsString> stringData = do_QueryInterface(data);
1010+
if (!stringData) {
1011+
continue;
1012+
}
1013+
1014+
// Remove NULs from stringData. If that does anything then the size of the
1015+
// string will be reduced.
1016+
nsAutoString newString;
1017+
rv = stringData->GetData(newString);
1018+
NS_ENSURE_SUCCESS(rv, rv);
1019+
auto oldLength = newString.Length();
1020+
newString.StripChar(L'\0');
1021+
if (newString.Length() != oldLength) {
1022+
rv = stringData->SetData(newString);
1023+
NS_ENSURE_SUCCESS(rv, rv);
1024+
}
1025+
}
1026+
return NS_OK;
1027+
}
1028+
9911029
NS_IMPL_ISUPPORTS(nsBaseClipboard::ClipboardDataSnapshot,
9921030
nsIClipboardDataSnapshot)
9931031

widget/nsBaseClipboard.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,10 @@ class nsBaseClipboard : public nsIClipboard {
252252
const nsTArray<nsCString>& aFlavorList, ClipboardType aClipboardType,
253253
mozilla::dom::WindowContext* aRequestingWindowContext);
254254

255+
// Clean up data in transferable for posting to clipboard or dragging. This
256+
// guarantees that text data does not include NUL characters.
257+
static nsresult SanitizeForClipboard(nsITransferable* aTransferable);
258+
255259
// Track the pending request for each clipboard type separately. And only need
256260
// to track the latest request for each clipboard type as the prior pending
257261
// request will be canceled when a new request is made.

widget/tests/mochitest.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ skip-if = [
4646
support-files = "file_test_clipboard_pngPreservesTransparency.js"
4747
scheme = "https" # ImageDecoder requires secure context
4848

49+
["test_clipboard_read_paste_ignoreNuls.html"]
50+
4951
["test_contextmenu_by_mouse_on_unix.html"]
5052
run-if = [
5153
"os == 'linux'",
@@ -60,6 +62,9 @@ skip-if = [
6062
"xorigin", # high-frequently intermittent fail
6163
]
6264

65+
["test_dnd_ignoreNuls.html"]
66+
skip-if = ["os == 'android'"] # Fx on android does not currently support DND
67+
6368
["test_keypress_event_with_alt_on_mac.html"]
6469
run-if = ["os == 'mac'"]
6570

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
<!DOCTYPE HTML>
2+
<!-- This Source Code Form is subject to the terms of the Mozilla Public
3+
- License, v. 2.0. If a copy of the MPL was not distributed with this
4+
- file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
5+
<html>
6+
<head>
7+
<meta charset="utf-8" />
8+
<title>Test for Bug 1966443</title>
9+
<link rel="stylesheet" href="/tests/SimpleTest/test.css">
10+
<script src="/tests/SimpleTest/SimpleTest.js"></script>
11+
</head>
12+
13+
<body>
14+
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1966443">Mozilla Bug 1966443 Clipboard Test</a>
15+
<textarea id="source"></textarea>
16+
<input type="text" id="target" />
17+
<script type="application/javascript">
18+
SimpleTest.waitForExplicitFinish();
19+
const {AppConstants} = SpecialPowers.ChromeUtils.importESModule(
20+
"resource://gre/modules/AppConstants.sys.mjs"
21+
);
22+
const { AppInfo } = SpecialPowers.ChromeUtils.importESModule(
23+
"chrome://remote/content/shared/AppInfo.sys.mjs"
24+
);
25+
26+
// copied from clipboard_helper.js
27+
async function getWindowProtocol() {
28+
return await SpecialPowers.spawnChrome([], () => {
29+
try {
30+
return Cc["@mozilla.org/gfx/info;1"].getService(Ci.nsIGfxInfo)
31+
.windowProtocol;
32+
} catch {
33+
return "";
34+
}
35+
});
36+
}
37+
38+
const isAndroid = AppConstants.platform == "android";
39+
const isWayland = getWindowProtocol() === "wayland";
40+
41+
const kNothingCopied = "<nothing copied...>";
42+
const kNothingPasted = "<nothing pasted...>";
43+
const Ci = SpecialPowers.Ci;
44+
let sourceElt = document.getElementById("source");
45+
let targetElt = document.getElementById("target");
46+
47+
function copyToClipboard(actual) {
48+
// First, perform a copy that should always succeed so that we know the
49+
// clipboard contents before setting them to 'actual'. This not only
50+
// keeps prior checks from influencing this one but also is required
51+
// to detect "empty" copies (like "\0") that won't update the
52+
// clipboard.
53+
sourceElt.textContent = kNothingCopied;
54+
sourceElt.focus();
55+
sourceElt.select();
56+
SpecialPowers.wrap(document).execCommand("copy", false, null);
57+
58+
sourceElt.textContent = actual;
59+
sourceElt.select();
60+
SpecialPowers.wrap(document).execCommand("copy", false, null);
61+
}
62+
63+
function pasteFromClipboard() {
64+
targetElt.value = kNothingPasted;
65+
targetElt.focus();
66+
// Select contents so that paste replaces them
67+
targetElt.select();
68+
SpecialPowers.wrap(document).execCommand("paste", false, null);
69+
}
70+
71+
async function test(actual, expected, paste_is_todo, read_is_todo) {
72+
info(`setting and copying: '${actual}'`);
73+
copyToClipboard(actual);
74+
75+
let readTest = read_is_todo ? todo_is : is;
76+
readTest(
77+
SpecialPowers.getClipboardData("text/plain"), expected,
78+
"clipboard read data is correct");
79+
80+
info(`pasting: '${sourceElt.textContent}'`);
81+
pasteFromClipboard();
82+
83+
let pasteTest = paste_is_todo ? todo_is : is;
84+
pasteTest(targetElt.value, expected, `Pasted string is correct`);
85+
}
86+
87+
async function runTests() {
88+
for (let currentTest of tests) {
89+
await test(currentTest.actual, currentTest.expected, currentTest.paste_is_todo, currentTest.read_is_todo);
90+
}
91+
SimpleTest.finish();
92+
}
93+
94+
// actual is the string sent to the clipboard.
95+
// expected is the result of reading or pasting the clipboard.
96+
// paste_is_todo means a paste check is a todo.
97+
// read_is_todo means the clipboard read check is a todo.
98+
//
99+
// At present, pasting a string with only NULs does nothing, even
100+
// though clipboard reads report it as the empty string. Note that an
101+
// empty string *without* NULs cannot be copied to the clipboard (in
102+
// Gecko).
103+
//
104+
// Android and Wayland don't allow copying empty strings in either case.
105+
let tests = [
106+
{ actual: "abc", expected: "abc"},
107+
{ actual: "\0", expected: "", paste_is_todo: true, read_is_todo: isAndroid || isWayland},
108+
{ actual: " \0", expected: " "},
109+
{ actual: "\0 ", expected: " "},
110+
{ actual: " \0 ", expected: " "},
111+
{ actual: "\0\0", expected: "", paste_is_todo: true, read_is_todo: isAndroid || isWayland},
112+
{ actual: " \0\0", expected: " "},
113+
{ actual: "\0\0 ", expected: " "},
114+
{ actual: " \0\0 ", expected: " "},
115+
{ actual: " \0 \0 ", expected: " "},
116+
{ actual: "a\0b\0c", expected: "abc"},
117+
];
118+
119+
runTests();
120+
</script>
121+
</body>
122+
</html>
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<!DOCTYPE HTML>
2+
<!-- This Source Code Form is subject to the terms of the Mozilla Public
3+
- License, v. 2.0. If a copy of the MPL was not distributed with this
4+
- file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
5+
<html>
6+
<head>
7+
<meta charset="utf-8" />
8+
<title>Test for Bug 1966443</title>
9+
<link rel="stylesheet" href="/tests/SimpleTest/test.css">
10+
<script src="/tests/SimpleTest/SimpleTest.js"></script>
11+
<script src="/tests/SimpleTest/SpecialPowers.js"></script>
12+
<script src="/tests/SimpleTest/EventUtils.js"></script>
13+
</head>
14+
15+
<body>
16+
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1966443">Mozilla Bug 1966443 DND test</a>
17+
<div></div>
18+
<textarea style="cursor: pointer;" id="source"></textarea>
19+
<div></div>
20+
<input type="text" id="target" ondragover="event.preventDefault();"/>
21+
<script type="application/javascript">
22+
SimpleTest.waitForExplicitFinish();
23+
SpecialPowers.contentTransformsReceived(window);
24+
25+
const Ci = SpecialPowers.Ci;
26+
const sourceElt = document.getElementById("source");
27+
const targetElt = document.getElementById("target");
28+
const scale = window.devicePixelRatio;
29+
// For the test, assume the special NUL character is 13px wide.
30+
const NULCharWidth = 13;
31+
32+
async function test(actual, expected) {
33+
info(`initializing source to '${actual}' and clearing target`);
34+
targetElt.value = "";
35+
sourceElt.textContent = actual;
36+
sourceElt.select();
37+
38+
// Find the first non-NUL character and use its position as drag start.
39+
// If there are no non-NUL characters then use the first NUL.
40+
let nonNulIndex = 0;
41+
for (let i = 0; i < actual.length; i++) {
42+
if (actual.charAt(i) != '\0') {
43+
nonNulIndex = i;
44+
break;
45+
}
46+
}
47+
48+
info(`dragging '${sourceElt.textContent}' from source to target`);
49+
// Add a small offset to cover borders and text margins.
50+
const offset = 8;
51+
let sourceOffset = [ (nonNulIndex * NULCharWidth) * scale + offset, offset ];
52+
let targetOffset = [ offset, offset ];
53+
await synthesizeMockDragAndDrop({
54+
srcElement: "source",
55+
targetElement: "target",
56+
sourceOffset,
57+
targetOffset,
58+
record,
59+
info,
60+
// No drag should happen if the expected string is empty.
61+
expectNoDragEvents: expected === "",
62+
});
63+
64+
is(targetElt.value, expected, "dropped data is correct");
65+
}
66+
67+
async function runTests() {
68+
for (let currentTest of tests) {
69+
await test(currentTest.actual, currentTest.expected);
70+
}
71+
SimpleTest.finish();
72+
}
73+
74+
// actual is the string sent to the clipboard.
75+
// expected is the result of reading or pasting the clipboard.
76+
// Tests that expect the empty string are only sanity checks -- they will
77+
// pass because the target is initialized to the empty string. In those
78+
// cases, we should not receive any drag events. This actually happens
79+
// because clicking on a special character representing a NUL undoes the
80+
// selection that would be dragged, so there is nowhere to click to start
81+
// an empty drag. This also means that we need to search for a non-NUL
82+
// character to click when initiating a drag.
83+
let tests = [
84+
{ actual: "abc", expected: "abc"},
85+
{ actual: "\0", expected: ""},
86+
{ actual: "a\0", expected: "a"},
87+
{ actual: "\0a", expected: "a"},
88+
{ actual: "a\0b", expected: "ab"},
89+
{ actual: "\0\0", expected: ""},
90+
{ actual: "a\0\0", expected: "a"},
91+
{ actual: "\0\0a", expected: "a"},
92+
{ actual: "a\0\0b", expected: "ab"},
93+
{ actual: "a\0b\0c", expected: "abc"},
94+
];
95+
96+
runTests();
97+
</script>
98+
</body>
99+
</html>

0 commit comments

Comments
 (0)