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

Switch dependency from xmldom to @xmldom/xmldom #262

Closed
rossgp opened this issue Jan 30, 2023 · 13 comments · Fixed by #265
Closed

Switch dependency from xmldom to @xmldom/xmldom #262

rossgp opened this issue Jan 30, 2023 · 13 comments · Fixed by #265

Comments

@rossgp
Copy link

rossgp commented Jan 30, 2023

The recently published version 3.6.0 introduced a dependency on xmldom (v0.6.0). This package however was migrated from v0.7.0 to be published under @xmldom/xmldom
the reasons this had to be done explained here: xmldom/xmldom#271

xmldom has several security vulnerabilities that are only fixed in version published under @xmldom/xmldom so ideally the dependency would be switched to @xmldom/xmldom

@julianhille
Copy link
Owner

Thanks. Will do together with electron 23 rls. Would this work for you?

@rossgp
Copy link
Author

rossgp commented Jan 30, 2023

Makes sense. Thanks!

@leantorres73
Copy link

leantorres73 commented Jan 30, 2023

This is exactly what I was looking for! 👏 .. we have npm security checks to deploy our services and I was moving away from phantomJS and trying to implement this library and it is a blocker for me. Awesome you have this in your roadmap,
@julianhille any estimated release date?

Thanks!

@julianhille
Copy link
Owner

If you send a PR I can release a 3.6.1 by tomorrow

@julianhille
Copy link
Owner

I'm just confused that GitHub security does not inform me here. Or did o overlook it?

@leantorres73
Copy link

image

Not sure why Github is not informing the issue, I agree..

julianhille added a commit that referenced this issue Jan 30, 2023
@julianhille
Copy link
Owner

Not as easy as i thought, because the added muhammara / hummus recipe depends on an old xmldom.
The old version allows multiple nodes on root whereas the newer version only allows a single root node.
That's why i waste a bit of cpu cycles and map it into root node and extract it back from there.

@julianhille
Copy link
Owner

I just had another look at the security warnings and i must have overlooked it, i got a warning a week ago which i was totally unaware of. Once again, thanks for reporting.

@rossgp
Copy link
Author

rossgp commented Jan 31, 2023

Not as easy as i thought, because the added muhammara / hummus recipe depends on an old xmldom. The old version allows multiple nodes on root whereas the newer version only allows a single root node. That's why i waste a bit of cpu cycles and map it into root node and extract it back from there.

Thanks so much for look into the update. I would have struggled to find that problem myself as I've never used the muhammara / hummus recipe component before

@julianhille
Copy link
Owner

the recipe component is new, as the libs did not update / evolve anymore. So i decided to integrate it.

@julianhille
Copy link
Owner

Still has a pending run, but overall all action steps worked.
There is another issue #263 which id like to fix with a 3.6.1 as this currently prevents m1 users from using muhammara.

So, in the end this means it will take another day, because all the actions take a serious amount of time to finish.

@julianhille
Copy link
Owner

3.7.0 will be released today. sorry for the delay but every issue introduced a new one while fixing the other.

@julianhille
Copy link
Owner

thank you for your patience. there was another issue which needed to be resolved, which it is now, somehow.
Currently the builds are running and it should be released in about 4 hours.

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.

3 participants