Added checks for undefined

[AnthonyHarwood]

Found a simpler solution.

@NuSkooler - thanks for helping me get there :)

[NuSkooler]

I think this may actually break the parser, I need to take a closer look -- but IIRC the regex allows e.g. m[1] (which is checked later) but not [2], etc.

[AnthonyHarwood]

Arrgh. . You're right.
It breaks foo@host.com and Bar <baz@foobar.net>
I'll keep working on it.
Thanks.

[AnthonyHarwood]

When I tried sending mail to foo@host.com, the code change that I made was never hit in the debugger. So that leads me to think this issue is unrelated.

What I did notice in the debugger was sending to foo@host.com resulted in a local database lookup. Is that normal? I'm uncertain if sending to foo@host.com is supposed to send to a local user, or if the BBS is supposed to function as an email gateway.

Fwiw, I reverted my code change, and results were the same. Sending to foo@host.com still resulted in "Invalid username" on my system.

After I created a foo@host.com user, I stopped getting the "Invalid username" error. However I don't know if that is how things are supposed to work. I have a feeling the BBS is supposed to function as an email gateway. Please correct me if I am wrong.

Finally, I logged in to xibalba to try and determine how things are behaving there. Unfortunately I ran out of time this evening.

I'll pick this up again tomorrow. Thanks for your help.

[NuSkooler]

What I did notice in the debugger was sending to foo@host.com resulted in a local database lookup. Is that normal? I'm uncertain if sending to foo@host.com is supposed to send to a local user, or if the BBS is supposed to function as an email gateway.

The email parsing right now is mostly just a placeholder - enig can currently automatically send password resets, auth validation, etc. emails, but is not quite yet set up to map users to username@yourbbshost.com, which is the ultimate intent.

If the parser (from the previous PR) doesn't think it's a email, it would default to a local lookup.

[AnthonyHarwood]

Thanks for the info.

It seems the current implementation in master branch behaves slightly differently than you describe. We're actually getting a db lookup whenever the address is determined to be anything other than FTN flavor.

Although this PR doesn't get us any closer to the ultimate intent of username@yourbbshost.com it does resolve the socket disconnect that happens when a number is entered into the To field.

[NuSkooler]

Sorry you're right, this change is in the ActivityPub branch:

function validateGeneralMailAddressedTo(data, cb) {
    //
    //  Allow any supported addressing:
    //  - Local username or real name
    //  - Supported remote flavors such as FTN, email, ...
    //
    const addressedToInfo = getAddressedToInfo(data);
    if (Message.AddressFlavor.Local !== addressedToInfo.flavor) {
        return cb(null);
    }

    return validateUserNameOrRealNameExists(data, cb);
}

There are actually quite a few fixes in there. I'd like to get a lot of that to main, just need some time on it. Free free to change it to the above.

[AnthonyHarwood]

I made the change you requested. Thanks for reviewing.

[NuSkooler]

I think we need to revert this part, right?

[AnthonyHarwood]

The test for m[2] might not be needed.

To be honest, I never found a way to cause m[2] to be null. I just figured if testing for m[3] was necessary, testing for m[2] was probably not a bad idea. Perhaps I was being a bit too defensive.

Anyways... I tried removing the test for m[3] just now, and found that entering a numeric value in the address field will cause an error and disconnect me. So I believe that test is still needed.

I'm curious to know if the tests for m[2] and m[3] might become unnecessary if FTN_ADDRESS_REGEXP was tweaked a bit. And then we could just test m??? Just a thought.

[NuSkooler]

Did this fix your issue still?

[AnthonyHarwood]

Yes, it did. I tried a variety of FTN, email and local username schemes. All are working for me now.

[NuSkooler]

Before this can be merged, I left a comment in the FTN parser - still appears to have the previous fix?