From: Dmitry Volyntsev Date: Wed, 16 Sep 2026 23:17:58 +0000 (-0700) Subject: XML: return complete attribute values X-Git-Tag: 1.0.2~1 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=aaf322c9d0fb7c753223d5d628ddcedea8a00e84;p=njs.git XML: return complete attribute values Previously, $attr$ returned undefined for attributes containing entity references, while $attrs returned only the first text fragment. libxml2 canonicalization expands the complete attribute value, creating an interpretation difference for signed XML consumers. Use xmlNodeGetContent() for both getters in the njs and QuickJS backends. This preserves the existing explicit-attribute lookup and makes the JavaScript value agree with canonicalization. --- diff --git a/external/njs_xml_module.c b/external/njs_xml_module.c index d58e222c..26c3cd1c 100644 --- a/external/njs_xml_module.c +++ b/external/njs_xml_module.c @@ -102,6 +102,8 @@ static njs_int_t njs_xml_node_tag_handler(njs_vm_t *vm, njs_value_t *value, njs_str_t *name, njs_value_t *setval, njs_value_t *retval); static njs_int_t njs_xml_node_tags_handler(njs_vm_t *vm, njs_value_t *value, njs_str_t *name, njs_value_t *setval, njs_value_t *retval); +static njs_int_t njs_xml_attr_value(njs_vm_t *vm, xmlAttr *attr, + njs_value_t *retval); static xmlNode *njs_xml_external_node(njs_vm_t *vm, njs_value_t *value); static const u_char *njs_xml_string_to_c_string(njs_vm_t *vm, njs_str_t *str, @@ -1157,7 +1159,7 @@ njs_xml_node_attr_handler(njs_vm_t *vm, xmlNode *current, njs_str_t *name, njs_int_t ret; njs_str_t str; xmlAttr *attr; - const u_char *content, *name_c, *value; + const u_char *name_c, *value; u_char name_buf[512], value_buf[1024]; if (retval != NULL && setval == NULL) { @@ -1176,15 +1178,7 @@ njs_xml_node_attr_handler(njs_vm_t *vm, xmlNode *current, njs_str_t *name, continue; } - if (attr->children != NULL - && attr->children->next == NULL - && attr->children->type == XML_TEXT_NODE) - { - content = (const u_char *) attr->children->content; - - return njs_vm_value_string_create(vm, retval, content, - njs_strlen(content)); - } + return njs_xml_attr_value(vm, attr, retval); } njs_value_undefined_set(retval); @@ -1250,6 +1244,25 @@ njs_xml_node_attr_handler(njs_vm_t *vm, xmlNode *current, njs_str_t *name, } +static njs_int_t +njs_xml_attr_value(njs_vm_t *vm, xmlAttr *attr, njs_value_t *retval) +{ + njs_int_t ret; + xmlChar *text; + + text = xmlNodeGetContent((xmlNode *) attr); + if (njs_slow_path(text == NULL)) { + njs_vm_internal_error(vm, "xmlNodeGetContent() failed"); + return NJS_ERROR; + } + + ret = njs_vm_value_string_create(vm, retval, text, njs_strlen(text)); + xmlFree(text); + + return ret; +} + + static njs_int_t njs_xml_node_tag_remove(njs_vm_t *vm, njs_value_t *value, njs_str_t *name) { @@ -2153,8 +2166,7 @@ njs_xml_attr_ext_prop_handler(njs_vm_t *vm, njs_object_prop_t *prop, continue; } - return njs_vm_value_string_create(vm, retval, node->children->content, - njs_strlen(node->children->content)); + return njs_xml_attr_value(vm, node, retval); } njs_value_undefined_set(retval); diff --git a/external/qjs_xml_module.c b/external/qjs_xml_module.c index e210ad9e..14cb0633 100644 --- a/external/qjs_xml_module.c +++ b/external/qjs_xml_module.c @@ -73,6 +73,7 @@ static JSValue qjs_xml_node_make(JSContext *cx, qjs_xml_doc_t *doc, xmlNode *node); static int qjs_xml_node_has_method(JSContext *cx, JSValueConst obj, JSAtom prop, njs_str_t *name); +static JSValue qjs_xml_attr_value(JSContext *cx, xmlAttr *attr); static int qjs_xml_node_get_own_property(JSContext *cx, JSPropertyDescriptor *pdesc, JSValueConst obj, JSAtom prop); static int qjs_xml_node_get_own_property_names(JSContext *cx, @@ -707,19 +708,32 @@ qjs_xml_node_attr_handler(JSContext *cx, qjs_xml_node_t *current, continue; } - if (attr->children != NULL - && attr->children->next == NULL - && attr->children->type == XML_TEXT_NODE - && attr->children->content != NULL) - { - return JS_NewString(cx, (char *) attr->children->content); - } + return qjs_xml_attr_value(cx, attr); } return JS_UNDEFINED; } +static JSValue +qjs_xml_attr_value(JSContext *cx, xmlAttr *attr) +{ + JSValue value; + xmlChar *text; + + text = xmlNodeGetContent((xmlNode *) attr); + if (text == NULL) { + JS_ThrowInternalError(cx, "xmlNodeGetContent() failed"); + return JS_EXCEPTION; + } + + value = JS_NewString(cx, (char *) text); + xmlFree(text); + + return value; +} + + static int qjs_xml_node_attr_modify(JSContext *cx, JSValue current, const u_char *name, JSValue setval) @@ -1829,7 +1843,6 @@ qjs_xml_attr_get_own_property(JSContext *cx, JSPropertyDescriptor *pdesc, JSValueConst obj, JSAtom prop) { size_t size; - u_char *text; xmlAttr *attr; njs_str_t name; qjs_xml_attr_t *current; @@ -1862,24 +1875,16 @@ qjs_xml_attr_get_own_property(JSContext *cx, JSPropertyDescriptor *pdesc, JS_FreeCString(cx, (char *) name.start); - text = xmlNodeGetContent(attr->children); - if (text == NULL) { - return 0; - } - if (pdesc != NULL) { pdesc->flags = JS_PROP_ENUMERABLE; pdesc->getter = JS_UNDEFINED; pdesc->setter = JS_UNDEFINED; - pdesc->value = JS_NewString(cx, (char *) text); + pdesc->value = qjs_xml_attr_value(cx, attr); if (JS_IsException(pdesc->value)) { - xmlFree(text); return -1; } } - xmlFree(text); - return 1; } diff --git a/test/xml/saml_verify.t.mjs b/test/xml/saml_verify.t.mjs index 65d4e06b..7cdbfcf6 100644 --- a/test/xml/saml_verify.t.mjs +++ b/test/xml/saml_verify.t.mjs @@ -294,9 +294,45 @@ let saml_verify_tsuite = { { saml: "response_assertion_and_message_signed.xml", expected: true }, { saml: "auth_r.xml", sign: true, key: { sign_file: "rsa.pkcs8" }, expected: true }, -]}; + ]}; + +let entity_attribute_tsuite = { + name: "signed entity-backed attribute", + skip: () => (!has_njs() || !has_webcrypto()), + T: async (params) => { + let private_key = fs.readFileSync('test/webcrypto/rsa.pkcs8'); + let public_key = fs.readFileSync('test/webcrypto/rsa.spki'); + let source = xml.parse(fs.readFileSync('test/xml/auth_r.xml')); + let root = source.$root; + + root.setAttribute('NotOnOrAfter', '2024-01-18T06:21:48Z'); + await signSAML(source, private_key); + + let saml = xml.parse(` + ]>${xml.c14n(source).toString().replace('2024-01-18T06:21:48Z', + '&njs_entity;')}`); + + if (!await verifySAMLSignature(saml, public_key)) { + throw Error('signature verification failed'); + } + + root = saml.$root; + if (root.$attr$NotOnOrAfter !== '2024-01-18T06:21:48Z' + || root.$attrs.NotOnOrAfter !== '2024-01-18T06:21:48Z') + { + throw Error('signed attribute value mismatch'); + } + + return 'SUCCESS'; + }, + + tests: [ + {}, + ]}; run([ saml_verify_tsuite, + entity_attribute_tsuite, ]) .then($DONE, $DONE); diff --git a/test/xml/xml.t.mjs b/test/xml/xml.t.mjs index 220c4b7e..51b94bab 100644 --- a/test/xml/xml.t.mjs +++ b/test/xml/xml.t.mjs @@ -168,6 +168,24 @@ let parse_tsuite = { get: (doc) => JSON.stringify([doc['俄语'].$name[1],doc['俄语']['$attr$լեզու'][7],doc['俄语'].$text[5]]), expected: '["语","ն","е"]' }, + { doc: `]>`, + get: (doc) => JSON.stringify([doc.r.$attr$a, doc.r.$attrs.a]), + expected: '["value","value"]' }, + { doc: `]>`, + get: (doc) => JSON.stringify([doc.r.$attr$a, doc.r.$attrs.a, + xml.serializeToString(doc.r)]), + expected: '["left-middle-right","left-middle-right",' + + '""]' }, + { doc: `]>`, + get: (doc) => JSON.stringify([doc.r.$attr$a, doc.r.$attrs.a]), + expected: '["AB","AB"]' }, + { doc: `]>`, + get: (doc) => JSON.stringify([doc.r.$attr$a, doc.r.$attrs.a]), + expected: '["",""]' }, + { doc: `]>`, + get: (doc) => JSON.stringify([doc.r.$attr$a, doc.r.$attrs]), + expected: '[null,null]' }, + { get: (doc) => JSON.stringify(Object.getOwnPropertyNames(doc)), expected: '["note"]' }, { get: (doc) => JSON.stringify(Object.getOwnPropertyNames(doc.note)), @@ -291,6 +309,9 @@ let c14n_tsuite = { { doc: `ToveJani`, call: (doc) => xml.exclusiveC14n(doc.note, doc.note.to), expected: `Jani` }, + { doc: `]>`, + call: (doc) => xml.c14n(doc.r), + expected: `` }, { doc: `A`, call: (doc) => { let other = xml.parse(``);