Bug 133069 - [Curl] Invalid content in cache file, causes broken rendering.
Summary: [Curl] Invalid content in cache file, causes broken rendering.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Curl
Depends on:
Blocks:
 
Reported: 2014-05-19 07:05 PDT by peavo
Modified: 2014-05-20 10:02 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.97 KB, patch)
2014-05-19 07:16 PDT, peavo
no flags Details | Formatted Diff | Diff
Patch (4.95 KB, patch)
2014-05-19 09:38 PDT, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.