[WIP] allow to use `libxml2-wasm` for XML validation by SierraNL · Pull Request #1184 · CycloneDX/cyclonedx-javascript-library

SierraNL · 2024-11-26T10:45:04Z

Due to libxmljs2 not being maintained and contains a vulnerability, a replacement needed to be found. This commit replaces it with libxml2-wasm, which is a new, but maintained library, which serves the purpose of validating XML.

The implementation is as close the the previous library in regards to flags passed to libxml2, but only adapted to a different interface and the recommendation to dispose all objects.

This is my first contribution to this project, and typescript isn't my usual language, so comments are welcome.

related to: #1079

Due to libxmljs2 not being maintained and contains a vulnerability, a replacement needed to be found. This commit replaces it with libxml2-wasm, which is a new, but maintained library, which serves the purpose of validating XML. The implementation is as close the the previous library in regards to flags passed to libxml2, but only adapted to a different interface and the recommendation to dispose all objects. This is my first contribution to this project, and typescript isn't my usual language, so comments are welcome. Resolves: CycloneDX#1079 Signed-off-by: Leon Grave <l.grave@gmail.com>

jkowalleck · 2024-11-26T11:20:23Z

thanks for donating this feature, @SierraNL .

let me clarify some things:

Due to libxmljs2 [...] contains a vulnerability [...]

this is not the case. The current libxml2 library contains a feature, that, if used wrong downstream, could lead to a vulnerability downstream. The downstream usage in the CycloneDX-JS-lib does not use it wrong, so no vulnerability exists.

[...] a replacement needed to be found

This is true in the long term, but we do not intend to replace libxmljs2 right away. Instead, we want to allow alternatives.
Therefore, some of your changes need to be reverted.

src/_optPlug.node/xmlValidator.ts

src/_optPlug.node/__xmlValidators/libxmljs2.ts

package.json

jkowalleck · 2024-11-26T11:24:45Z

src/_optPlug.node/__xmlValidators/libxml2-wasm.ts

