Bug 154156 – Separate out !allowsAccess path in JSDOMWindowCustom getOwnPropertySlot

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Gavin Barraclough]

Make the decision as to what we allow access to more explicit.

[Gavin Barraclough]

Created attachment 271138 [details]
WIP

[WebKit Commit Bot]

Attachment 271138 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:126:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:160:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:160:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:161:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:162:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:163:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:164:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:165:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:166:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:167:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 10 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gavin Barraclough]

Created attachment 271139 [details]
WIP 2

[Build Bot]

Comment on attachment 271139 [details]
WIP 2

Attachment 271139 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/818151

New failing tests:
http/tests/security/xss-DENIED-window-name-navigator.html
imported/w3c/web-platform-tests/html/dom/interfaces.html
fast/dom/Window/window-postmessage-clone-frames.html

[Build Bot]

Created attachment 271143 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 271139 [details]
WIP 2

Attachment 271139 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/818150

New failing tests:
http/tests/security/xss-DENIED-window-name-navigator.html
imported/w3c/web-platform-tests/html/dom/interfaces.html

[Build Bot]

Created attachment 271144 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 271139 [details]
WIP 2

Attachment 271139 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/818149

New failing tests:
http/tests/security/xss-DENIED-window-name-navigator.html
fast/dom/Window/window-postmessage-clone-frames.html

[Build Bot]

Created attachment 271145 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Gavin Barraclough]

Created attachment 271152 [details]
Patch

[WebKit Commit Bot]

Attachment 271152 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:148:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gavin Barraclough]

Created attachment 271153 [details]
Patch

[Build Bot]

Comment on attachment 271153 [details]
Patch

Attachment 271153 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/818581

New failing tests:
http/tests/security/document-all.html
http/tests/security/window-named-valueOf.html
http/tests/security/window-named-proto.html

[Build Bot]

Created attachment 271157 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 271153 [details]
Patch

Attachment 271153 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/818605

New failing tests:
http/tests/security/document-all.html
http/tests/security/window-named-valueOf.html
http/tests/security/window-named-proto.html

[Build Bot]

Created attachment 271158 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 271153 [details]
Patch

Attachment 271153 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/818600

New failing tests:
http/tests/security/document-all.html
http/tests/security/window-named-valueOf.html
http/tests/security/window-named-proto.html

[Build Bot]

Created attachment 271159 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Gavin Barraclough]

Created attachment 271184 [details]
Patch

[Chris Dumez]

Comment on attachment 271184 [details]
Patch

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

r=me

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:115
> +static bool jsDOMWindowGetOwnPropertySlotDisallowAccess(JSDOMWindow* thisObject, ExecState* exec, PropertyName propertyName, PropertySlot& slot, String& errorMessage)

I would have gone with "CrossOrigin" instead of "DisallowAccess" but no big deal.

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:160
> +        // For any other entrires in the static property table, deny access. (Early return also prevents

"entries"

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:175
> +        if (asObject(proto)->getPropertySlot(exec, propertyName, slot)) {

Could be in the previous if condition with &&

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:200
> +    if (auto scopedChild = thisObject->wrapped().frame()->tree().scopedChild(propertyNameToAtomicString(propertyName))) {

auto* for clarity

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:316
> +        if (asObject(proto)->getPropertySlot(exec, propertyName, slot))

Could be in the previous if condition with &&. Then we can loose the curly brackets.

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:331
> +    if (auto scopedChild = thisObject->wrapped().frame()->tree().scopedChild(propertyNameToAtomicString(propertyName))) {

auto*

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:390
> +    if (auto scopedChild = thisObject->wrapped().frame()->tree().scopedChild(propertyNameToAtomicString(propertyName))) {

auto*

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:399
> +        if (asObject(proto)->getPropertySlot(exec, index, slot))

Could be in previous if condition with &&

[Gavin Barraclough]

Made all code review changes; Committed revision 196494.