Bug 96284

Summary: [GTK] WebKitGtk+ crashes with non-UTF8 HTTP header names
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: WebKitGTKAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: danw, gustavo, mrobinson, rakuco, svillar, tmpsantos, webkit.review.bot, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: https://sec-virtual.usc.es/Secretaria/Login.asp
Bug Depends on: 96395, 96504    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Sergio Villar Senin 2012-09-10 10:07:42 PDT
This is the backtrace generously provided by mrobinson:

#0 0x00007ffff489ca34 in WTF::StringImpl::is8Bit (this=0x0) at ../../Source/WTF/wtf/text/StringImpl.h:375
#1 0x00007ffff489d0d5 in WTF::CaseFoldingHash::hash (str=0x0) at ../../Source/WTF/wtf/text/StringHash.h:105
#2 0x00007ffff489d20c in WTF::CaseFoldingHash::hash (key=...) at ../../Source/WTF/wtf/text/StringHash.h:148
#3 0x00007ffff52a979c in WTF::HashMapTranslator<WTF::HashMapValueTraits<WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >, WTF::CaseFoldingHash>::hash<WTF::AtomicString> (
key=...) at ../../Source/WTF/wtf/HashMap.h:220
#4 0x00007ffff52a9188 in WTF::HashTable<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::String>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::String> >, WTF::CaseFoldingHash, WTF::HashMapValueTraits<WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >, WTF::HashTraits<WTF::AtomicString> >::add<WTF::HashMapTranslator<WTF::HashMapValueTraits<WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >, WTF::CaseFoldingHash>, WTF::AtomicString, WTF::String> (this=0x7fffffffcc38, key=..., extra=...)
at ../../Source/WTF/wtf/HashTable.h:825
#5 0x00007ffff52a8c92 in WTF::HashMap<WTF::AtomicString, WTF::String, WTF::CaseFoldingHash, WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >::inlineAdd (this=0x7fffffffcc38,
key=..., mapped=...) at ../../Source/WTF/wtf/HashMap.h:334
#6 0x00007ffff52a8a20 in WTF::HashMap<WTF::AtomicString, WTF::String, WTF::CaseFoldingHash, WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >::set (this=0x7fffffffcc38,
key=..., mapped=...) at ../../Source/WTF/wtf/HashMap.h:341
#7 0x00007ffff52c269f in WebCore::ResourceResponse::updateFromSoupMessage (this=0x7fffffffcbd0, soupMessage=0xb101e0) at ../../Source/WebCore/platform/network/soup/ResourceResponseSoup.cpp:73
#8 0x00007ffff52bb969 in WebCore::gotHeadersCallback (msg=0xb101e0, data=0x68c130) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:291
#9 0x00007ffff3e764c2 in g_closure_invoke (closure=0x87a2c0, return_value=0x0, n_param_values=1, param_values=0x7fffffffcee0, invocation_hint=<optimized out>) at gclosure.c:777
#10 0x00007ffff3e87ff3 in signal_emit_unlocked_R (node=<optimized out>, detail=0, instance=<optimized out>, emission_return=0x0, instance_and_params=0x7fffffffcee0) at gsignal.c:3547
#11 0x00007ffff3e908e0 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=0, var_args=<optimized out>) at gsignal.c:3296
#12 0x00007ffff3e90b22 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3352
#13 0x00007ffff7fc2c39 in io_read (msg=0xb101e0, cancellable=0x6dd6f0, error=0x7fffffffd2b8) at soup-message-io.c:607
#14 0x00007ffff7fc2ef2 in io_run_until (msg=0xb101e0, read_state=SOUP_MESSAGE_IO_STATE_BODY, write_state=SOUP_MESSAGE_IO_STATE_NOT_STARTED, cancellable=0x6dd6f0, error=0x7fffffffd318)
at soup-message-io.c:846
#15 0x00007ffff7fcf7d6 in try_run_until_read (item=0x7cc980) at soup-session-async.c:689
#16 0x00007ffff7fcf97c in read_ready_cb (msg=<optimized out>, user_data=<optimized out>) at soup-session-async.c:678
Comment 1 Sergio Villar Senin 2012-09-10 10:17:52 PDT
As Martin perfectly points out the problem is that the server sends a non-UTF8 header name and thus we end up with a NULL String causing the crash. Will attach a patch with a test case soon
Comment 2 Sergio Villar Senin 2012-09-11 04:22:29 PDT
Created attachment 163330 [details]
Patch
Comment 3 Sergio Villar Senin 2012-09-11 06:11:01 PDT
Committed r128175: <http://trac.webkit.org/changeset/128175>
Comment 4 Thiago Marcos P. Santos 2012-09-11 08:35:03 PDT
133 failures on the EFL bots after this change, the number is also high on GTK bots.
Comment 5 WebKit Review Bot 2012-09-11 08:36:56 PDT
Re-opened since this is blocked by 96395
Comment 6 Sergio Villar Senin 2012-09-11 08:59:00 PDT
Created attachment 163379 [details]
Patch
Comment 7 Sergio Villar Senin 2012-09-11 09:00:49 PDT
Beh, so basically I used headerValue instead of headerName in the strlen().

