Bug 19151

Summary: prepareForTextInsertion assumes Position is not null (even though it can be)
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: gsherloc, ian.eng.webkit, justin.garcia
Priority: P1 Keywords: InRadar
Version: 525.x (Safari 3.1)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
crash log none

Eric Seidel (no email)
Reported 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::RemoveFormatCommand::doApply() WebCore::EditCommand::apply() WebCore::applyCommand(WTF::PassRefPtr<WebCore::EditCommand>) WebCore::executeRemoveFormat 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).
Attachments
crash log (35.63 KB, text/plain)
2009-02-19 11:49 PST, Gavin Sherlock 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Justin Garcia
Comment 1 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.
Justin Garcia
Comment 2 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.
Eric Seidel (no email)
Comment 3 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).
Eric Seidel (no email)
Comment 4 2008-05-23 13:19:57 PDT
I saw this crash while dealing with editing fuzzer, I think this was the input: <BODY><SCRIPT> 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>'); </SCRIPT> 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!?
Jon@Chromium
Comment 5 2009-02-04 15:23:33 PST
Also in Chromium see http://code.google.com/p/chromium/issues/detail?id=3932
Eric Seidel (no email)
Comment 6 2009-02-19 11:16:37 PST
http://code.google.com/p/chromium/issues/detail?id=6509 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. :(
Eric Seidel (no email)
Comment 7 2009-02-19 11:31:48 PST
Yay! http://sms.orange.pl/ did crash Windows Safari! We have a reproducible crasher!
Eric Seidel (no email)
Comment 8 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.
Eric Seidel (no email)
Comment 9 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.
Gavin Sherlock
Comment 10 2009-02-19 11:47:44 PST
The website in comment #8 also crashes release Safari on the Mac. Crash log coming.
Gavin Sherlock
Comment 11 2009-02-19 11:49:05 PST
Created attachment 27804 [details] crash log
Gavin Sherlock
Comment 12 2009-02-19 11:50:47 PST
however, it doesn't crash r41071
Justin Garcia
Comment 13 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.
Note You need to log in before you can comment on or make changes to this bug.