Clipboard.AsText crash
lazarus / 32-Linux

----------------------------------------------------------------------
dbannon · 2018-12-31 03:39

On Linux, its apparently not safe to read Clipboard.asText if the
clipboard contains, eg only image data, nothing that can be regarded
as text. While that may sound reasonable, there seems a lot of code
that does check the formats in clipboard and returns an empty string
if nothing available. But it causes a crash.

I find, in minimal Lazarus app, calling clipboard.astext sets up a
SegV when exiting but in my real application (which is far from
minimal) it causes a crash a few lines after being called.

Should I call this a bug or should I check what text I have available
before calling asText ?

To replicate -

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i uses
ClipBrd; ...procedure TForm1.Button1Click(Sender: TObject);begin     
 Memo1.Append('[' +  Clipboard.AsText + ']');end;
Set up your clipboard with just image data. Whats in the clipboard
(obviously) depends on other apps. Easy example is using Firefox,
right click on an image, select "copy image" - in my case that puts
the image itself and no text in the clipboard. (Opening an image in a
viewer and copying may copy the image and may put the filename as text
in the clipboard as well)

Run the app, click the button and then exit the app. Crash !

----------------------------------------------------------------------
lucamar · 2018-12-31 10:31

IMHO it should either cause an exception or return an empty string, so
I would consider it a bug.

But let me check my assumptions first...

ETA:

Ok, I've tested the following:
Copying an image from the browser: pastes the image's URL;Copying an
image from the image ivewer: pastes the file name;Copying a newly
created image from GIMP: pastes an empty string.As you see, I'm unable
to replicate your problem: Clipboard.AsText() always returns something
(for various values of "something"); at least it does for me.

Oh! My system: Laz/FPC 1.8.4/3.04 + Ubuntu 12.04-i386

----------------------------------------------------------------------
balazsszekely · 2018-12-31 10:49

Try this:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i if
Clipboard.HasFormat(CF_Text) then   Memo1.Append('[' +
 Clipboard.AsText + ']');

----------------------------------------------------------------------
dbannon · 2018-12-31 11:51

So, lucamar you cannot reproduce my problem ?  Its the third case
thats interesting, where you get an empty string, as do I but you
don't see a crash afterwards ? Interesting, but 32bit Ubuntu 12 ?

GetMem, yep, aware that I can test for it but my actual problem is in
KControls, not my code, I am hoping for some reassurance before I send
TK a patch.

I'll package up my demo AND a small tool to show actual clipboard
contents - if I am only one seeing this, well, i'll need to think
about it.

thanks for your help ....

davo

----------------------------------------------------------------------
lucamar · 2018-12-31 12:08

Quote from: dbannon on December 31, 2018, 12:51:42 pmSo, lucamar you
cannot reproduce my problem ?  Its the third case thats interesting,
where you get an empty string, as do I but you don't see a crash
afterwards ? Interesting, but 32bit Ubuntu 12 ?

Rechecked in Kubuntu 14.04 and 16.04 (32 and 64 bits) with the same
results. Still cannot reproduce, but it's worth mentioning that I'm
using standard controls not KControls.

Re. 32 bits Ubuntu 12 (in a Pentium 4 box ): Yeah; I'm in Spain, you
see, and whatever the pundits say we are not yet out of the shoals, so
people keeps using their old equipment (and praying). At least this
year we finally managed to convince our last Ubuntu 8 clients to
update to version 12 (which update is not as easy as it sounds, beleve
me) and teach them how to use Unity.

----------------------------------------------------------------------
wp · 2018-12-31 14:25

The attached demo does what should produce the error according to your
description in the first post: It copies an image, the Lazarus
cheetah, to the clipboard and pastes it back into a label caption as a
string -- no problem, no crash, the string is just empty as expected.

Looking at the source code used by the getter of Clipboard.AsText it
can be seen that this operation is safe:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i function
TClipboard.GetAsText: string;var  MemStream: TMemoryStream;  ASize:
int64;begin  //DebugLn('[TClipboard.GetAsText] A
',ClipboardTypeName[ClipboardType]);  Result:=''; 
MemStream:=TMemoryStream.Create;  try    if
GetFormat(PredefinedClipboardFormat(pcfText),MemStream) then begin   
 ASize:=MemStream.Size;      if (ASize>0) and
