Copy and paste from AOO crashes firefox

[ydario]

While composing a new message from gmail site, a copy&paste of data from a AOO sheet into message body is crashing firefox. Pasting the same informations into destinations does not crash and firefox correctly parses email addresses.
Attaching trap file.

https://gist.github.com/ydario/e1da41c2c6ac963395fa

[SilvanScherrer]

is this still the case with ffox 38.6.1?

[LewisR]

I can confirm that this happens with SM 2.35b7, when composing an email. Attempting to paste two text cells into the email composition window results in a crash. Pasting the same content into a text editor yields tab-delimited text.

ftp://ftp.2rosenthals.com/pub/os2/trp/008B_01.TRP

[dryeo]

Seems I screwed up packaging xul.xqs :) I'll upload it

[LewisR]

No worries, Dave. I honestly did not look closely at the TRP file. I have a decent test case, so once you have the file available, I'll re-run this and update the file linked above.

[dryeo]

On 07/25/16 11:17 PM, Lewis Rosenthal wrote:

No worries, Dave. I honestly did not look closely at the TRP file. I
have a decent test case, so once you have the file available, I'll
re-run this and update the file linked above.

https://bitbucket.org/dryeo/dry-comm-esr31/downloads/sm38_8_xul_xqs.7z
What special characters are you pasting? Have you tried pasting them
into FF or loading them into FF?

[LewisR]

Let's see...

There are two adjacent, left-aligned cells in a spreadsheet. One says "Test" and the other says "Data" (without quotes). When copied to the clipboard, using Dave Saville's ClipView, I can save the clipboard data and open that saved file in a hex editor. The hex is:
54 65 73 74 09 44 61 74 61 0D 0A 00 30 30 30 30 30 42 30 34 20 30 20 31 20 35 37 35 31 42 44 36 43
before the next 20 separator preceding the next clipboard entry.

Now, typing the same text in an editor, separated by the same tab, yields:
54 65 73 74 09 44 61 74 61 00 30 30 30 30 30 30 32 45 20 30 20 30 20 35 37 39 38 32 45 41 32

I am not sure what is in the trailing data which could bother SM. I have not tried pasting them into FF, but I will do so (and will update the TRP file) shortly.

[LewisR]

While I did not duplicate Yuri's test in his original comment, I did paste the text (with ClipView shut down, so straight from the OS/2 clipboard) into the quick draft box in WordPress running FF 38.8b7, and could not get it to crash. I tried pasting into the search box and the url bar, and still I got no crash.

Pasting the same content into a fresh SeaMonkey message draft - even in safe mode - generated the trap. TRP file updated.

I'm not completely convinced that this isn't something to do with my profile or something else (Infinality or one of the other things in the environment), so I'll do some more testing (fresh profile, another machine, etc.).

FWIW, my initial attempts this time around (using the saved clipboard content which I converted to hex, above) would not crash SeaMonkey. Instead, I had to copy the two cells again, and pasting those - first with ClipView, then without - got me the crash consistently.

[dryeo]

I couldn't reproduce in any Mozilla app, though that might just mean my OO is set up differently, I got fewer hex codes when pasting. I'd guess that there are more control chars that are not appearing in clipview as I can't see anything in what you posted that could crash. I think the crash is triggered in our widget clipboard code (actually this last one was in libc but the widget code is close to the top of the stack labels). You or Yuri are going to have to reproduce the trp in FF so Dmik can look at it.

[LewisR]

Okay, I can reproduce the trap under FF, when composing a message at Gmail.com. Of course, it would have helped had I checked to see whether I had xqs files installed before doing that, so now I'll need to get everything in place and go for it again.

I also made sure that I didn't have anything special set in the environment, by starting the exe from CMD and not via my usual script.

One more go-round...

[LewisR]

Trap under FF 38.8b7 when pasting text content (two adjacent cells) from Calc 4.1.2 into Gmail.com compose window:

ftp://ftp.2rosenthals.com/pub/os2/trp/0123_01.TRP

[LewisR]

Same condition, under SM 2.35b7 (pasting two text cells into Gmail.com compose window):

ftp://ftp.2rosenthals.com/pub/os2/trp/0115_01.TRP

[StevenLevine]

FWIW, both traps are the same. This looks like an off by one length
calculation error. The code is doing a memcpy and the remaining copy
length in ecx is way to large. I suspect ecx was 0xffffffff at the start
of the copy. The exception occurred when the memcpy tried to access
uncommitted memory that happened to follow the stack. The copy is from an
buffer on the stack (esi) to a buffer allocated in the heap (edi).

esi + ecx will give you that address of the source buffer on the stack for
the movsb trap.

