Bug 16906

Summary: [CURL] Crash below ResourceHandleManager::setupPOST when job->request().httpBody() is NULL
Product: WebKit Reporter: Grzegorz D&#261;browski <grzegorz.dabrowski>
Component: Page LoadingAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: zwarich
Priority: P2 Keywords: Curl
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Test case using jquery
none
Simple test case
none
Add null checks for httpBody()
none
Added test case zecke: review+

Description Grzegorz D&#261;browski 2008-01-17 06:45:12 PST
Sorry, I cannot add an URL for test case, because the page is from Intranet.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1252783920 (LWP 29631)]
Vector (this=0xbfaad424, other=@0x8) at ../JavaScriptCore/wtf/Vector.h:513
513             , m_impl(other.capacity())
Current language:  auto; currently c++
(gdb) bt
#0  Vector (this=0xbfaad424, other=@0x8) at ../JavaScriptCore/wtf/Vector.h:513
#1  0xb6421785 in WebCore::ResourceHandleManager::setupPOST (this=0x80d04a8, job=0x868c688, headers=0xbfaad54c)
    at ../WebCore/platform/network/curl/ResourceHandleManager.cpp:367
#2  0xb6422e45 in WebCore::ResourceHandleManager::startJob (this=0x80d04a8, job=0x868c688)
    at ../WebCore/platform/network/curl/ResourceHandleManager.cpp:575
#3  0xb6423704 in WebCore::ResourceHandleManager::startScheduledJobs (this=0x80d04a8)
    at ../WebCore/platform/network/curl/ResourceHandleManager.cpp:447
#4  0xb642453b in WebCore::ResourceHandleManager::downloadTimerCallback (this=0x80d04a8, timer=0x80d04a8)
    at ../WebCore/platform/network/curl/ResourceHandleManager.cpp:338
#5  0xb642481b in WebCore::Timer<WebCore::ResourceHandleManager>::fired (this=0x80d04a8) at ../WebCore/platform/Timer.h:98
#6  0xb631de54 in WebCore::TimerBase::fireTimers (fireTime=1200580262.381525, firingTimers=@0xbfaad97c)
    at ../WebCore/platform/Timer.cpp:336
#7  0xb631df5f in WebCore::TimerBase::sharedTimerFired () at ../WebCore/platform/Timer.cpp:357
#8  0xb64d119e in timeout_cb () at ../WebCore/platform/gtk/SharedTimerGtk.cpp:48
#9  0xb78bd8d6 in ?? () from /usr/lib/libglib-2.0.so.0
#10 0x00000000 in ?? ()
Comment 1 Cameron Zwarich (cpst) 2008-01-17 12:28:22 PST
Can you reproduce this with the latest nightly build? It may have been fixed by r29542.
Comment 2 Grzegorz D&#261;browski 2008-01-17 12:52:40 PST
I was using at least r29554, so the problem still exists.
Comment 3 Mark Rowe (bdash) 2008-01-17 23:02:04 PST
This is unlikely to be a bug in Vector.  

Frame 0 in the backtrace is:

#0  Vector (this=0xbfaad424, other=@0x8) at ../JavaScriptCore/wtf/Vector.h:513

which indicates a very low memory address, 0x8, is being passed to the copy constructor of Vector.

Frame 1 in the backtrace is:

#1  0xb6421785 in WebCore::ResourceHandleManager::setupPOST (this=0x80d04a8,
job=0x868c688, headers=0xbfaad54c)
    at ../WebCore/platform/network/curl/ResourceHandleManager.cpp:367

Line 367 of ResourceHandleManager.cpp is the following:

    Vector<FormDataElement> elements = job->request().httpBody()->elements();

The httpBody() method of ResourceRequest returns a FormData*.  If this return value is 0, then retrieving the m_elements member of it will result in a very small offset from 0, rather than a small offset from the address of the object.

The bug here is either that job->request().httpBody() is returning 0, or that the code in setupPOST does not handle that case correctly.
Comment 4 David Kilzer (:ddkilzer) 2008-01-19 10:39:31 PST
(In reply to comment #0)
> Sorry, I cannot add an URL for test case, because the page is from Intranet.

Can you post a reduced test case based on the original web page that reproduced the issue?  Start by saving the web page "as source", then removing HTML and JavaScript until you're left with just enough of the original page to reproduce the bug.

Comment 5 Grzegorz D&#261;browski 2008-01-19 12:08:41 PST
Created attachment 18545 [details]
Test case using jquery

I've added a test case for this bug. It's not extracted from original Intranet page, but it has the same effect.
Comment 6 Grzegorz D&#261;browski 2008-01-20 10:10:19 PST
Created attachment 18565 [details]
Simple test case

I've added another test case without jquery.
Comment 7 Julien Chaffraix 2008-03-11 15:42:48 PDT
Created attachment 19680 [details]
Add null checks for httpBody()

Thanks for the 2 test cases !
Comment 8 Holger Freyther 2008-03-12 02:33:01 PDT
(In reply to comment #7)
> Created an attachment (id=19680) [edit]
> Add null checks for httpBody()
> 
> Thanks for the 2 test cases !
> 

Yeah, you touched both cases. It would be appreciated if you could add the test cases to the LayoutTests and provide a result for it as well. It should be a simple if(layoutController) {layoutController.dumpAsText()} test result, there are many examples for crash fixes. We really need to get the regression test suite going for the Gtk+ port.
Comment 9 Julien Chaffraix 2008-03-14 12:12:22 PDT
Created attachment 19767 [details]
Added test case

Discussed with Zecke on IRC :

Added the reduced test case for safety even thought LayoutTests/http/tests/xmlhttprequest/methods-async.html did reproduce the crash.
Comment 10 Holger Freyther 2008-03-15 07:18:10 PDT
Comment on attachment 19767 [details]
Added test case

Thanks.
Comment 11 Holger Freyther 2008-03-15 07:19:02 PDT
Landed in r31074.