(pchar(MemStream.Memory)[ASize-1]=#0) then        Dec(ASize);     
MemStream.Position:=0;      SetLength(Result,ASize);      if ASize>0
then        MemStream.Read(Result[1],ASize);    end;  finally   
MemStream.Free;  end;  //DebugLn('[TClipboard.GetAsText] END
',ClipboardTypeName[ClipboardType],' "',dbgstr(Result),'"');end;
Note the "GetFormat(PredefinedClipboardFormat(pcfText)...)". This
makes sure that that clipboard is checked whether it contains text -
if not the function returns an empty string.

The only thing I could imagine how a crash could happen here it that
the copying application writes text but does not specify the correct
length of the stream, or corrupts the text stream when writing another
clipboard format.

Please post which one of the KControls is having this issue.

----------------------------------------------------------------------
dbannon · 2019-01-01 01:11

OK, have done some more 'research' !
Attached is my demo programme. (Does not depend on KControls).
My Observations
I can reproduce it on 4 Linux boxes, 64bit and 32bit, Ubuntu and
Fedora.An empty clipboard does not show the problem. Only one with
image data but no text.The problem shows up when built with Lazarus
1.8.something. Using fixes_2_0 'fixes' the problem but, I suspect,
does so by introducing another problem, it does not report any image
data in the clipboard so, seems like an empty clipboard which does not
exhibit the problem. Be good if someone else with both 1.8.4 and
[trunk, fixes_2_0, rc3] could compare clipboard contents when they
have a clipboard of image but not text.I create that clipboard with
image but not text by right clicking an image in my Firefox browser
and choosing 'copy image' - have not managed to do it any other way.
Maybe its a Firefox bug ? Even so, memory corruption should not happen
....It does not show up on Mac Sierra or Windows 10, just a Linux
issue.
Typical of the error messages I see on the console after a sigsegV -

Code: [Select]Marked memory at $00007FFFEC013C40 invalid
Wrong signature $AAAAAAAA instead of D348D123
  $000000000063B7E1 line 305 of include/clipbrd.inc
  $000000000063C141 line 462 of include/clipbrd.inc
  $0000000000461904 line 73 of unit1.pas
#73 of unit1 is the line that called clipboard.astext


wp, the problem does not depend on KControls, its just that my
project, where I first found it, uses KControls, specifically KMemo. 
Like you, I trolled through the (1.8.4) clipboard code and could see
nothing obviously (and it would have to be obvious for me to spot it)
wrong.  But as its a clearly a memory issues, its quite nasty, I have
not been seen anything in bug tracker to indicate its a known issue
thats been fixed. So, perhaps, its still in there ? I have not yet
looked at your attachment, will do so now, things moving slowly on New
Year's Day......

lucamar - how can you make your clients use Unity ?  its dead and gone
(thankfully) - I run U18.04 on a 2011 Atom powered netbook, it works
OK, rather that than a scary unsupported OS - I hope your luck holds
...

----------------------------------------------------------------------
lucamar · 2019-01-01 01:45

Quote from: dbannon on January 01, 2019, 02:11:06 amlucamar - how can
you make your clients use Unity ?  its dead and gone (thankfully) - I
run U18.04 on a 2011 Atom powered netbook, it works OK, rather that
than a scary unsupported OS - I hope your luck holds ...

I don't make them do anything: I had to convince them to update from
8.04. Their boxes are Pentium 4's of 2004 (probably the last ever
built with that processor  ). Updating to Ubuntu 10.04 was (almost) a
snap but from there it did cost sweat and blood to get up to 12.04,
and then they just had enough of seeing my face in their offices
Who knows, maybe this new year I can convince them to update to Ubuntu
14.04, if my Pentium 4 box doesn't explode first.

Hey, much worse is the one old customer we have who actually uses
(daily!) an almost bare-bones CP/M machine! His programs have to be
updated with TP3-CP/M: now, that's fun!

----------------------------------------------------------------------
engkin · 2019-01-01 01:55

Quote from: dbannon on January 01, 2019, 02:11:06 amTypical of the
error messages I see on the console after a sigsegV -

Code: [Select]Marked memory at $00007FFFEC013C40 invalid
Wrong signature $AAAAAAAA instead of D348D123
  $000000000063B7E1 line 305 of include/clipbrd.inc
  $000000000063C141 line 462 of include/clipbrd.inc
  $0000000000461904 line 73 of unit1.pas
#73 of unit1 is the line that called clipboard.astext
Execuse my ignorance as a Windows user, what WS are you using? Line
305 is:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i    
Result:=ClipboardGetData(ClipboardType,FormatID,Stream);
and it leads to ws-dependent code:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i function
ClipboardGetData(ClipboardType: TClipboardType;  FormatID:
TClipboardFormat; Stream: TStream): boolean;begin  Result :=
WidgetSet.ClipboardGetData(ClipboardType, FormatID, Stream);end;

----------------------------------------------------------------------
lucamar · 2019-01-01 02:35

