htmlunit-develop Mailing List for HtmlUnit (Page 18)

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

- **status**: open --> accepted
- **assigned_to**: RBRi



---

** [bugs:#1956] java.lang.StackOverflowError when div.style.length = 100**

**Status:** accepted
**Group:** Latest SVN
**Created:** Mon Mar 12, 2018 04:04 AM UTC by Atsushi Nakagawa
**Last Updated:** Sat Mar 17, 2018 04:53 PM UTC
**Owner:** RBRi


This bug report encompasses three problems caused by the same root cause.

I'll first outline the problems, then go over what I think is the cause, and then offer my suggested fix.

## Problems

* Setting `length` field of `CSSStyleDeclaration` in *non-strict mode* causes `java.lang.StackOverflowError`.
* Setting the `size` field of a `Map` should not be allowed but is possible in HtmlUnit in *non-strict mode*.
* Setting the `length` field of `javascript.host.dom.AbstractList` should not be allowed, but is possible in HtmlUnit, in both *strict mode* and *non-strict mode*.

## Test case

This test is for demonstrating the behaviour in both conventional browsers and HtmlUnit.

```html
<!DOCTYPE html>
<html>
<head>
<script>
function check(tn, func) {
	try { console.log(tn + ':', func()) } catch (e) { console.log(tn + ':', e.message) }
}

function test() {
	check('t1', function () { 'use strict'; var x = document.body.style; x.length = 100; return x.length })
	check('t2', function () { var x = document.body.style; x.length = 100; return x.length })

	check('t3', function () { 'use strict'; var x = new Map(); x.size = 100; return x.size })
	check('t4', function () { var x = new Map(); x.size = 100; return x.size })

	check('t5', function () { 'use strict'; var x = document.children; x.length = 100; return x.length })
	check('t6', function () { var x = document.children; x.length = 100; return x.length })
}
</script>
</head>
<body>
<input type="button" onclick="test()" value="test"/>
</body>
</html>
```

### Result in browsers

#### Chrome:

```text
t1: Cannot assign to read only property 'length' of object '#<CSSStyleDeclaration>'
t2: 0
t3: Cannot set property size of #<Map> which has only a getter
t4: 0
t5: Cannot assign to read only property 'length' of object '#<HTMLCollection>'
t6: 1
```

#### Firefox:

Odd lines differ as follows:

```text
t1: setting getter-only property "length"
t3: setting getter-only property "size"
t5: setting getter-only property "length"
```

#### Edge:

Odd lines differ as follows:

```text
t1: Assignment to read-only properties is not allowed in strict mode
t3: Assignment to read-only properties is not allowed in strict mode
t5: Assignment to read-only properties is not allowed in strict mode
```

### Result in HtmlUnit

```text
t1: Cannot set property [CSSStyleDeclaration].length that has only a getter to 100.

t3: Cannot set property [Map].size that has only a getter to 100.
t4: 100.0
t5. 100.0
t6. 100.0
```

`t2` doesn't complete but instead throws `java.lang.StackOverflowException`.

## Possible cause

1) The problem appears to be caused foremost by HtmlUnit's rhino fork [attempting to `setValue()` in a `ScriptableObject$GetterSlot` that has no setter](https://github.com/HtmlUnit/htmlunit-rhino-fork/blob/f41c211235de141394bff2ca1bf9ee02dfc3e58f/src/org/mozilla/javascript/ScriptableObject.java#L271).

As to *why* it's doing that, I think it's partially caused by a regression in [this commit](https://github.com/HtmlUnit/htmlunit-rhino-fork/commit/d01bf85dd0149d73e6c057d1c64c0fa2b780daae).  This commit message says *"Remove unused parameters"*  but it appears to be removing it in the logically opposite manner for the three parameters `FEATURE_HTMLUNIT_ASK_OBJECT_TO_WRITE_READONLY`, `FEATURE_HTMLUNIT_FUNCTION_NULL_SETTER` and `FEATURE_HTMLUNIT_EVAL_LOCAL_SCOPE`.

For example, the literal value of `FEATURE_HTMLUNIT_ASK_OBJECT_TO_WRITE_READONLY` was `false`.  So logically, "`if (FEATURE_HTMLUNIT_ASK_OBJECT_TO_WRITE_READONLY) { ... } else { ... }`" blocks should leave the "`else`" part.  The commit however leaves "`if`" part.

2) A follow up problem is that just reverting commit won't suffice as the previous code throws `TypeError` even in *non-strict mode*.  Judging from [surround code](https://github.com/HtmlUnit/htmlunit-rhino-fork/blob/f41c211235de141394bff2ca1bf9ee02dfc3e58f/src/org/mozilla/javascript/ScriptableObject.java#L2736), it should just `return true` in *non-strict mode*.

