Bug 20577 – REGRESSION (r36006): Gmail is broken

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ismail Donmez]

After r36006 trying do anything from GMail interface throws a GMail #500 error, can't delete spam messages etc. Tested FF successfully.

[Ismail Donmez]

I was too fast to blame Darin on this, sorry! r36005 is not working either.

[Ismail Donmez]

Ok apperently my build script did a svn up before building so it was building r35006 instead of r35005. I fixed the script and indeed the problem is in r35006. CC'in Darin again. Sorry for the mess.

[Cameron Zwarich (cpst)]

I can confirm this.

[Matt Lilek]

I'm assuming Cameron already knows this, but I'll mention it here for future reference - this is what's hit when loading GMail:

ASSERTION FAILED: offset + length <= static_cast<unsigned>(s.size())
(/Users/matt/Code/WebKit/JavaScriptCore/kjs/JSString.cpp:135 KJS::JSString* KJS::jsSubstring(KJS::ExecState*, const KJS::UString&, unsigned int, unsigned int))

Thread 0 Crashed:
0   com.apple.JavaScriptCore      	0x00485282 KJS::jsSubstring(KJS::ExecState*, KJS::UString const&, unsigned int, unsigned int) + 238 (JSString.cpp:135)
1   com.apple.JavaScriptCore      	0x004858f3 KJS::stringProtoFuncSubstr(KJS::ExecState*, KJS::JSObject*, KJS::JSValue*, KJS::ArgList const&) + 571 (StringPrototype.cpp:578)
2   com.apple.JavaScriptCore      	0x0050b1bb KJS::Machine::privateExecute(KJS::Machine::ExecutionFlag, KJS::ExecState*, KJS::RegisterFile*, KJS::Register*, KJS::ScopeChainNode*, KJS::CodeBlock*, KJS::JSValue**) + 30083 (Machine.cpp:2484)
3   com.apple.JavaScriptCore      	0x0050d286 KJS::Machine::execute(KJS::FunctionBodyNode*, KJS::ExecState*, KJS::JSFunction*, KJS::JSObject*, KJS::ArgList const&, KJS::ScopeChainNode*, KJS::JSValue**) + 716 (Machine.cpp:857)
4   com.apple.JavaScriptCore      	0x0046aa2b KJS::JSFunction::call(KJS::ExecState*, KJS::JSValue*, KJS::ArgList const&) + 139 (JSFunction.cpp:71)
5   com.apple.JavaScriptCore      	0x0046aac7 KJS::call(KJS::ExecState*, KJS::JSValue*, KJS::CallType, KJS::CallData const&, KJS::JSValue*, KJS::ArgList const&) + 149 (CallData.cpp:39)
6   com.apple.JavaScriptCore      	0x00479112 KJS::functionProtoFuncApply(KJS::ExecState*, KJS::JSObject*, KJS::JSValue*, KJS::ArgList const&) + 494 (FunctionPrototype.cpp:107)
7   com.apple.JavaScriptCore      	0x0050b1bb KJS::Machine::privateExecute(KJS::Machine::ExecutionFlag, KJS::ExecState*, KJS::RegisterFile*, KJS::Register*, KJS::ScopeChainNode*, KJS::CodeBlock*, KJS::JSValue**) + 30083 (Machine.cpp:2484)
[snip]

[Cameron Zwarich (cpst)]

Here is a reduction from Gmail that works on the console:

"GMAIL_IMP=bf-i%2Fd-0-0%2Ftl-v".substr(10)

[Cameron Zwarich (cpst)]

The problem is basically that stringProtoFuncSubstr() was written to do its index calculations for the UString::substr() method, which, like the JS function substr, ignores lengths that are too long. Just plugging in jsSubstring doesn't quite work unless the index computations are changed to never overflow the length of the string. I'll assign this to myself, because Darin is probably with his family.

There are similar replacements of UString::substr() with jsSubstring() throughout Darin's patch, so after this is fixed it might be a good idea to go through and check that all of them won't fall prey to the same problem in unusual situations.

[Cameron Zwarich (cpst)]

Created attachment 23092 [details]
Proposed patch

[Maciej Stachowiak]

Comment on attachment 23092 [details]
Proposed patch

r=me

[Cameron Zwarich (cpst)]

Landed in r36009. I'll make a separate bug to check the other cases where UString::substr() was removed.

[Darin Adler]

(In reply to comment #9)
> Landed in r36009. I'll make a separate bug to check the other cases where
> UString::substr() was removed.

Thanks. This was the perfect fix. For what it's worth, I did fix some call sites that used to rely on substr's range checking; I just missed this one. I hope I didn't miss others.