Skip to content

Commit 6cc57b2

Browse files
mxaminclaude
andcommitted
Match id attributes by local name under the shadow (#356)
The shadow's register_id validated the attribute with node.get(), which only finds an unqualified attribute, while the fast path's xmlHasProp() matches by local name whatever the namespace. A valid register_id(node, "Id") for a namespaced Id could therefore raise "missing attribute." in shadow mode only. Without id_ns the lookup now scans the element's attribute names and compares local names, as xmlHasProp does — and as the ID replay onto the copy already did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent a3b1fce commit 6cc57b2

3 files changed

Lines changed: 72 additions & 18 deletions

File tree

‎developer.md‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,7 @@ All invisible to the documented API:
231231
document until grafted;
232232
- `encrypted_data_ensure_key_info(ns=...)` on an existing `KeyInfo` returns
233233
a new element object rather than the original proxy;
234-
- `register_id` skips the live duplicate-id check (it runs per copy instead)
235-
and, without `id_ns`, looks the attribute up namespace-strictly where the
236-
raw `xmlHasProp` is namespace-agnostic;
234+
- `register_id` skips the live duplicate-id check (it runs per copy instead);
237235
- `encrypt_xml` *copies* a template that is attached inside the target tree
238236
rather than moving it, so it also remains at its original position;
239237
- signature/encryption contexts keep no live result nodes after the call

‎src/ds.c‎

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,50 @@ static int PyXmlSec_SignatureContextKeySet(PyObject* self, PyObject* value, void
119119
return 0;
120120
}
121121

122+
// Non-zero when `node` carries an attribute whose *local* name is `name`,
123+
// whatever its namespace, -1 with an exception set on failure. The fast
124+
// path's xmlHasProp() matches that way, while lxml's node.get(name) only
125+
// finds an unqualified attribute; the shadow's validation goes through this
126+
// so that both modes accept the same calls.
127+
static int PyXmlSec_LxmlHasAttrByLocalName(PyXmlSec_LxmlElementPtr node, const char* name) {
128+
PyObject* keys;
129+
PyObject* seq;
130+
Py_ssize_t i;
131+
Py_ssize_t n;
132+
int found = 0;
133+
134+
keys = PyObject_CallMethod((PyObject*)node, "keys", NULL);
135+
if (keys == NULL) {
136+
return -1;
137+
}
138+
seq = PySequence_Fast(keys, "unexpected attribute names.");
139+
Py_DECREF(keys);
140+
if (seq == NULL) {
141+
return -1;
142+
}
143+
144+
n = PySequence_Fast_GET_SIZE(seq);
145+
for (i = 0; i < n && !found; ++i) {
146+
PyObject* key = PySequence_Fast_GET_ITEM(seq, i); // borrowed
147+
const char* local;
148+
const char* end;
149+
if (!PyUnicode_Check(key) || (local = PyUnicode_AsUTF8(key)) == NULL) {
150+
if (!PyErr_Occurred()) {
151+
PyErr_SetString(PyXmlSec_InternalError, "unexpected attribute name.");
152+
}
153+
Py_DECREF(seq);
154+
return -1;
155+
}
156+
// lxml spells a namespaced attribute "{href}local".
157+
if (local[0] == '{' && (end = strchr(local, '}')) != NULL) {
158+
local = end + 1;
159+
}
160+
found = strcmp(local, name) == 0;
161+
}
162+
Py_DECREF(seq);
163+
return found;
164+
}
165+
122166
static const char PyXmlSec_SignatureContextRegisterId__doc__[] = \
123167
"register_id(node, id_attr = 'ID', id_ns = None) -> None\n"
124168
"Registers new id.\n\n"
@@ -151,27 +195,31 @@ static PyObject* PyXmlSec_SignatureContextRegisterId(PyObject* self, PyObject* a
151195
// whole-document shadows (sign/verify/decrypt) replay it onto their
152196
// private copies. The duplicate-id check runs there, per copy.
153197
if (PyXmlSec_LxmlShadowIsActive()) {
154-
PyObject* key;
155-
PyObject* value;
198+
int found;
156199
if (id_ns != NULL) {
157-
key = PyUnicode_FromFormat("{%s}%s", id_ns, id_attr);
200+
PyObject* key = PyUnicode_FromFormat("{%s}%s", id_ns, id_attr);
201+
PyObject* value;
202+
if (key == NULL) {
203+
goto ON_FAIL;
204+
}
205+
value = PyObject_CallMethod((PyObject*)node, "get", "O", key);
206+
Py_DECREF(key);
207+
if (value == NULL) {
208+
goto ON_FAIL;
209+
}
210+
found = value != Py_None;
211+
Py_DECREF(value);
158212
} else {
159-
key = PyUnicode_FromString(id_attr);
160-
}
161-
if (key == NULL) {
162-
goto ON_FAIL;
163-
}
164-
value = PyObject_CallMethod((PyObject*)node, "get", "O", key);
165-
Py_DECREF(key);
166-
if (value == NULL) {
167-
goto ON_FAIL;
213+
// As xmlHasProp() does on the fast path: by local name.
214+
found = PyXmlSec_LxmlHasAttrByLocalName(node, id_attr);
215+
if (found < 0) {
216+
goto ON_FAIL;
217+
}
168218
}
169-
if (value == Py_None) {
170-
Py_DECREF(value);
219+
if (!found) {
171220
PyErr_SetString(PyXmlSec_Error, "missing attribute.");
172221
goto ON_FAIL;
173222
}
174-
Py_DECREF(value);
175223
if (PyXmlSec_LxmlShadowRecordId(node, id_attr, id_ns) < 0) {
176224
goto ON_FAIL;
177225
}

‎tests/test_ds.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ def test_register_id_bad_args(self):
5555
with self.assertRaises(TypeError):
5656
ctx.register_id('')
5757

58+
def test_register_id_matches_namespaced_attribute_by_local_name(self):
59+
"""Should accept a namespaced id attribute when no id_ns is given, as xmlHasProp does."""
60+
ctx = xmlsec.SignatureContext()
61+
root = self.load_xml('sign_template.xml')
62+
sign = xmlsec.template.create(root, consts.TransformExclC14N, consts.TransformRsaSha1)
63+
sign.set('{http://www.example.org/ns}Id', 'sig-1')
64+
ctx.register_id(sign, 'Id')
65+
5866
def test_register_id_with_namespace_without_attribute(self):
5967
ctx = xmlsec.SignatureContext()
6068
root = self.load_xml('sign_template.xml')

0 commit comments

Comments
 (0)