+  const schema = XmlDocument.fromString(
+    await readFile(schemaPath, 'utf-8'),
+    {
+      option: ParseOption.XML_PARSE_NONET | ParseOption.XML_PARSE_COMPACT,


is this XEE safe? see https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing

The interface for this wrapper is somewhat different, building the parse options is combining the flags you want on. In the other implementation it's an object where they could be turned on and off explicitely. So this should result in the same options.

I also added this implementation to the xmlValidator tests, and that includes an XXE test.

lets see how the tests turn out.

Signed-off-by: Leon Grave <l.grave@gmail.com>

docs/dev/decisions/XmlValidator.md

src/_optPlug.node/__xmlValidators/libxmljs2.ts

Signed-off-by: Leon Grave <l.grave@gmail.com>

docs/dev/decisions/XmlValidator.md

src/_optPlug.node/__xmlValidators/libxml2-wasm.ts

jkowalleck · 2024-11-26T12:17:18Z

src/_optPlug.node/__xmlValidators/libxml2-wasm.ts

+    }
+
+    doc.dispose();
+    validator.dispose();


really free/dispose the validator and schema here?

Good point, this will go wrong the second call. I could just not dispose the validator and the schema. But the library emphasises proper disposing (https://jameslan.github.io/libxml2-wasm/v0.4/documents/Memory_Management.html). Here I'm really lacking in Typescript knowledge on how to solve this, could I use a using here?

regarding using, read here: https://www.totaltypescript.com/typescript-5-2-new-keyword-using

regarding manually disposing/freeing: maybe just try it out. in the end, it all is javascript - just see what you can do.

@SierraNL , could you see if changing this makes the tests pass?

Signed-off-by: Leon Grave <l.grave@gmail.com>

jkowalleck

please modify the README to reflect this new optional dependency:
see

cyclonedx-javascript-library/README.md

Lines 129 to 131 in fca1db5

    
           * Validation of XML on _Node.js_ requires all of: 
        
             * [`libxmljs2`](https://www.npmjs.com/package/libxmljs2)   
        
             * the system might need to meet the requirements for [`node-gyp`](https://github.com/TooTallNate/node-gyp#installation), in certain cases.

- * Validation of XML on _Node.js_ requires all of:
+ * Validation of XML on _Node.js_ requires any of: 
    * [`libxmljs2`](https://www.npmjs.com/package/libxmljs2)
+   * [`libxml2-wasm@`](https://www.npmjs.com/package/libxml2-wasm@)
    * the system might need to meet the requirements for [`node-gyp`](https://github.com/TooTallNate/node-gyp#installation), in certain cases.

jkowalleck · 2024-12-21T08:37:56Z

HISTORY.md

  * Apply latest code style guide (via [#1170], [#1181])
 * Dependencies
  * Support `libxmljs2@^0.35` (via [#1173])
+  * Support `libxml2-wasm@^0.41` as an alternative for `libxmljs2` (via [#1184])


Since a release of 7.0.0in the meantime, and a forward-merge,
this needs to be updated to be in the unreleased category.

Moved the line (and a header) to unreleased.

jkowalleck · 2025-01-09T08:30:22Z

@SierraNL, do you have any updates on this?

Signed-off-by: Leon Grave <l.grave@gmail.com>

SierraNL · 2025-01-10T16:14:40Z

@SierraNL, do you have any updates on this?

Was a bit busy over the holidays, processed your comments. Can't run the tests locally, so need to see if the Github build succeeds.

jkowalleck · 2025-01-11T22:27:48Z

could you rebase/merge this branch wit the latest master?

jkowalleck · 2025-01-11T22:30:47Z

@SierraNL, do you have any updates on this?

Was a bit busy over the holidays, processed your comments. Can't run the tests locally, so need to see if the Github build succeeds.

you may want to dispatch the QA workflow on your fork, when needed.
https://github.com/SierraNL/cyclonedx-javascript-library/actions/workflows/nodejs.yml should have a button "run workflow" at the top right, which then lets you select on which branch you want to run the tests.

Signed-off-by: Leon Grave <l.grave@gmail.com>

SierraNL · 2025-01-12T10:12:14Z

@SierraNL, do you have any updates on this?

Was a bit busy over the holidays, processed your comments. Can't run the tests locally, so need to see if the Github build succeeds.

you may want to dispatch the QA workflow on your fork, when needed. https://github.com/SierraNL/cyclonedx-javascript-library/actions/workflows/nodejs.yml should have a button "run workflow" at the top right, which then lets you select on which branch you want to run the tests.

Thanks, I also managed to get the tests running locally, and the underlying reason of the test failing is this message:
Uncaught Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. To see where the top-level await comes from, use --experimental-print-required-tla.

I'll have a look on how this fix this, but I have the feeling the libxml2-wasm library is not compatible with the module structure of the cyclonedx library, or the way Typescript is transpiled, since imports are transpiled to require.

jkowalleck · 2025-01-12T12:03:27Z

src/_optPlug.node/__xmlValidators/libxml2-wasm.ts

+*/
+
+import { readFile } from 'fs/promises';
+import { ParseOption, XmlDocument, XsdValidator } from 'libxml2-wasm';


did not look into details of libxml2-wasm yet.

untested code for #1184 (comment)

Suggested change

import { ParseOption, XmlDocument, XsdValidator } from 'libxml2-wasm';

const { ParseOption, XmlDocument, XsdValidator } = await import('libxml2-wasm');

or, we could see if we can get libxml2-wasm CJS-compatible, which i'd prefer.
will drop them a PR as soon as possible.

or you could ask the library authors for help.

did not look into details of libxml2-wasm yet.

untested code for #1184 (comment)

This isn't possible with module set to CommonJS in the tsconfig:

would it be possible to move the ... = await import('libxml2-wasm');
down into exported async function? this should be a fitting solution.

export default (async function (schemaPath: string): Promise<Validator> { const { ParseOption, XmlDocument, XsdValidator } = await import('libxml2-wasm'); // ... }) satisfies Functionality

I also tried that, but it always gets transpiled into a require, which triggers the error.

I see.

So we might require dropping node14 support,
change some TS compiler options, modify some code here and there,
to make this work, right?

This will cause breaking-changes - which is no blocker, just a remark.

I would be happy working with you to make this happen. 👍

You have carte blanche - change whatever is needed to make this feature work. Just don't rush, good things may take a while.
I will take care of the change management and processes, so that this feature can be integrated.

I can give it a shot, I've done some tests locally and it seems the most work is in the Mocha tests, since they're written in Javascript with requires. They all need to be rewritten to imports.

jkowalleck · 2025-06-03T08:58:19Z

PR is stale. will set it to "DRAFT".

jkowalleck · 2025-06-05T17:00:39Z

lets wait for v5 of this very library -
noppa/xmllint-wasm#24

SierraNL requested a review from a team as a code owner November 26, 2024 10:45

jkowalleck requested changes Nov 26, 2024

View reviewed changes

Processed review comments

a4c45c1

Signed-off-by: Leon Grave <l.grave@gmail.com>

jkowalleck reviewed Nov 26, 2024

View reviewed changes

docs/dev/decisions/XmlValidator.md Outdated Show resolved Hide resolved

jkowalleck reviewed Nov 26, 2024

View reviewed changes

src/_optPlug.node/__xmlValidators/libxmljs2.ts Show resolved Hide resolved

SierraNL added 2 commits November 26, 2024 13:11

Revert file deletion, removed decision

818b1e5

Signed-off-by: Leon Grave <l.grave@gmail.com>

Revert removal of line breaks in header

caa8cd2

Signed-off-by: Leon Grave <l.grave@gmail.com>

jkowalleck requested changes Nov 26, 2024

View reviewed changes

SierraNL added 2 commits November 26, 2024 21:04

Review comments

d329805

Signed-off-by: Leon Grave <l.grave@gmail.com>

Merge branch 'main' into replace-libxmljs2-with-libxml2-wasm

dc6530f

jkowalleck changed the title ~~Switch to libxml2-wasm for XML validation~~ allow to use libxml2-wasm for XML validation Dec 2, 2024

jkowalleck requested changes Dec 21, 2024

View reviewed changes

SierraNL added 2 commits January 10, 2025 16:52

No disposing of schema and validator

0e59ff4

Signed-off-by: Leon Grave <l.grave@gmail.com>

Fix documentation after 7.0.0 release

c5f922f

Signed-off-by: Leon Grave <l.grave@gmail.com>

SierraNL requested a review from jkowalleck January 11, 2025 10:06

SierraNL added 2 commits January 12, 2025 09:21

Merge branch 'CycloneDX:main' into replace-libxmljs2-with-libxml2-wasm

452d572

Bump libxml2-wasm to 0.5.0

ef3bd62

Signed-off-by: Leon Grave <l.grave@gmail.com>

jkowalleck reviewed Jan 12, 2025

View reviewed changes

jkowalleck self-requested a review January 13, 2025 09:39

jkowalleck marked this pull request as draft June 3, 2025 08:58

jkowalleck changed the title ~~allow to use libxml2-wasm for XML validation~~ [WIP] allow to use libxml2-wasm for XML validation Jun 3, 2025

	* Validation of XML on _Node.js_ requires all of:
	* [`libxmljs2`](https://www.npmjs.com/package/libxmljs2)
	* the system might need to meet the requirements for [`node-gyp`](https://github.com/TooTallNate/node-gyp#installation), in certain cases.

	import { ParseOption, XmlDocument, XsdValidator } from 'libxml2-wasm';
	const { ParseOption, XmlDocument, XsdValidator } = await import('libxml2-wasm');

Uh oh!

Conversation

SierraNL commented Nov 26, 2024 • edited by jkowalleck Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkowalleck commented Nov 26, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SierraNL Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkowalleck left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkowalleck commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SierraNL commented Jan 10, 2025

Uh oh!

jkowalleck commented Jan 11, 2025

Uh oh!

jkowalleck commented Jan 11, 2025

Uh oh!

SierraNL commented Jan 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkowalleck Jan 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkowalleck Jan 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SierraNL Jan 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkowalleck Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkowalleck Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkowalleck commented Jun 3, 2025

Uh oh!

jkowalleck commented Jun 5, 2025

Uh oh!

SierraNL commented Nov 26, 2024 •

edited by jkowalleck

Loading

SierraNL Nov 26, 2024 •

edited

Loading

jkowalleck left a comment •

edited

Loading

jkowalleck commented Jan 9, 2025 •

edited

Loading

SierraNL commented Jan 12, 2025 •

edited

Loading

jkowalleck Jan 12, 2025 •

edited

Loading

jkowalleck Jan 12, 2025 •

edited

Loading

SierraNL Jan 12, 2025 •

edited

Loading

jkowalleck Jan 13, 2025 •

edited

Loading

jkowalleck Jan 13, 2025 •

edited

Loading