QuickJS: fix XML value leaks in "in" checks

Previously, property existence checks such as "'$attr$a' in node",
"'to' in node", or "'$tags$to' in node" created the property value and
never released it when no property descriptor was requested.  A leaked
XMLNode also held a reference to its document, so the whole document
was never freed.

Also, qjs_xml_node_make() and qjs_xml_attr_make() incremented the
document reference count before creating the object, so a failed
JS_NewObjectClass() left the document permanently referenced.

The fix is to free the value when no descriptor is requested and to
take the document reference only after the object is created.

diff --git a/external/qjs_xml_module.c b/external/qjs_xml_module.c
index 82b4786ac4b303596c086c9bf72f60feae523045..c0f795c7d68d97250b6cfe7c3b079ba7e4835bd9 100644 (file)
--- a/external/qjs_xml_module.c
+++ b/external/qjs_xml_module.c
@@ -579,7 +579,6 @@ qjs_xml_node_make(JSContext *cx, qjs_xml_doc_t *doc, xmlNode *node)
 
     current->node = node;
     current->doc = doc;
-    doc->ref_count++;
 
     ret = JS_NewObjectClass(cx, QJS_CORE_CLASS_ID_XML_NODE);
     if (JS_IsException(ret)) {
@@ -587,6 +586,7 @@ qjs_xml_node_make(JSContext *cx, qjs_xml_doc_t *doc, xmlNode *node)
         return ret;
     }
 
+    doc->ref_count++;
     JS_SetOpaque(ret, current);
 
     return ret;
@@ -1064,13 +1064,16 @@ qjs_xml_node_get_own_property(JSContext *cx, JSPropertyDescriptor *pdesc,
                 return 0;
             }
 
-            if (pdesc != NULL) {
-                pdesc->flags = JS_PROP_ENUMERABLE;
-                pdesc->getter = JS_UNDEFINED;
-                pdesc->setter = JS_UNDEFINED;
-                pdesc->value  = value;
+            if (pdesc == NULL) {
+                JS_FreeValue(cx, value);
+                return 1;
             }
 
+            pdesc->flags = JS_PROP_ENUMERABLE;
+            pdesc->getter = JS_UNDEFINED;
+            pdesc->setter = JS_UNDEFINED;
+            pdesc->value  = value;
+
             return 1;
         }
 
@@ -1176,13 +1179,16 @@ qjs_xml_node_get_own_property(JSContext *cx, JSPropertyDescriptor *pdesc,
                 return -1;
             }
 
-            if (pdesc != NULL) {
-                pdesc->flags = JS_PROP_ENUMERABLE;
-                pdesc->getter = JS_UNDEFINED;
-                pdesc->setter = JS_UNDEFINED;
-                pdesc->value = value;
+            if (pdesc == NULL) {
+                JS_FreeValue(cx, value);
+                return 1;
             }
 
+            pdesc->flags = JS_PROP_ENUMERABLE;
+            pdesc->getter = JS_UNDEFINED;
+            pdesc->setter = JS_UNDEFINED;
+            pdesc->value = value;
+
             return 1;
         }
 
@@ -1235,13 +1241,16 @@ tag:
         return 0;
     }
 
-    if (pdesc != NULL) {
-        pdesc->flags = JS_PROP_ENUMERABLE;
-        pdesc->getter = JS_UNDEFINED;
-        pdesc->setter = JS_UNDEFINED;
-        pdesc->value  = value;
+    if (pdesc == NULL) {
+        JS_FreeValue(cx, value);
+        return 1;
     }
 
+    pdesc->flags = JS_PROP_ENUMERABLE;
+    pdesc->getter = JS_UNDEFINED;
+    pdesc->setter = JS_UNDEFINED;
+    pdesc->value  = value;
+
     return 1;
 }
 
@@ -1665,7 +1674,6 @@ qjs_xml_attr_make(JSContext *cx, qjs_xml_doc_t *doc, xmlNode *node)
 
     current->node = node;
     current->doc = doc;
-    doc->ref_count++;
 
     ret = JS_NewObjectClass(cx, QJS_CORE_CLASS_ID_XML_ATTR);
     if (JS_IsException(ret)) {
@@ -1673,6 +1681,7 @@ qjs_xml_attr_make(JSContext *cx, qjs_xml_doc_t *doc, xmlNode *node)
         return ret;
     }
 
+    doc->ref_count++;
     JS_SetOpaque(ret, current);
 
     return ret;

diff --git a/test/xml/xml.t.mjs b/test/xml/xml.t.mjs
index e4c8bf75700b583caf2a09c3daf598383c392162..aff28956c8e0d3259e3d9c780e9f7c9bf0aa031a 100644 (file)
--- a/test/xml/xml.t.mjs
+++ b/test/xml/xml.t.mjs
@@ -38,6 +38,11 @@ let parse_tsuite = {
         { get: (doc) => doc.note.to.$attrs.b, expected: 'bar' },
         { get: (doc) => doc.note.to.$attr$b, expected: 'bar' },
         { get: (doc) => doc.note.$attr$a, expected: undefined },
+        { get: (doc) => '$attr$a' in doc.note.to, expected: true },
+        { get: (doc) => '$attr$missing' in doc.note.to, expected: false },
+        { get: (doc) => 'to' in doc.note, expected: true },
+        { get: (doc) => 'missing' in doc.note, expected: false },
+        { get: (doc) => '$tags$to' in doc.note, expected: true },
         { get: (doc) => Array.isArray(doc.note.$tags), expected: true },
         { get: (doc) => doc.note.$tags[0].$text, expected: 'Tove' },
         { get: (doc) => doc.note.$tags[1].$text, expected: 'Jani' },