<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[202030] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/202030">202030</a></dd>
<dt>Author</dt> <dd>barraclough@apple.com</dd>
<dt>Date</dt> <dd>2016-06-13 23:17:05 -0700 (Mon, 13 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate)
https://bugs.webkit.org/show_bug.cgi?id=158431
Reviewed by Geoff Garen.
All uses of hasStaticPropertyTable flag generated by bindings are wrong.
JSLocation::putDelegate checks the static property table redundantly.
In the case of same origin access, if the property is not in the static
table the method will call JSObject::put and return true (indicating the
delegate handled the put). If the property is in the static table, the
method will return false (indicating the the delegate did not handle the
access) - in which case the calling function will call JSObject::put.
Checking for the property in the static table is redundant - same origin
access does not require any special handling, and should just always
return false & let the caller handle the put.
In the case of cross origin access, if the property is not in the static
table we return true (indicating the access was handled, and silently
blocking it). If it is a static property, we check the name, and if the
name is not 'href' we also return true, silently blocking. In the case
that the name is 'href' we'll return false, indicating to the caller
that the access was not handled by the delegate, resulting in it taking
place. The additional check of the static table is redundant, since we
only have special behaviour in the case of 'href'. (Moreover it is
unnecesszarily fragile, since if we made a change such that 'href' was no
longer implemented as a static property with would fail.)
- for same origin, always return false.
- for cross origin, return false for 'href', otherwise return true.
* bindings/js/JSLocationCustom.cpp:
(WebCore::JSLocation::putDelegate):
- restructure & remove static table check.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSLocationCustomcpp">trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (202029 => 202030)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-06-14 06:09:53 UTC (rev 202029)
+++ trunk/Source/WebCore/ChangeLog 2016-06-14 06:17:05 UTC (rev 202030)
</span><span class="lines">@@ -1,5 +1,43 @@
</span><span class="cx"> 2016-06-13 Gavin & Ellie Barraclough <barraclough@apple.com>
</span><span class="cx">
</span><ins>+ Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate)
+ https://bugs.webkit.org/show_bug.cgi?id=158431
+
+ Reviewed by Geoff Garen.
+
+ All uses of hasStaticPropertyTable flag generated by bindings are wrong.
+
+ JSLocation::putDelegate checks the static property table redundantly.
+
+ In the case of same origin access, if the property is not in the static
+ table the method will call JSObject::put and return true (indicating the
+ delegate handled the put). If the property is in the static table, the
+ method will return false (indicating the the delegate did not handle the
+ access) - in which case the calling function will call JSObject::put.
+ Checking for the property in the static table is redundant - same origin
+ access does not require any special handling, and should just always
+ return false & let the caller handle the put.
+
+ In the case of cross origin access, if the property is not in the static
+ table we return true (indicating the access was handled, and silently
+ blocking it). If it is a static property, we check the name, and if the
+ name is not 'href' we also return true, silently blocking. In the case
+ that the name is 'href' we'll return false, indicating to the caller
+ that the access was not handled by the delegate, resulting in it taking
+ place. The additional check of the static table is redundant, since we
+ only have special behaviour in the case of 'href'. (Moreover it is
+ unnecesszarily fragile, since if we made a change such that 'href' was no
+ longer implemented as a static property with would fail.)
+
+ - for same origin, always return false.
+ - for cross origin, return false for 'href', otherwise return true.
+
+ * bindings/js/JSLocationCustom.cpp:
+ (WebCore::JSLocation::putDelegate):
+ - restructure & remove static table check.
+
+2016-06-13 Gavin & Ellie Barraclough <barraclough@apple.com>
+
</ins><span class="cx"> Remove hasStaticPropertyTable (part 2: JSPluginElement)
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=158431
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSLocationCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp (202029 => 202030)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp 2016-06-14 06:09:53 UTC (rev 202029)
+++ trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp 2016-06-14 06:17:05 UTC (rev 202030)
</span><span class="lines">@@ -60,7 +60,7 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool JSLocation::putDelegate(ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot, bool& putResult)
</del><ins>+bool JSLocation::putDelegate(ExecState* exec, PropertyName propertyName, JSValue, PutPropertySlot&, bool& putResult)
</ins><span class="cx"> {
</span><span class="cx"> putResult = false;
</span><span class="cx"> Frame* frame = wrapped().frame();
</span><span class="lines">@@ -70,23 +70,13 @@
</span><span class="cx"> if (propertyName == exec->propertyNames().toString || propertyName == exec->propertyNames().valueOf)
</span><span class="cx"> return true;
</span><span class="cx">
</span><del>- bool sameDomainAccess = shouldAllowAccessToFrame(exec, frame);
</del><ins>+ if (shouldAllowAccessToFrame(exec, frame))
+ return false;
</ins><span class="cx">
</span><del>- static_assert(hasStaticPropertyTable, "The implementation dereferences ClassInfo::staticPropHashTable without null check");
- const HashTableValue* entry = JSLocation::info()->staticPropHashTable->entry(propertyName);
- if (!entry) {
- if (sameDomainAccess)
- putResult = JSObject::put(this, exec, propertyName, value, slot);
- return true;
- }
-
</del><span class="cx"> // Cross-domain access to the location is allowed when assigning the whole location,
</span><del>- // but not when assigning the individual pieces, since that might inadvertently
</del><ins>+ // but not when assigning the individual pieces, since that might inadvertently
</ins><span class="cx"> // disclose other parts of the original location.
</span><del>- if (propertyName != exec->propertyNames().href && !sameDomainAccess)
- return true;
-
- return false;
</del><ins>+ return propertyName != exec->propertyNames().href;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool JSLocation::deleteProperty(JSCell* cell, ExecState* exec, PropertyName propertyName)
</span></span></pre>
</div>
</div>
</body>
</html>