Skip to content
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

Upgrade minidom to v 0.13 #21

Open
rimutaka opened this issue Jan 27, 2022 · 6 comments · May be fixed by #22
Open

Upgrade minidom to v 0.13 #21

rimutaka opened this issue Jan 27, 2022 · 6 comments · May be fixed by #22

Comments

@rimutaka
Copy link
Collaborator

The latest minidom v 0.13 doesn't work with our code. Changes are needed before we can switch to that version. Not sure how hard/trivial that may be.

running 7 tests
test tests::test_parse_text ... ok
test tests::test_malformed_xml ... ok
test tests::test_empty_elements_invalid ... FAILED
test tests::test_numbers ... FAILED
test tests::test_mixed_nodes ... FAILED
test tests::test_empty_elements_valid ... FAILED
test tests::convert_test_files ... FAILED
@AlecTroemel
Copy link
Owner

here's the changelog in minidom https://gitlab.com/xmpp-rs/xmpp-rs/-/blob/main/minidom/CHANGELOG.md

looks like they now Force namespaces on Element

@rimutaka
Copy link
Collaborator Author

rimutaka commented Apr 12, 2022

Minidom is at v.0.14 now and requires a NS declaration, which is NOT compliant with XML spec.

Reproduction:

    assert!(Element::from_str(r#"<a b="1"><x/></a>"#).is_err()); // <-- this is a totally well formed XML, but minidom fails it

    assert!(Element::from_str(r#"<a xmlns="ns1" b="1"><x/></a>"#).is_ok()); // <- adding a NS fixes it

    assert!(Element::from_str(r#"<a xmlns="" b="1"><x/></a>"#).is_ok());

All 3 versions of the XML in the example above are correct, but minidom fails to parse the first one with no xmlns declaration.

They have an issue for this under https://gitlab.com/xmpp-rs/xmpp-rs/-/issues/41. The maintainer said they are not interested in fixing it because it works for them (XMPP has NS for everything and then some)

It may be wiser to switch to a more generic XML parser like https://github.com/RazrFalcon/roxmltree.

It looks like a better tool with a similar API. I think we can make a backward-compatible release with it.

@AlecTroemel , thoughts?

@AlecTroemel
Copy link
Owner

@rimutaka I have no problem switching away from minidom/xmpp-rs. roxmltree looks promising. I dont think we're mutating any part of the xml "DOM", so the fact that its read only should be ok. I'll have time to look into it more later this week!

@ghost
Copy link

ghost commented Mar 23, 2023

Hello,

Recently, compiling my projects leads to the following warning message from the cargo utility:

% cargo build --release
    Finished release [optimized] target(s) in 0.31s
warning: the following packages contain code that will be rejected by a future version of Rust: quick-xml v0.17.2
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 2`

I did some digging, and the real culprit is quickxml_to_serde, because it has the following outdated dependency (see source):

minidom = "0.12"

which in turn has the following outdated dependency (see source):

quick-xml = "0.17"

Therefore, quickxml_to_serde is entirely responsible for the warning.

@AlecTroemel
Copy link
Owner

Hey all, sorry for the inactivity on this. Im finally spending some time getting this project updated.

@AlecTroemel AlecTroemel linked a pull request Mar 24, 2023 that will close this issue
@ghost
Copy link

ghost commented May 19, 2023

The warnings were bothering me enough that I switched now to xml2json-rs. It provides a build_from_xml function, which replaces quickxml_to_serde for my needs. I'll return back to quickxml_to_serde if I encounter problems.

Have a nice weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants