Recent

Author Topic: Clipboard.AsText crash  (Read 3048 times)

dbannon

  • Hero Member
  • *****
  • Posts: 600
    • tomboy-ng, a rewrite of the classic Tomboy
Clipboard.AsText crash
« on: December 31, 2018, 04:39:16 am »
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]
  1. uses ClipBrd;
  2. ...
  3. procedure TForm1.Button1Click(Sender: TObject);
  4. begin
  5.         Memo1.Append('[' +  Clipboard.AsText + ']');
  6. 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 !
Lazarus 1.8, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

lucamar

  • Hero Member
  • *****
  • Posts: 1943
Re: Clipboard.AsText crash
« Reply #1 on: December 31, 2018, 11:31:56 am »
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
« Last Edit: December 31, 2018, 12:08:54 pm by lucamar »
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus 2.0.2/2.0.4  - FPC 3.0.4 on:
(K|L)Ubuntu 12..16, Windows XP SP3, various DOSes.

GetMem

  • Hero Member
  • *****
  • Posts: 3477
Re: Clipboard.AsText crash
« Reply #2 on: December 31, 2018, 11:49:37 am »
Try this:
Code: Pascal  [Select]
  1. if Clipboard.HasFormat(CF_Text) then
  2.    Memo1.Append('[' +  Clipboard.AsText + ']');
  3.  

dbannon

  • Hero Member
  • *****
  • Posts: 600
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Clipboard.AsText crash
« Reply #3 on: December 31, 2018, 12:51:42 pm »
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




Lazarus 1.8, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

lucamar

  • Hero Member
  • *****
  • Posts: 1943
Re: Clipboard.AsText crash
« Reply #4 on: December 31, 2018, 01:08:14 pm »
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 ?

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 :D): 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. :)
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus 2.0.2/2.0.4  - FPC 3.0.4 on:
(K|L)Ubuntu 12..16, Windows XP SP3, various DOSes.

wp

  • Hero Member
  • *****
  • Posts: 5994
Re: Clipboard.AsText crash
« Reply #5 on: December 31, 2018, 03:25:24 pm »
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]
  1. function TClipboard.GetAsText: string;
  2. var
  3.   MemStream: TMemoryStream;
  4.   ASize: int64;
  5. begin
  6.   //DebugLn('[TClipboard.GetAsText] A ',ClipboardTypeName[ClipboardType]);
  7.   Result:='';
  8.   MemStream:=TMemoryStream.Create;
  9.   try
  10.     if GetFormat(PredefinedClipboardFormat(pcfText),MemStream) then begin
  11.       ASize:=MemStream.Size;
  12.       if (ASize>0) and (pchar(MemStream.Memory)[ASize-1]=#0) then
  13.         Dec(ASize);
  14.       MemStream.Position:=0;
  15.       SetLength(Result,ASize);
  16.       if ASize>0 then
  17.         MemStream.Read(Result[1],ASize);
  18.     end;
  19.   finally
  20.     MemStream.Free;
  21.   end;
  22.   //DebugLn('[TClipboard.GetAsText] END ',ClipboardTypeName[ClipboardType],' "',dbgstr(Result),'"');
  23. 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.
Lazarus trunk / fpc 3.0.4 / all 32-bit on Win-10

dbannon

  • Hero Member
  • *****
  • Posts: 600
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Clipboard.AsText crash
« Reply #6 on: January 01, 2019, 02:11:06 am »
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 ...



Lazarus 1.8, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

lucamar

  • Hero Member
  • *****
  • Posts: 1943
Re: Clipboard.AsText crash
« Reply #7 on: January 01, 2019, 02:45:07 am »
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 ...

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. :D

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!
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus 2.0.2/2.0.4  - FPC 3.0.4 on:
(K|L)Ubuntu 12..16, Windows XP SP3, various DOSes.

engkin

  • Hero Member
  • *****
  • Posts: 2513
Re: Clipboard.AsText crash
« Reply #8 on: January 01, 2019, 02:55:05 am »
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
Execuse my ignorance as a Windows user, what WS are you using? Line 305 is:
Code: Pascal  [Select]
  1.     Result:=ClipboardGetData(ClipboardType,FormatID,Stream);

and it leads to ws-dependent code:
Code: Pascal  [Select]
  1. function ClipboardGetData(ClipboardType: TClipboardType;
  2.   FormatID: TClipboardFormat; Stream: TStream): boolean;
  3. begin
  4.   Result := WidgetSet.ClipboardGetData(ClipboardType, FormatID, Stream);
  5. end;

lucamar

  • Hero Member
  • *****
  • Posts: 1943
Re: Clipboard.AsText crash
« Reply #9 on: January 01, 2019, 03:35:20 am »
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 program
Interesting findings. Will have to investigate a little more ... in a few hours. Time to go to bed O:-)
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus 2.0.2/2.0.4  - FPC 3.0.4 on:
(K|L)Ubuntu 12..16, Windows XP SP3, various DOSes.