I just tested your code with ... surprising results.
Copy/pasting an image from Firefox does indeed lead to an "access
violation" exception (attached image); maybe related to the three
"text/*" contents it sets?With an image from GIMP, as before, it seems
to work because GIMP doesn't set any text content. But:
Clicking once the "AsText" button shows "(empty)" in the label;Setting
"Be Safe" and clicking again shows "Not safe"Reseting "Be Safe" and
clicking again makes an abrupt (and quite!) end of the
programInteresting findings. Will have to investigate a little more
... in a few hours. Time to go to bed

----------------------------------------------------------------------
dbannon · 2019-01-01 02:54

Phew, I'm glad someone else can see it !

I tried wp's little demo app, it does not crash when you call
clipboard.astext but its a great source of "image in clipboard" that
will crash my demo.

I'm not surprised we cannot crash an app that has itself inserted an
image into the clipboard.  line #278 of clipbrd.inc, GetFormat(121,
Stream) makes a decision on how to get clipboard contents, if the app
itself 'owns' the content, it goes one way, if not, it calls
ClipboardGetData() - and that must be where the problem lies.

Good its been narrowed down but I still cannot see whats wrong with
remaining code, it has to be down at or near the widgetset layer.
Scary place ....

Davo

----------------------------------------------------------------------
dbannon · 2019-01-01 11:23

Sorry engkin, I missed your questions :

Quote from: engkin on January 01, 2019, 02:55:05 amExecuse my
ignorance as a Windows user, what WS are you using?
Line 305 is:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i    
Result:=ClipboardGetData(ClipboardType,FormatID,Stream);
and it leads to ws-dependent code:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i function
ClipboardGetData(ClipboardType: TClipboardType;  FormatID:
TClipboardFormat; Stream: TStream): boolean;begin  Result :=
WidgetSet.ClipboardGetData(ClipboardType, FormatID, Stream);end;
WidgetSet ?  Pretty much the 'standard witgetset for Linus, GTK2

Yep, it does have to get down to widgetset level when accessing the
clipboard, each platform does it differently ....

Davo

----------------------------------------------------------------------
engkin · 2019-01-01 19:58

Function TGtk2WidgetSet.ClipboardGetData in
\lcl\interfaces\gtk2\gtk2winapi.inc file:

Sometimes SupportedFormats gets the same address from SelData.Data:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
//\lcl\interfaces\gtk2\gtk2winapi.incfunction
TGtk2WidgetSet.ClipboardGetData(ClipboardType: TClipboardType; 
FormatID: TClipboardFormat; Stream: TStream): boolean;var.. 
SupportedFormats: PGdkAtom;  SelData: TGtkSelectionData;..  function
IsFormatSupported(CurFormat: TGdkAtom): boolean;  var...  begin...   
 SupportedFormats:=PGdkAtom(SelData.Data); 
but at the end of ClipboardGetData both get freed always:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
//\lcl\interfaces\gtk2\gtk2winapi.incfunction
TGtk2WidgetSet.ClipboardGetData(ClipboardType: TClipboardType; 
FormatID: TClipboardFormat; Stream: TStream): boolean;...  finally   
if SupportedFormats<>nil then FreeMem(SupportedFormats);    if
SelData.Data<>nil then FreeMem(SelData.Data);  end;end;
As the least intrusive fix: if they hold the same address, only one
needs to be freed at the end of ClipboardGetData:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
//\lcl\interfaces\gtk2\gtk2winapi.incfunction
TGtk2WidgetSet.ClipboardGetData(ClipboardType: TClipboardType; 
FormatID: TClipboardFormat; Stream: TStream): boolean;...  finally   
if SupportedFormats<>nil then FreeMem(SupportedFormats);    if
(SelData.Data<>nil) and (PGdkAtom(SelData.Data)<>SupportedFormats)
then FreeMem(SelData.Data);  end;end;

----------------------------------------------------------------------
dbannon · 2019-01-01 21:07

Quotebut at the end of ClipboardGetData both get freed always:
Wow, I would not have spotted that in a million years !

I'm flying to Hobart today but will try this out first chance I get.

Thanks engkin, first glimmer of hope with a problem niggling me for
days !

Davo

----------------------------------------------------------------------
Bart · 2019-01-02 09:55

@engkin: can you file a bugreport (with your relevant findings)?
Bart

----------------------------------------------------------------------
engkin · 2019-01-02 19:32

Just did: 34795.

----------------------------------------------------------------------
dbannon · 2019-01-02 21:27

And, yes, I can confirm, somewhat belatedly,  that has fixed the
problem I was rattling on about initially.

Thanks engkin, lucamar, GetMem and WP !

I'll now, as time permits look at that other incidental matter of why
trunk does not report about an image at all. I suspect that might mean
it cannot paste an image ? Won't be quick, quite busy ....

Davo