Skip to content

libxml2 version check #299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions src/lxml.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "common.h"
#include "lxml.h"
#include "exception.h"

#include <etree_defs.h>
#include <etree_api.h>
Expand All @@ -17,8 +18,85 @@
#include <libxml/parser.h>
#include <libxml/dict.h>

#define XMLSEC_EXTRACT_VERSION(x, y) ((x / (y)) % 100)

#define XMLSEC_EXTRACT_MAJOR(x) XMLSEC_EXTRACT_VERSION(x, 100 * 100)
#define XMLSEC_EXTRACT_MINOR(x) XMLSEC_EXTRACT_VERSION(x, 100)
#define XMLSEC_EXTRACT_PATCH(x) XMLSEC_EXTRACT_VERSION(x, 1)

static long PyXmlSec_GetLibXmlVersionLong() {
return PyOS_strtol(xmlParserVersion, NULL, 10);
}
long PyXmlSec_GetLibXmlVersionMajor() {
return XMLSEC_EXTRACT_MAJOR(PyXmlSec_GetLibXmlVersionLong());
}
long PyXmlSec_GetLibXmlVersionMinor() {
return XMLSEC_EXTRACT_MINOR(PyXmlSec_GetLibXmlVersionLong());
}
long PyXmlSec_GetLibXmlVersionPatch() {
return XMLSEC_EXTRACT_PATCH(PyXmlSec_GetLibXmlVersionLong());
}

long PyXmlSec_GetLibXmlCompiledVersionMajor() {
return XMLSEC_EXTRACT_MAJOR(LIBXML_VERSION);
}
long PyXmlSec_GetLibXmlCompiledVersionMinor() {
return XMLSEC_EXTRACT_MINOR(LIBXML_VERSION);
}
long PyXmlSec_GetLibXmlCompiledVersionPatch() {
return XMLSEC_EXTRACT_PATCH(LIBXML_VERSION);
}

static int PyXmlSec_CheckLxmlLibraryVersion(void) {
// Make sure that the version of libxml2 lxml is using is the same as the one we are using. Because
// we pass trees between the two libraries, we need to make sure that they are using the same version
// of libxml2, or we could run into difficult to debug segfaults.
// See: https://github.com/xmlsec/python-xmlsec/issues/283

PyObject* lxml = NULL;
PyObject* version = NULL;

// Default to failure
int result = -1;

lxml = PyImport_ImportModule("lxml.etree");
if (lxml == NULL) {
goto FINALIZE;
}
version = PyObject_GetAttrString(lxml, "LIBXML_VERSION");
if (version == NULL) {
goto FINALIZE;
}
if (!PyTuple_Check(version) || PyTuple_Size(version) != 3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably enough to check "size < 2" here, if all you look at is the first two entries.

goto FINALIZE;
}

PyObject* major = PyTuple_GetItem(version, 0);
PyObject* minor = PyTuple_GetItem(version, 1);

if (!PyLong_Check(major) || !PyLong_Check(minor)) {
goto FINALIZE;
}

if (PyLong_AsLong(major) != PyXmlSec_GetLibXmlVersionMajor() || PyLong_AsLong(minor) != PyXmlSec_GetLibXmlVersionMinor()) {
goto FINALIZE;
}
Comment on lines +74 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this should usually work, users can easily assign tuples with large integers to etree.LIBXML_VERSION, which would then make PyLong_AsLong() fail here and the code continues with an exception set.

I suggest adding an exception check also to the PyTuple_GetItem() calls above, because why not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that PyLong_Check handled this case, but re-reading the docs now I see that it doesn't. Thanks for the feedback here.


result = 0;

FINALIZE:
// Cleanup our references, and return the result
Py_XDECREF(lxml);
Py_XDECREF(version);
return result;
}
Comment on lines +87 to +92
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should call PyErr_Clear(), in case we got here due to an exception.


int PyXmlSec_InitLxmlModule(void) {
if (PyXmlSec_CheckLxmlLibraryVersion() < 0) {
PyXmlSec_SetLastError("lxml & xmlsec libxml2 library version mismatch");
return -1;
}

return import_lxml__etree();
}