3) Lastly, a problem that will still break t5 and t6 is this line in [`AbstractList.has(String, Scriptable)`](https://sourceforge.net/p/htmlunit/code/15163/tree/trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/dom/AbstractList.java#l468) which makes HtmlUnit look for the `length` field in the *object* instance even though it should be looking at the *prototype* instance.

This simple test in Chrone / FF / Edge shows there is no `"length"` field  on the object for `document.children` but there is on `__proto__`.

```text
> Object.getOwnPropertyDescriptor(document.children, 'length')
< undefined
> Object.getOwnPropertyDescriptor(document.children.__proto__, 'length')
< {get: ƒ, set: undefined, enumerable: true, configurable: true}
```

## Suggested fix

These are the fixes we're using locally.

#### Fix for `GetterSlot.setValue(Object value, Scriptable owner, Scriptable start)`.

This change to [`ScriptableObject$GetterSlot.setValue()`](https://github.com/HtmlUnit/htmlunit-rhino-fork/blob/f41c211235de141394bff2ca1bf9ee02dfc3e58f/src/org/mozilla/javascript/ScriptableObject.java#L260) puts it more in line with [`ScriptableObject$Slot.setValue()`]( https://github.com/HtmlUnit/htmlunit-rhino-fork/blob/f41c211235de141394bff2ca1bf9ee02dfc3e58f/src/org/mozilla/javascript/ScriptableObject.java#L180).

```diff
@@ -286,32 +286,14 @@ ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) {
         @Override
         boolean setValue(Object value, Scriptable owner, Scriptable start) {
             if (setter == null) {
                 if (getter != null) {
                     Context cx = Context.getContext();
-                    if (cx.isStrictMode() ||
-                        // Based on TC39 ES3.1 Draft of 9-Feb-2009, 8.12.4, step 2,
-                        // we should throw a TypeError in this case.
-                        cx.hasFeature(Context.FEATURE_STRICT_MODE)) {
-
+                    if (cx.isStrictMode()) {
                         throw ScriptRuntime.typeError3("msg.set.prop.no.setter", name, start.getClassName(), Context.toString(value));
                     }
-                    Scriptable scriptable = start;
-
-                    if (scriptable instanceof Delegator) {
-                        scriptable = ((Delegator) scriptable).getDelegee();
-                    }
-
-                    if (scriptable instanceof ScriptableObject) {
-                        boolean allowSetting = ((ScriptableObject) scriptable).isReadOnlySettable(name, value);
-                        if (!allowSetting) {
-                            return true;
-                        }
-                    }
-                    if (owner == start) {
-                        getter = null;
-                    }
+                    return true;
                 }
             } else {
                 Context cx = Context.getContext();
                 if (setter instanceof MemberBox) {
                     MemberBox nativeSetter = (MemberBox)setter;
```

This change obseletes [`ScriptableObject.isReadOnlySettable()`](https://github.com/HtmlUnit/htmlunit-rhino-fork/blob/f41c211235de141394bff2ca1bf9ee02dfc3e58f/src/org/mozilla/javascript/ScriptableObject.java#L3288), [`SimpleScriptable.isReadOnlySettable()`](https://sourceforge.net/p/htmlunit/code/15163/tree/trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/SimpleScriptable.java#l392) and the annotation [`CanSetReadOnly`](https://sourceforge.net/p/htmlunit/code/15163/tree/trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/configuration/CanSetReadOnly.java) as they become unused code.  They might be salvagable but `isReadOnlySettable()`'s default return value is `true` which isn't easy to work with, and I don't know the use case.

#### Extra fix for `AbstractList.has(final String name, final Scriptable start)`.

This is the extra fix to [`AbstractList.has(final String name, final Scriptable start)`](https://sourceforge.net/p/htmlunit/code/15163/tree/trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/dom/AbstractList.java#l455) is needed for t5 and t6.

```diff
@@ -457,13 +457,10 @@ public boolean has(final String name, final Scriptable start) {
         }
         catch (final NumberFormatException e) {
             // Ignore.
         }
 
-        if ("length".equals(name)) {
-            return true;
-        }
         final BrowserVersion browserVersion = getBrowserVersion();
         if (browserVersion.hasFeature(JS_NODE_LIST_ENUMERATE_FUNCTIONS)) {
             final JavaScriptConfiguration jsConfig = getWindow().getWebWindow().getWebClient()
                     .getJavaScriptEngine().getJavaScriptConfiguration();
             if (jsConfig.getClassConfiguration(getClassName()).getFunctionKeys().contains(name)) {
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

2002	Jan	Feb	Mar	Apr	May	Jun	Jul (10)	Aug (30)	Sep (15)	Oct (26)	Nov (12)	Dec (17)
2003	Jan (7)	Feb (27)	Mar (73)	Apr (17)	May (17)	Jun (78)	Jul (67)	Aug (60)	Sep (89)	Oct (140)	Nov (173)	Dec (46)
2004	Jan (39)	Feb (7)	Mar (21)	Apr (31)	May (13)	Jun (86)	Jul (14)	Aug (14)	Sep (53)	Oct (184)	Nov (186)	Dec (319)
2005	Jan (336)	Feb (274)	Mar (226)	Apr (102)	May (196)	Jun (130)	Jul (119)	Aug (143)	Sep (76)	Oct (85)	Nov (70)	Dec (159)
2006	Jan (125)	Feb (100)	Mar (80)	Apr (39)	May (55)	Jun (58)	Jul (50)	Aug (76)	Sep (55)	Oct (101)	Nov (163)	Dec (85)
2007	Jan (56)	Feb (53)	Mar (180)	Apr (221)	May (290)	Jun (199)	Jul (322)	Aug (515)	Sep (121)	Oct (297)	Nov (177)	Dec (103)
2008	Jan (516)	Feb (315)	Mar (586)	Apr (615)	May (197)	Jun (381)	Jul (390)	Aug (195)	Sep (603)	Oct (499)	Nov (622)	Dec (350)
2009	Jan (313)	Feb (338)	Mar (507)	Apr (317)	May (197)	Jun (375)	Jul (235)	Aug (424)	Sep (410)	Oct (338)	Nov (286)	Dec (306)
2010	Jan (367)	Feb (339)	Mar (371)	Apr (172)	May (233)	Jun (264)	Jul (421)	Aug (110)	Sep (218)	Oct (189)	Nov (185)	Dec (168)
2011	Jan (145)	Feb (213)	Mar (205)	Apr (64)	May (159)	Jun (67)	Jul (104)	Aug (126)	Sep (144)	Oct (106)	Nov (154)	Dec (225)
2012	Jan (111)	Feb (87)	Mar (131)	Apr (102)	May (180)	Jun (160)	Jul (412)	Aug (315)	Sep (311)	Oct (369)	Nov (464)	Dec (284)
2013	Jan (343)	Feb (165)	Mar (174)	Apr (120)	May (153)	Jun (134)	Jul (202)	Aug (105)	Sep (228)	Oct (332)	Nov (192)	Dec (219)
2014	Jan (348)	Feb (194)	Mar (189)	Apr (188)	May (297)	Jun (206)	Jul (79)	Aug (279)	Sep (111)	Oct (159)	Nov (61)	Dec (78)
2015	Jan (152)	Feb (145)	Mar (239)	Apr (223)	May (248)	Jun (296)	Jul (172)	Aug (189)	Sep (338)	Oct (217)	Nov (131)	Dec (184)
2016	Jan (118)	Feb (221)	Mar (414)	Apr (412)	May (303)	Jun (133)	Jul (129)	Aug (121)	Sep (136)	Oct (67)	Nov (89)	Dec (245)
2017	Jan (349)	Feb (90)	Mar (328)	Apr (430)	May (284)	Jun (199)	Jul (164)	Aug (120)	Sep (57)	Oct (105)	Nov (108)	Dec (146)
2018	Jan (85)	Feb (48)	Mar (97)	Apr (62)	May (64)	Jun (136)	Jul (123)	Aug (87)	Sep (17)	Oct (27)	Nov (9)	Dec (16)
2019	Jan (9)	Feb (17)	Mar (18)	Apr (14)	May (8)	Jun	Jul (6)	Aug (12)	Sep (5)	Oct	Nov (2)	Dec
2020	Jan (8)	Feb	Mar (6)	Apr	May	Jun	Jul (2)	Aug	Sep (4)	Oct (1)	Nov	Dec
2021	Jan	Feb	Mar	Apr (2)	May (4)	Jun	Jul	Aug	Sep	Oct (1)	Nov (4)	Dec
2022	Jan	Feb	Mar	Apr (1)	May	Jun	Jul	Aug	Sep	Oct	Nov (2)	Dec
2023	Jan	Feb (6)	Mar (9)	Apr	May	Jun	Jul	Aug	Sep	Oct	Nov	Dec
2024	Jan	Feb	Mar	Apr	May (2)	Jun	Jul	Aug	Sep	Oct	Nov	Dec

htmlunit-develop Mailing List for HtmlUnit (Page 18)

Java GUI-Less browser, supporting JavaScript, to run against web pages

htmlunit-develop — General discussion of the development of HtmlUnit