For the movsd trap is esi + (ecx * 4).

[LewisR]

Thanks for that, Steve.

They looked quite similar, and I was hoping they were in common code.

The interesting thing about this is that with ClipView, at least, switching to a different clip and then switching back seems to allow the original clip to be pasted without the crash. Thus, I'm not sure whether the dump I got from the saved clips file was entirely accurate (which would support Dave's comment about not seeing anything which should be causing a problem in the content itself).

[SilvanScherrer]

is this still true in Firefox 45?

[ydario]

Yes, still same issue with 45.9
0057_01.txt

[dmik]

Confirmed the crash (reproducible with just copying a single cell in the spreadsheet) and found the cause. For some reason, in addition to plain text formats the Spreadsheet app puts a CF_BITMAP format to the clipboard when you copy the cell contents, even if the cell contains simple text. This is the first part of the problem. And the second part is that FF provides incomplete support for CF_BITMAP and image/* formats. It reports that nsClipboard supports image/* while in fact there are just stubs in widget/os2/nsClipboard.cpp saying in the comments that conversion between CF_BITMAP and image/* has to be added yet. Due to a bug in stub handling the data pointer and the data length field remain uninitialized by these stubs which then ends up with weird values passed to nsPrimitiveHelpers::CreatePrimitiveForData for these fields (like the data length being -1 or so). Which then causes the observed crash when doing a further memcpy (just like @StevenLevine said).

There is also the third part. The gmail web app first requests for image/png when it pastes clipboard contents to the message body and if it sees there is such a format available, it doesn't request anything else. When image/* is missing (or when pasting to simple text fields like the message subject) it requests text/unicode so all works.

Needless to say that pasting CF_BITMAP from any other application (e.g. from PMView after copying a selected area of an image the clipboard) to gmail would also crash FF (though with different crash data in the .TRP file due to different random data pointer and length values).

I fixed all the mentioned issues by temporarily disabling reporting image/* as being available when there is CF_BITMAP in the clipboard (a commit will follow) and will create a separate ticket to track the task of implementing real CF_BITMAP<->image conversion.

Now an attempt to pass cell contents from OO results in cells being pasted as text with tab and new line separators. And an attempt to paste CF_BITMAP from other applications will result in no action (as it should for unsupported formats).

[dmik]

@ydario btw it would be interesting to hear why CF_BITMAP is put by OO to the clipboard for cell data.

[dmik]

A test drop of XUL.DLL is available at http://rpm.netlabs.org/test/yb9p3.zip (in 5 min or so), please test.

[ydario]

This is becase Calc adds many different formats when copying cells: if copy a set of cells and try 'Paste special' in Writer you get the following list of formats:
calc8
metafile
bitmap
html
dde link
unformatted text
rtf text

FF should select the best format from the list.

[dmik]

@ydario ok thanks. Now, after I disabled missing bitmap support, it does — text/unicode is selected and pasted.

[dmik]

@StevenLevine BTW, .TRP generated for this crash are not correct in terms of call stack traversal and local variable display. In particular, in the above 0057_01.txt report from Yuri the following is wrong:

1. The crash place itself (i.e. the top call) is correct (nsPrimitiveHelpers::CreatePrimitiveForData() in nsPrimitiveHelpers.cpp#67) but the previous call is not: the TRP file says it's nsBaseFilePickerEnumerator::GetNext() in nsBaseFilePicker.cpp#126 while in fact it's nsClipboard::GetClipboardDataByID() in widget/os2/nsClipboard.cpp#182 (the current tree).

2. The value of the aDataLen argument to the top call is displayed as 9 while in fact it's -1 (0xFFFFFFFF).

I assume that somehow EXCEPTQ calculates a wrong offset for the stack frame (or within the stack frame) of the Firefox code. And this is not the first time when I get stack traces not matching the reality (except the first call) in FF. Often it just can't go beyond the top call at all (shows invalid address or such). Which also smells like some offset error. How can I help you to reproduce this on your side and improve/fix EXCEPTQ? This stuff is really important as it could save a lot of time if I got the correct stack trace — w/o it I have to spend time on checking wrong paths and then use source search and printf to trace the real execution chain.

An easy way to reproduce it on your side is to do something like

{ int *ptr = nullptr; *ptr = 0x1234; }

at a desired place deep in the Firefox source where it should crash. In this case this should have been at #67 in nsPrimitiveHelpers.cpp instead of a memcpy (which now works). Or you could pass -1 deliberately to this memcpy etc.

In either case I'm ready to assist you in this if needed. Correct call stack traversal is vital for fast crash resolving.