oozcitak / xmlbuilder-js

An XML builder for node.js
MIT License
918 stars 107 forks source link

assertLegalName With Spaces #234

Closed scriptsure closed 4 years ago

scriptsure commented 4 years ago

I have been using xmlbuilder by way of xml2js for many years to serialize XML for prescriptions that we send through our application. The standard that the XML is based off of was made by committee of course, and when you look at it they definitely did things that a developer would never do. An example is creating repeating objects outside of an array. So for example, the insurance information on a prescription could be sent like this:

<BenefitCoordination>
    ...INSURANCE INFO
</BenefitCoordination>
<BenefitCoordination>
    ...INSURANCE INFO
</BenefitCoordination>

So just objects placed, repeating, with no parent array.

We got around this by creating the JSON with repeating field names with trailing spaces increasing with each iteration. For example,

'BenefitCoordination ': {
    ...INSURANCE INFO
}
'BenefitCoordination  ': {
    ...INSURANCE INFO
 }

So as you can see the first object name has one space at the end the second has 2.

For the XMLStringify processing it worked great. Additionally, it does not effect any processors that receive the XML. BUT with some recent updates to the assertLegalName function the RegEx is not allowing spaces at the beginning or end of the field name. Thus in later releases we get:

Invalid character in name

I created a code pen to solve the issue. Basically all it needs is to add 0+ spaces to the beginning or end of the field name:

https://codepen.io/scriptsure/pen/Jjdrypm

var regex = /^ *([:A-Z_a-z\xC0-\xD6\xD8-\xF6\xF8-\u02FF\u0370-\u037D\u037F-\u1FFF\u200C\u200D\u2070-\u218F\u2C00-\u2FEF\u3001-\uD7FF\uF900-\uFDCF\uFDF0-\uFFFD]|[\uD800-\uDB7F][\uDC00-\uDFFF])([\x2D\.0-:A-Z_a-z\xB7\xC0-\xD6\xD8-\xF6\xF8-\u037D\u037F-\u1FFF\u200C\u200D\u203F\u2040\u2070-\u218F\u2C00-\u2FEF\u3001-\uD7FF\uF900-\uFDCF\uFDF0-\uFFFD]|[\uD800-\uDB7F][\uDC00-\uDFFF])* *$/

The changes above for the regex string that is being used in assertLegalName, consist of a simple space and * at the beginning and end of the regex.

OR

Possibly before the field is evaluated simply TRIM the string.

Any chance someone could push that in?

oozcitak commented 4 years ago

Can I recommend the new release: xmlbuilder2? It should do what you are after with the fragment function. For example:

const { fragment } = require('xmlbuilder2');

const obj = [
  { BenefitCoordination: '1'},
  { BenefitCoordination: '2'},
  { BenefitCoordination: '3'}
];
const frag = fragment(obj);
const xml = frag.toString({ prettyPrint: true });
console.log(xml);
<BenefitCoordination>1</BenefitCoordination>
<BenefitCoordination>2</BenefitCoordination>
<BenefitCoordination>3</BenefitCoordination>
scriptsure commented 4 years ago

well makes sense but i am utilizing the xml2js npm and dont think they are using xmlbuilder2.... Plus there are some arrays in XML and some are these object style repeats... so both can exist in the same large json object.

oozcitak commented 4 years ago

Can you send me the JS object? I am sure xmlbuilder2 can handle it.

oozcitak commented 4 years ago

Another alternative is to pass your own stringifier with:

var root = xmlbuilder.create('root', {
  stringify: {
    name: function(val) { // check val with your version of assertLegalName here }
  }
});

This is assuming xml2js passes builder options to xmlbuilder.

scriptsure commented 4 years ago

@oozcitak ok i will check with the team on using that function override. Just curious though... why are you opposed putting in that small modification?

oozcitak commented 4 years ago

why are you opposed putting in that small modification?

Well that defeats the purpose of name validation. It will work for you but it may break for someone else.

My proposals to solve your issue:

I'd recommend xmlbuilder2 because you will still get what you want without relaxing spec compliance.

Edit: xmlbuilder2 can convert between JS objects and strings out of the box so you don't need xml2js.

scriptsure commented 4 years ago

@oozcitak can you point me to where in the spec that says space at beginning or end is bad? I know it is inside of a field. But can’t find it when they are trailing. Don’t believe this is against policy.

oozcitak commented 4 years ago

XML name is defined here: https://www.w3.org/TR/2008/REC-xml-20081126/#NT-Name

scriptsure commented 4 years ago

Could you not just allow an option to allow for trailing spaces to give dev the option?

oozcitak commented 4 years ago

Could you not just allow an option to allow for trailing spaces to give dev the option?

No, sorry. If I did that, xmlbuilder-js would no longer be compliant with the XML spec. Please consider the other alternative I proposed above.