Bug 19151 - prepareForTextInsertion assumes Position is not null (even though it can be)
Summary: prepareForTextInsertion assumes Position is not null (even though it can be)
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 525.x (Safari 3.1)
Hardware: Macintosh OS X 10.5
: P1 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2008-05-20 12:40 PDT by Eric Seidel (no email)
Modified: 2017-07-18 08:26 PDT (History)
3 users (show)

See Also:

crash log (35.63 KB, text/plain)
2009-02-19 11:49 PST, Gavin Sherlock
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2008-05-20 12:40:07 PDT
prepareForTextInsertion assumes Position is not null (even though it can be)

I've seen Safari crash on an internal site a few times.  Seems to be due to a bad position object:

WebCore::InsertTextCommand::prepareForTextInsertion(WebCore::Position const &)
WebCore::InsertTextCommand::input(WebCore::String const &,bool)
WebCore::CompositeEditCommand::inputText(WebCore::String const &,bool)
WebCore::Editor::Command::execute(WebCore::String const &,WebCore::Event *)

Looking at TOT, looks like Position still could be null.  This is probably covered by the editing fuzzer, but I'm posting this here anyway just in case justin has an opinion as to what prepareForTextInsertion should be doing in this case.

Unfortunately I don't have a reduction (yet).
Comment 1 Justin Garcia 2008-05-20 14:07:05 PDT
We have this filed internally as: <rdar://problem/5659795>, which is just a series of reports of the crash and doesn't include steps to reproduce.  Don't really need a reduction, just a series of steps, no matter how long, that recreate the crash would be helpful because currently I don't quite understand how prepareForTextInsertion's input ends up being null.
Comment 2 Justin Garcia 2008-05-20 14:08:29 PDT
PS, Eric, what offset inside prepareForTextInsertion are you crashing at?  Could you just attach the full backtrace to the bug?  Thanks.
Comment 3 Eric Seidel (no email) 2008-05-20 14:16:29 PDT
Unfortunately I don't have a full backtrace anymore.  That paste buffer is long gone. :(  In general my setup on windows sucks.  IIRC right above this was document.execCommand called from JS.  The question is just how do you get the document into a position where document.execCommand("removeFormat") is going to cause this crash?

I was crashing on this line:
    if (!pos.node()->isTextNode()) {

pos.node() was NULL.

Which seems totally reasonable, looking back through how we got there.  input() doesn't check the Postition and positionAvoidingSpecialElementBoundary() (which is how we generated what position we use) just returns a null position back if passed in one.

If I find out any more, I'll be sure to add it here.  Again, I expect the fuzzer (attached to another bug) could probably repro this crash, but right now there are way to many other ASSERTs and crashes in the way.  I guess I could write a harness to enable a more complete search of the input space (w/o having to do it all manually).
Comment 4 Eric Seidel (no email) 2008-05-23 13:19:57 PDT
I saw this crash while dealing with editing fuzzer, I think this was the input:

document.execCommand('selectall', false, '<iframe src=about:blank>');
document.designMode = 'on';
document.execCommand('insertunorderedlist', false, '<script src=https://webkit.org>');
document.execCommand('delete', false, 'courier');
document.execCommand('justifyleft', false, 'courier');
document.execCommand('undo', false, 'red');
document.execCommand('justifyright', false, '</table>');
document.execCommand('inserthorizontalrule', false, false);
document.execCommand('InsertParagraph', false, '<script src=https://webkit.org>');
document.execCommand('selectall', false, 'red');
document.execCommand('justifyleft', false, '<pre>');
document.execCommand('InsertParagraph', false, '<td>');
document.execCommand('selectall', false, true);
document.execCommand('removeformat', false, '<table>');

But I can't get it to reproduce given that input in Safari again.  I need to come up with a more reliable debugging setup.

Whatever input I was debugging, the Selection was a CARET with DOWNSTREAM affinity, the selected node was a <br> which wasn't actually in any document.  I'm not sure how such a selection ever came to be.  Thus selection was valid, but when we turned the selection into a visible position it was a bogus position which caused a crash.

I'll see if I can find a reproducible case.  Or maybe the above input was actually the fuzzed input I was using, and it just only reproduces sometimes!?
Comment 5 Jon@Chromium 2009-02-04 15:23:33 PST
Also in Chromium see http://code.google.com/p/chromium/issues/detail?id=3932
Comment 6 Eric Seidel (no email) 2009-02-19 11:16:37 PST
is supposedly a reproducible case of this bug in Chromium.  I was not able to reproduce with those steps in a Debug build of ToT WebKit however. :(
Comment 7 Eric Seidel (no email) 2009-02-19 11:31:48 PST
Yay!  http://sms.orange.pl/ did crash Windows Safari!  We have a reproducible crasher!
Comment 8 Eric Seidel (no email) 2009-02-19 11:32:48 PST
The steps to crash:
1.  Load http://sms.orange.pl/
2.  Type some LETTERS (not numbers) into the "odbiorca" field.
3.  Crash.
Comment 9 Eric Seidel (no email) 2009-02-19 11:38:33 PST
http://sms.orange.pl/scripts/all.js is where it wires up the (modified?) jquery 1.2.6 input validator to the RECIPIENT input field.  I still don't quite know what the validator is doing.
Comment 10 Gavin Sherlock 2009-02-19 11:47:44 PST
The website in comment #8 also crashes release Safari on the Mac.  Crash log coming.
Comment 11 Gavin Sherlock 2009-02-19 11:49:05 PST
Created attachment 27804 [details]
crash log
Comment 12 Gavin Sherlock 2009-02-19 11:50:47 PST
however, it doesn't crash r41071
Comment 13 Justin Garcia 2010-07-27 15:01:17 PDT
We got a report that this happened when going to http://www.apple.com/jp/support/ and typing "おんが" ("onnga") in the search field, but it is not reproducible.