Bug 213035

Summary: Crash growing a CFData with incremental PDF loading
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, darin
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Brady Eidson 2020-06-10 11:27:31 PDT
Crash growing a CFData with incremental PDF loading

<rdar://problem/63670403>
Comment 1 Brady Eidson 2020-06-10 14:43:21 PDT
Created attachment 401584 [details]
Patch
Comment 2 Alex Christensen 2020-06-10 14:44:52 PDT
Comment on attachment 401584 [details]
Patch

Shouldn't we call CFDataIncreaseLength right after this?
Comment 3 Brady Eidson 2020-06-10 14:45:39 PDT
(In reply to Alex Christensen from comment #2)
> Comment on attachment 401584 [details]
> Patch
> 
> Shouldn't we call CFDataIncreaseLength right after this?

We do (Read the whole function)
Comment 4 Brady Eidson 2020-06-10 14:46:46 PDT
(In reply to Brady Eidson from comment #3)
> (In reply to Alex Christensen from comment #2)
> > Comment on attachment 401584 [details]
> > Patch
> > 
> > Shouldn't we call CFDataIncreaseLength right after this?
> 
> We do (Read the whole function)

NM - Incomplete patch uploaded.
Comment 5 Brady Eidson 2020-06-10 14:47:28 PDT
Created attachment 401585 [details]
Patch
Comment 6 Darin Adler 2020-06-10 14:54:44 PDT
Comment on attachment 401585 [details]
Patch

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

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1628
> +        m_data = adoptCF(CFDataCreateMutable(0, 0));

Should be "nullptr, 0". But don’t have to do that in this patch.
Comment 7 Brady Eidson 2020-06-10 15:05:02 PDT
(In reply to Darin Adler from comment #6)
> Comment on attachment 401585 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401585&action=review
> 
> > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1628
> > +        m_data = adoptCF(CFDataCreateMutable(0, 0));
> 
> Should be "nullptr, 0". But don’t have to do that in this patch.

A followup will actually move off CFData to NSData, per the advice of experts
Comment 8 EWS 2020-06-10 15:23:40 PDT
Committed r262864: <https://trac.webkit.org/changeset/262864>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401585 [details].