Openpgpjs: possible parseUserId bug when name is also an email

[tomholub]

openpgp.util.parseUserId('someemail.com <[email protected]>')
// Object { name: "someemail.com", email: "[email protected]", comment: "" }

openpgp.util.parseUserId('[email protected] <[email protected]>')
// Error: Invalid user id format

People do seem to use these ids email <email> out in the wild, it would be good to be able to support them.

[chesnokovilya]

Hmm, this is feature of address-rfc2822 library.

[tomholub]

Correct, we may want to raise the issue there. But I have a feeling we'll have to fix it & submit a patch anyway.

[twiss]

See https://gitlab.com/sequoia-pgp/sequoia/issues/298#note_189291296.

Since GPG, x/crypto/openpgp and even OpenPGP.js itself just concatenate the components together like in this example, we may want to just hand-roll a super simple parser which splits those components and nothing else, and get rid of our address-rfc2822 dependency (saving some space in the process).

OTOH, since the spec mentions RFC2822, and OpenPGP.js did generate User IDs like that from v3.0.10 to v4.4.10, as presumably does Sequoia PGP, maybe we should still retain the capability to parse RFC2822, but perhaps switch to a more tolerant parser, or fix the one we use (as proposed above).

Finally, another option is to leave the parser as is but add a simple fallback parser for cases where it throws.

PRs for any of these options are welcome.

[tomholub]

We have been happily using the following.

It used to be a regex, but it would choke on maliciously large user ids. So we ended up with:

  public static parseEmail = (full: string) => {
    let email: string | undefined;
    let name: string | undefined;
    if (full.includes('<') && full.includes('>')) {
      email = full.substr(full.indexOf('<') + 1, full.indexOf('>') - full.indexOf('<') - 1).replace(/["']/g, '').trim().toLowerCase();
      name = full.substr(0, full.indexOf('<')).replace(/["']/g, '').trim();
    } else {
      email = full.replace(/["']/g, '').trim().toLowerCase();
    }
    return { email, name, full };
  }

Releasing into public domain, in case it's useful. It is far from a full RFC compliant parser (obviously) but it actually works out in the field.

There's a email.toLowerCase() because I really have a thing against uppercase letters in emails :laughing: which could potentially be removed if reused in OpenPGP.js.

[chesnokovilya]

Do we need email address validation in parseUserId function?

[chesnokovilya]

I will put following comment by Twiss here for the reference:
Personally, I think the simple format of Name (Comment) <email>, each of them optional, and rejecting ()<>\0 inside each component, as GPG does, is not so bad. Sure, it's not a spec, but if we put it in rfc4880, then it is ;)

[twiss]

Do we need email address validation in parseUserId function?

No, I think if someone puts an invalid email in there it's not really our responsibility to throw on that. The spec even says it's a "convention" to put one there, but it can contain anything.

[chesnokovilya]

I have submitted fix to https://github.com/jackbearheart/email-addresses/pull/46

it is used by address-rfc2822 library. I think in the future we should switch to direct use of email-address since it is much lighter library.

[twiss]

Fixed by #930.