Do not use copy&paste at home kids.
Comment 8 Sergio Villar Senin 2012-09-11 09:41:53 PDT
Comment on attachment 163379 [details]
Patch

Clearing flags on attachment: 163379

Committed r128195: <http://trac.webkit.org/changeset/128195>
Comment 9 Sergio Villar Senin 2012-09-11 09:42:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Zan Dobersek 2012-09-11 13:36:06 PDT
This was rolled out again in r128221:
http://trac.webkit.org/changeset/128221

The patch was causing crashes in debug builds, here's a backtrace:

Crash log for DumpRenderTree (pid 23767):

[New LWP 23767]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/Programs/DumpR'.
Program terminated with signal 11, Segmentation fault.
#0  0x00007fb17ed45e9b in WebCore::parseHeader (header="Expires", headerLength=10, response=...) at ../../Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1058
1058	    ASSERT(pos != notFound);

Thread 1 (Thread 0x7fb173cab900 (LWP 23767)):
#0  0x00007fb17ed45e9b in WebCore::parseHeader (header="Expires", headerLength=10, response=...) at ../../Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1058
#1  0x00007fb17ed45fbd in WebCore::parseHeaders (headers="Keep-Alive\000\302\224Õ\177\000\000\000\000:timeout=15, max=92\nCache-Control\000\000\000 \000\000\000\000\000\000\000A:no-cache, must-revalidate\nCo:85\nServer\000\000¸\302\216B\302\224Õ\177\000\000\000\000\000\000\000\000\000\000Q\000\000\000\000\000\000\000Apache/2.2.22 (Debian) mod_ssl/2:Apache/2.2.22 (Debian) mod_ssl/2.2.22 O"..., response=...) at ../../Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1073
#2  0x00007fb17ed46432 in WebCore::ApplicationCacheStorage::loadCache (this=0x6e75b0, storageID=302517) at ../../Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1127
#3  0x00007fb17ed423f0 in WebCore::ApplicationCacheStorage::cacheGroupForURL (this=0x6e75b0, url=...) at ../../Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:238
#4  0x00007fb17ed2fd0c in WebCore::ApplicationCacheGroup::cacheForMainRequest (request=...) at ../../Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:108
#5  0x00007fb17ed3c0f0 in WebCore::ApplicationCacheHost::maybeLoadMainResource (this=0x7cb030, request=..., substituteData=...) at ../../Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:81
#6  0x00007fb17edd7938 in WebCore::MainResourceLoader::load (this=0x6d7650, r=..., substituteData=...) at ../../Source/WebCore/loader/MainResourceLoader.cpp:659
#7  0x00007fb17ed8bf0a in WebCore::DocumentLoader::startLoadingMainResource (this=0x74b5b0) at ../../Source/WebCore/loader/DocumentLoader.cpp:871
#8  0x00007fb17eda6494 in WebCore::FrameLoader::continueLoadAfterWillSubmitForm (this=0x77c928) at ../../Source/WebCore/loader/FrameLoader.cpp:2227
#9  0x00007fb17eda9031 in WebCore::FrameLoader::continueLoadAfterNavigationPolicy (this=0x77c928, formState=..., shouldContinue=true) at ../../Source/WebCore/loader/FrameLoader.cpp:2835
#10 0x00007fb17eda8743 in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy (argument=0x77c928, request=..., formState=..., shouldContinue=true) at ../../Source/WebCore/loader/FrameLoader.cpp:2705
#11 0x00007fb17edda41b in WebCore::PolicyCallback::call (this=0x7fff16380050, shouldContinue=true) at ../../Source/WebCore/loader/PolicyCallback.cpp:103
#12 0x00007fb17eddb2c9 in WebCore::PolicyChecker::continueAfterNavigationPolicy (this=0x77c938, policy=WebCore::PolicyUse) at ../../Source/WebCore/loader/PolicyChecker.cpp:167
#13 0x00007fb17e56573e in webkit_web_policy_decision_use (decision=0x73b300) at ../../Source/WebKit/gtk/webkit/webkitwebpolicydecision.cpp:88
#14 0x00007fb17e5370be in WebKit::FrameLoaderClient::dispatchDecidePolicyForNavigationAction (this=0x77b4b0, policyFunction=(void (WebCore::PolicyChecker::*)(WebCore::PolicyChecker * const, WebCore::PolicyAction)) 0x7fb17eddb07e <WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction)>, action=..., resourceRequest=...) at ../../Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:467
#15 0x00007fb17eddacd0 in WebCore::PolicyChecker::checkNavigationPolicy (this=0x77c938, request=..., loader=0x74b5b0, formState=..., function=0x7fb17eda86f4 <WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>, argument=0x77c928) at ../../Source/WebCore/loader/PolicyChecker.cpp:89
#16 0x00007fb17eda2c6c in WebCore::FrameLoader::loadWithDocumentLoader (this=0x77c928, loader=0x74b5b0, type=WebCore::FrameLoadTypeStandard, prpFormState=...) at ../../Source/WebCore/loader/FrameLoader.cpp:1413
#17 0x00007fb17eda26a4 in WebCore::FrameLoader::load (this=0x77c928, newDocumentLoader=0x74b5b0) at ../../Source/WebCore/loader/FrameLoader.cpp:1354
#18 0x00007fb17eda2099 in WebCore::FrameLoader::load (this=0x77c928, request=..., substituteData=..., lockHistory=false) at ../../Source/WebCore/loader/FrameLoader.cpp:1288
#19 0x00007fb17eda1ec3 in WebCore::FrameLoader::load (this=0x77c928, request=..., lockHistory=false) at ../../Source/WebCore/loader/FrameLoader.cpp:1277
#20 0x00007fb17e55fa70 in webkit_web_frame_load_uri (frame=0x77bd20, uri=0x74d9b8 "http://127.0.0.1:8000/appcache/document-write-html-element-2.html") at ../../Source/WebKit/gtk/webkit/webkitwebframe.cpp:696
#21 0x00007fb17e5789d1 in webkit_web_view_load_uri (webView=0x730030, uri=0x74d9b8 "http://127.0.0.1:8000/appcache/document-write-html-element-2.html") at ../../Source/WebKit/gtk/webkit/webkitwebview.cpp:4102
#22 0x00007fb17e578787 in webkit_web_view_open (webView=0x730030, uri=0x74d9b8 "http://127.0.0.1:8000/appcache/document-write-html-element-2.html") at ../../Source/WebKit/gtk/webkit/webkitwebview.cpp:4062
#23 0x000000000047749b in runTest (inputLine=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:751
#24 0x0000000000476b74 in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:540
#25 0x0000000000479b2e in main (argc=2, argv=0x7fff16381688) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1445
Comment 11 Zan Dobersek 2012-09-12 01:46:10 PDT
(In reply to comment #10)
> This was rolled out again in r128221:
> http://trac.webkit.org/changeset/128221

This roll-out was unnecessary, the fix was already re-landed in r128195
http://trac.webkit.org/changeset/128221

I apologize, I'll clean up the mess.

There have been some crashes (as reported in comment #10) on the debug build even after this rollout -that's a bit strange. I'll reland the patch from r128195 and see how the debug builder will react.
Comment 12 Zan Dobersek 2012-09-12 06:18:57 PDT
The crashes just won't go away, hopefully rolling back in the proper patch will help to that.
http://trac.webkit.org/changeset/128302
Comment 13 Zan Dobersek 2012-09-12 08:23:59 PDT
The application cache was corrupted so it had to be cleared. The tests should be OK now.

Closing the bug as fixed.