Bug 78874 - Rename [CheckDomainSecurity] to [CheckSecurity]
Summary: Rename [CheckDomainSecurity] to [CheckSecurity]
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 77393
  Show dependency treegraph
 
Reported: 2012-02-16 21:38 PST by Kentaro Hara
Modified: 2012-02-19 15:47 PST (History)
3 users (show)

See Also:


Attachments
Patch (21.58 KB, patch)
2012-02-16 21:40 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (21.23 KB, patch)
2012-02-19 00:30 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-02-16 21:38:31 PST
For naming consistency with [CheckAccessToNode], we can rename the following IDL attributes:

[CheckDomainSecurity] => [CheckAccessToFrame]
[DoNotCheckDomainSecurity] => [DoNotCheckAccessToFrame]
[DoNotCheckDomainSecurityOnGetter] => [DoNotCheckAccessToFrameOnGetter]
[DoNotCheckDomainSecurityOnSetter] => [DoNotCheckAccessToFrameOnSetter]

Adam: Would it make sense? If so, I'll make a patch.
Comment 1 Kentaro Hara 2012-02-16 21:40:20 PST
Created attachment 127517 [details]
Patch
Comment 2 Adam Barth 2012-02-17 10:38:08 PST
Comment on attachment 127517 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        - [CheckDomainSecurity] => [CheckAccessToFrame]
> +        - [DoNotCheckDomainSecurity] => [DoNotCheckAccessToFrame]

I'm not super excited about mentioning "frame" here because that's not quite right.  We should be checking access to the ScriptExecutionContext (although we don't get that 100% correct yet).  How about:

CheckAccess
DoNotCheckAccess

?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1694
>                      push(@implContent, "    if (!castedThis->allowsAccessFrom(exec))\n");

For example, |castedThis| might not be a Frame.  We're just generally checking access.
Comment 3 Kentaro Hara 2012-02-18 19:46:19 PST
(In reply to comment #2)
> > +        - [CheckDomainSecurity] => [CheckAccessToFrame]
> > +        - [DoNotCheckDomainSecurity] => [DoNotCheckAccessToFrame]
> 
> I'm not super excited about mentioning "frame" here because that's not quite right.  We should be checking access to the ScriptExecutionContext (although we don't get that 100% correct yet).  How about:
> 
> CheckAccess
> DoNotCheckAccess

Thanks. Then [CheckDomainSecurity] might be better than [CheckAccess] in that it is clear that it is checking domain security. Instead, how about renaming [CheckAccessToNode] to [CheckDomainSecurityForNode]?
Comment 4 Adam Barth 2012-02-19 00:19:52 PST
> Instead, how about renaming [CheckAccessToNode] to [CheckDomainSecurityForNode]?

The problem is that "domain security" isn't a term that's used elsewhere.  We usually call it the same-origin policy...

How about just [CheckSecurity] or [CheckSameOriginPolicy]?  The name [CheckSecurityForNode] makes sense and seems better than [CheckDomainSecurityForNode] or [CheckSameOriginPolicyForNode]...
Comment 5 Kentaro Hara 2012-02-19 00:30:22 PST
Created attachment 127731 [details]
Patch
Comment 6 Kentaro Hara 2012-02-19 00:40:08 PST
(In reply to comment #4)
> How about just [CheckSecurity] or [CheckSameOriginPolicy]?  The name [CheckSecurityForNode] makes sense and seems better than [CheckDomainSecurityForNode] or [CheckSameOriginPolicyForNode]...

Sounds good! I uploaded patches:

[*CheckDomainSecurity*] => [*CheckSecurity*] : this patch
[CheckAccessToNode] => [CheckSecurityForNode] : bug 78991
Comment 7 WebKit Review Bot 2012-02-19 15:47:28 PST
Comment on attachment 127731 [details]
Patch

Clearing flags on attachment: 127731

Committed r108201: <http://trac.webkit.org/changeset/108201>
Comment 8 WebKit Review Bot 2012-02-19 15:47:34 PST
All reviewed patches have been landed.  Closing bug.