search

haraka / node-address-rfc2822

Parser for RFC2822 (Header) format email addresses
https://www.npmjs.com/package/address-rfc2822
53 stars 16 forks source link

Allow comma in display name #52

Closed osm closed 3 years ago

osm commented 3 years ago

This is a similar PR as I made earlier https://github.com/haraka/node-address-rfc2822/pull/47, but this time it allows , in the display name.

The feature was added to the email-addresses library with this PR: https://github.com/jackbearheart/email-addresses/pull/54

msimerson commented 3 years ago

I'm not strongly opposed to allowing this, but after perusing RFC 5322 quickly, and considering the security aspects of the Display Name, I'm not convinced it should default to enabled. I think the feature should default to off.

osm commented 3 years ago

That sounds reasonable.

I decided to add an options object after the startAt parameter that lets the user toggle comma in display name parsing. It would have been nicer to also include the startAt parameter within the options object but that would mean a breaking change. So I think this is probably the best way to do it.

msimerson commented 3 years ago

Switching to an object for passing in arguments is the right approach. The called function can check the argument type to maintain backwards compatibility.

osm commented 3 years ago

Yeah it's nicer, with the latest commit I've changed it to allow for an object to be passed instead and at the same time also accept anything else and use that as the startAt parameter to keep backwards compatibility.

msimerson commented 3 years ago

And while you're at it, please update Changes.md and bump the version number in package.json.