tngan / samlify

Node.js library for SAML SSO
https://samlify.js.org
MIT License
610 stars 217 forks source link

Suggest fast-xml-parser as default validator #521

Closed mattkrick closed 1 year ago

mattkrick commented 1 year ago

Love this package, thank you for your work on it!

I noticed that there is only 1 node-native validator (xml-lint). I also noticed that xml-lint has a memory leak in it, consuming almost 17MB every time some XML gets validated! I confirmed this in my own production app, but the issue is also noted by another dev here (cc @polomsky): https://github.com/authenio/samlify-node-xmllint/issues/6

As an alternative, I wrote a custom wrapper around fast-xml-parser, which does not have any extra dependencies like python, java, node binaries, etc. It's just JS! I tested it out & confirmed that it does not have a memory leak.

I'm no expert in XML validation, but if a small, fast JS-native validator does the same job as all the other validators, would it make sense to simplify the API & just use fast-xml-parser?

import {XMLValidator} from 'fast-xml-parser'

const fastValidator = {
  async validate(xml: string) {
    const isValid = XMLValidator.validate(xml, {
      allowBooleanAttributes: true
    })
    if (isValid === true) return 'SUCCESS_VALIDATE_XML'
    throw 'ERR_INVALID_XML'
  }
}
polomsky commented 1 year ago

@mattkrick From the first sight fast-xml-parser does not support XSD which all other validators support.

mattkrick commented 1 year ago

ah! that's true, looks like the author also wrote https://github.com/NaturalIntelligence/detailed-xml-validator for that use case, but i haven't tested that

tngan commented 1 year ago

@mattkrick Yes, I have received several reports telling the validator you mentioned has a memory leak issue.

https://github.com/authenio/samlify-xmllint-wasm

This is another alternative if you wanna get rid of the Java validator. Also, when you choose a validator, please be aware of XXE attack.

tngan commented 1 year ago

@mattkrick Feel free to continue the discussion in this closed thread if you have further question.