Bug 133069

Summary: [Curl] Invalid content in cache file, causes broken rendering.
Product: WebKit Reporter: peavo
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alex.christensen, bfulgham, commit-queue, galpeter, mmatyas
Priority: P2 Keywords: Curl
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description peavo 2014-05-19 07:05:35 PDT
When data for a url is received as multiple parts, the cache file for the url is truncated when opened for writing, and will only contain the last part of data received.
Comment 1 peavo 2014-05-19 07:16:34 PDT
Created attachment 231688 [details]
Patch
Comment 2 Brent Fulgham 2014-05-19 09:11:41 PDT
Comment on attachment 231688 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231688&action=review

r-: Please adjust a few functions for early return! :-)

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:365
> +    if (m_contentFile == invalidPlatformFileHandle) {

We prefer this as an early return:  "if (m_contentFile != invalidPlatformFileHandle) \r return true;"

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:366
> +        m_contentFile = openFile(m_contentFilename, OpenForWrite);

I was very surprised to see that we don't have an "OpenForWrite" style that does not concatenate the file!

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:367
> +        if (!isHandleValid(m_contentFile)) {

Again, we prefer to exit early if possible in tests like this.

> Source/WebCore/platform/network/curl/CurlCacheEntry.cpp:377
> +    if (m_contentFile != invalidPlatformFileHandle) {

Early return please.
Comment 3 peavo 2014-05-19 09:38:59 PDT
Created attachment 231694 [details]
Patch
Comment 4 peavo 2014-05-19 09:40:26 PDT
(In reply to comment #2)
> (From update of attachment 231688 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231688&action=review
> 

Thanks for reviewing :) Updated patch.

> 
> I was very surprised to see that we don't have an "OpenForWrite" style that does not concatenate the file!

Mee to :)
Comment 5 Brent Fulgham 2014-05-20 09:30:11 PDT
Comment on attachment 231694 [details]
Patch

Thanks for revising!  r=me.
Comment 6 WebKit Commit Bot 2014-05-20 10:02:20 PDT
Comment on attachment 231694 [details]
Patch

Clearing flags on attachment: 231694

Committed r169115: <http://trac.webkit.org/changeset/169115>
Comment 7 WebKit Commit Bot 2014-05-20 10:02:23 PDT
All reviewed patches have been landed.  Closing bug.