Skip to content

Conversation

@codebytere
Copy link
Member

@codebytere codebytere commented Oct 25, 2021

Description of Change

Closes #31556.

Fixes an issue where clipboard.read() would cause crashes with custom clipboard formats as a result of 2798d15, which was itself added to compensate for CL:3039323. Upon further reading of the CL, it's not clear to me that we need to restrict format types to those returned from ExtractCustomPlatformNames, since that is for example empty on macOS and seems to be addressing something other than what we do in clipboard.read(). This caused a DCHECK:

Stacktrace
[22922:1023/070046.410103:FATAL:electron_api_clipboard.cc(65)] Check failed: custom_format_names.find(format_string) != custom_format_names.end(). 
0   Electron Framework                  0x000000011ba497b9 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   Electron Framework                  0x000000011b9670b3 base::debug::StackTrace::StackTrace() + 19
2   Electron Framework                  0x000000011b9823df logging::LogMessage::~LogMessage() + 175
3   Electron Framework                  0x000000011b9833ae logging::LogMessage::~LogMessage() + 14
4   Electron Framework                  0x000000011732b72c electron::api::Clipboard::Read(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 700
5   Electron Framework                  0x000000011732ea92 base::internal::Invoker<base::internal::BindState<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (*)(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)>::Run(base::internal::BindStateBase*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 18
6   Electron Framework                  0x000000011732ea15 void gin_helper::Invoker<gin_helper::IndicesHolder<0ul>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&>::DispatchToCallback<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(base::RepeatingCallback<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)>) + 133
7   Electron Framework                  0x000000011732e8f1 gin_helper::Dispatcher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)>::DispatchToCallback(v8::FunctionCallbackInfo<v8::Value> const&) + 257
8   Electron Framework                  0x0000000118bcff93 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) + 979
9   Electron Framework                  0x0000000118bcde1d v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) + 1997
10  Electron Framework                  0x0000000118bcb7e3 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) + 547
11  Electron Framework                  0x0000000118bcb32d v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) + 109
12  ???                                 0x0000004000093938 0x0 + 274878511416
Task trace:
0   Electron Framework                  0x000000011c132e22 IPC::(anonymous namespace)::ChannelAssociatedGroupController::Accept(mojo::Message*) + 994

since the list is, as mentioned above, empty in our case.

cc @deepak1556 in case i am somehow missing something about the change in the CL.

Checklist

Release Notes

Notes: Fixes an issue where clipboard.read() could cause crashes with custom clipboard formats.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/15-x-y labels Oct 25, 2021
@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Oct 25, 2021
@deepak1556
Copy link
Member

The change was made to match the behavior of ReadBuffer with the new behavior of writeBuffer. With the CL, writeData call from electron::Clipboard::WriteBuffer will map custom mime types to web custom format via the ScopedClipboardWriter, for example clipboard.writeBuffer('public/utf8-plain-text', buffer); will map public/utf8-plain-text to com.web.custom.format1 and write the data to clipboard under this new format. So the reverse order of calls was performed in our read implementation to get the data of the original custom format clipboard.readBuffer('public/utf8-plain-text').

I didn't realize clipboard.read was also affected by this change, clipboard.readBuffer and clipboard.read should not share the same codepath to address this issue.

@codebytere
Copy link
Member Author

codebytere commented Oct 25, 2021

@deepak1556 ah, so I should move the logic into ReadBuffer to properly address the changes in the CL?

@deepak1556
Copy link
Member

Yup that would be the correct move. Thanks for making the change!

@codebytere codebytere changed the title fix: clipboard.read() crash fix: clipboard.read() crash with custom types Oct 25, 2021
@codebytere
Copy link
Member Author

@deepak1556 done, thanks!

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Oct 26, 2021
@codebytere
Copy link
Member Author

Failure not germane.

@codebytere codebytere merged commit deb7ab2 into main Oct 26, 2021
@codebytere codebytere deleted the fix-clipboard-read branch October 26, 2021 12:14
@release-clerk
Copy link

release-clerk bot commented Oct 26, 2021

Release Notes Persisted

Fixes an issue where clipboard.read() could cause crashes with custom clipboard formats.

@trop
Copy link
Contributor

trop bot commented Oct 26, 2021

I have automatically backported this PR to "15-x-y", please check out #31591

@trop
Copy link
Contributor

trop bot commented Oct 26, 2021

I have automatically backported this PR to "16-x-y", please check out #31592

t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Clipboard.read crashes the process

4 participants