dbannon

  • Hero Member
  • *****
  • Posts: 600
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Clipboard.AsText crash
« Reply #10 on: January 01, 2019, 03:54:44 am »
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
Lazarus 1.8, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

dbannon

  • Hero Member
  • *****
  • Posts: 600
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Clipboard.AsText crash
« Reply #11 on: January 01, 2019, 12:23:31 pm »
Sorry engkin, I missed your questions :

Execuse my ignorance as a Windows user, what WS are you using?
Line 305 is:
Code: Pascal  [Select]
  1.     Result:=ClipboardGetData(ClipboardType,FormatID,Stream);

and it leads to ws-dependent code:
Code: Pascal  [Select]
  1. function ClipboardGetData(ClipboardType: TClipboardType;
  2.   FormatID: TClipboardFormat; Stream: TStream): boolean;
  3. begin
  4.   Result := WidgetSet.ClipboardGetData(ClipboardType, FormatID, Stream);
  5. 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
Lazarus 1.8, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

engkin

  • Hero Member
  • *****
  • Posts: 2513
Re: Clipboard.AsText crash
« Reply #12 on: January 01, 2019, 08:58:06 pm »
Function TGtk2WidgetSet.ClipboardGetData in \lcl\interfaces\gtk2\gtk2winapi.inc file:

Sometimes SupportedFormats gets the same address from SelData.Data:
Code: Pascal  [Select]
  1. //\lcl\interfaces\gtk2\gtk2winapi.inc
  2. function TGtk2WidgetSet.ClipboardGetData(ClipboardType: TClipboardType;
  3.   FormatID: TClipboardFormat; Stream: TStream): boolean;
  4. var
  5. ..
  6.   SupportedFormats: PGdkAtom;
  7.   SelData: TGtkSelectionData;
  8. ..
  9.   function IsFormatSupported(CurFormat: TGdkAtom): boolean;
  10.   var
  11. ...
  12.   begin
  13. ...
  14.       SupportedFormats:=PGdkAtom(SelData.Data);
  15.  

but at the end of ClipboardGetData both get freed always:
Code: Pascal  [Select]
  1. //\lcl\interfaces\gtk2\gtk2winapi.inc
  2. function TGtk2WidgetSet.ClipboardGetData(ClipboardType: TClipboardType;
  3.   FormatID: TClipboardFormat; Stream: TStream): boolean;
  4. ...
  5.   finally
  6.     if SupportedFormats<>nil then FreeMem(SupportedFormats);
  7.     if SelData.Data<>nil then FreeMem(SelData.Data);
  8.   end;
  9. 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]
  1. //\lcl\interfaces\gtk2\gtk2winapi.inc
  2. function TGtk2WidgetSet.ClipboardGetData(ClipboardType: TClipboardType;
  3.   FormatID: TClipboardFormat; Stream: TStream): boolean;
  4. ...
  5.   finally
  6.     if SupportedFormats<>nil then FreeMem(SupportedFormats);
  7.     if (SelData.Data<>nil) and (PGdkAtom(SelData.Data)<>SupportedFormats) then FreeMem(SelData.Data);
  8.   end;
  9. end;

dbannon

  • Hero Member
  • *****
  • Posts: 600
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Clipboard.AsText crash
« Reply #13 on: January 01, 2019, 10:07:58 pm »
Quote
but 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
Lazarus 1.8, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

Bart

  • Hero Member
  • *****
  • Posts: 3465
    • Bart en Mariska's Webstek
Re: Clipboard.AsText crash
« Reply #14 on: January 02, 2019, 10:55:50 am »
@engkin: can you file a bugreport (with your relevant findings)?
Bart