Summary: | Add defensive initialization of iframe sandbox flags | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Patrik Persson <patrik.j.persson> | ||||||
Component: | Frames | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, darin | ||||||
Priority: | P3 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Patrik Persson
2009-12-10 01:50:38 PST
I think for HTMLFrameOwnerElement this issue probably doesn't apply. Using the DOM you can create a frame element a step at a time, and you can start using it at any point, so there is no real "illegal value" time for the DOM elements. For SecurityOrigin I think we might be able to make it happen, though. Created attachment 46230 [details]
Work in progress
Created attachment 46242 [details]
Patch
Comment on attachment 46242 [details] Patch > if (shouldTreatURLSchemeAsNoAccess(m_protocol)) > m_isUnique = true; > > - // If this ASSERT becomes false in the future, please consider the impact > - // of m_sandoxFlags on m_isUnique. > - ASSERT(m_sandboxFlags == SandboxNone); > + if (isSandboxed(SandboxOrigin)) > + m_isUnique = true; I suggest initializing m_unique to the correct value from the outset. m_isUnique(shouldTreatURLSchemeAsNoAccess(m_protocol) || isSandboxed(SandboxOrigin)) To some the if statements may be slightly clearer, so it seems OK to leave it this way if you think that way. > + static PassRefPtr<SecurityOrigin> create(const KURL&, SandboxFlags sandboxFlags = SandboxNone); The argument name isn't needed here because the type speaks for itself. > > + static PassRefPtr<SecurityOrigin> create(const KURL&, SandboxFlags sandboxFlags = SandboxNone);
>
> The argument name isn't needed here because the type speaks for itself.
Oh, I didn't know you could omit the argument name and provide a default value.
Thanks for the review.
Comments addressed and landed in http://trac.webkit.org/changeset/53056 |