Expand Down
9 changes: 9 additions & 0 deletions src/lxml.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,13 @@ PyXmlSec_LxmlElementPtr PyXmlSec_elementFactory(PyXmlSec_LxmlDocumentPtr doc, xm
// converts o to PyObject, None object is not allowed, does not increment ref_counts
int PyXmlSec_LxmlElementConverter(PyObject* o, PyXmlSec_LxmlElementPtr* p);

// get version numbers for libxml2 both compiled and loaded
long PyXmlSec_GetLibXmlVersionMajor();
long PyXmlSec_GetLibXmlVersionMinor();
long PyXmlSec_GetLibXmlVersionPatch();

long PyXmlSec_GetLibXmlCompiledVersionMajor();
long PyXmlSec_GetLibXmlCompiledVersionMinor();
long PyXmlSec_GetLibXmlCompiledVersionPatch();

#endif // __PYXMLSEC_LXML_H__
30 changes: 27 additions & 3 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "common.h"
#include "platform.h"
#include "exception.h"
#include "lxml.h"

#include <xmlsec/xmlsec.h>
#include <xmlsec/crypto.h>
Expand Down Expand Up @@ -127,10 +128,27 @@ static PyObject* PyXmlSec_GetLibXmlSecVersion() {
}

static char PyXmlSec_GetLibXmlVersion__doc__[] = \
"get_libxml_version() -> tuple\n"
"Returns Version tuple of wrapped libxml library.";
"get_libxml_version() -> tuple[int, int, int]\n"
"Returns version tuple of libxml2 library xmlsec is using.";
static PyObject* PyXmlSec_GetLibXmlVersion() {
return Py_BuildValue("(iii)", XMLSEC_LIBXML_VERSION_MAJOR, XMLSEC_LIBXML_VERSION_MINOR, XMLSEC_LIBXML_VERSION_PATCH);
return Py_BuildValue(
"(iii)",
PyXmlSec_GetLibXmlVersionMajor(),
PyXmlSec_GetLibXmlVersionMinor(),
PyXmlSec_GetLibXmlVersionPatch()
);
}

static char PyXmlSec_GetLibXmlCompiledVersion__doc__[] = \
"get_libxml_compiled_version() -> tuple[int, int, int]\n"
"Returns version tuple of libxml2 library xmlsec was compiled with.";
static PyObject* PyXmlSec_GetLibXmlCompiledVersion() {
return Py_BuildValue(
"(iii)",
PyXmlSec_GetLibXmlCompiledVersionMajor(),
PyXmlSec_GetLibXmlCompiledVersionMinor(),
PyXmlSec_GetLibXmlCompiledVersionPatch()
);
}

static char PyXmlSec_PyEnableDebugOutput__doc__[] = \
Expand Down Expand Up @@ -412,6 +430,12 @@ static PyMethodDef PyXmlSec_MainMethods[] = {
METH_NOARGS,
PyXmlSec_GetLibXmlVersion__doc__
},
{
"get_libxml_compiled_version",
(PyCFunction)PyXmlSec_GetLibXmlCompiledVersion,
METH_NOARGS,
PyXmlSec_GetLibXmlCompiledVersion__doc__
},
{
"enable_debug_trace",
(PyCFunction)PyXmlSec_PyEnableDebugOutput,
Expand Down
7 changes: 0 additions & 7 deletions src/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,13 @@

#define PY_SSIZE_T_CLEAN 1

#include <libxml/xmlversion.h>
#include <xmlsec/version.h>
#include <Python.h>

#ifdef MS_WIN32
#include <windows.h>
#endif /* MS_WIN32 */

#define XMLSEC_EXTRACT_VERSION(x, y) ((x / (y)) % 100)

#define XMLSEC_LIBXML_VERSION_MAJOR XMLSEC_EXTRACT_VERSION(LIBXML_VERSION, 100 * 100)
#define XMLSEC_LIBXML_VERSION_MINOR XMLSEC_EXTRACT_VERSION(LIBXML_VERSION, 100)
#define XMLSEC_LIBXML_VERSION_PATCH XMLSEC_EXTRACT_VERSION(LIBXML_VERSION, 1)

#define XMLSEC_VERSION_HEX ((XMLSEC_VERSION_MAJOR << 16) | (XMLSEC_VERSION_MINOR << 8) | (XMLSEC_VERSION_SUBMINOR))

// XKMS support was removed in version 1.2.21
Expand Down
2 changes: 2 additions & 0 deletions src/xmlsec/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ from xmlsec.constants import __Transform as Transform
_E = TypeVar('_E', bound=_Element)

def enable_debug_trace(enabled: bool = ...) -> None: ...
def get_libxml_version() -> tuple[int, int, int]: ...
def get_libxml_compiled_version() -> tuple[int, int, int]: ...
def init() -> None: ...
def shutdown() -> None: ...
def cleanup_callbacks() -> None: ...
Expand Down