Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support matching to local part of recipients #43

Merged
merged 3 commits into from
May 28, 2024

Conversation

piroor
Copy link
Member

@piroor piroor commented May 27, 2024

Which issue(s) this PR fixes:

N/A

What this PR does / why we need it:

Currently this addon supports only matching with the domain part of recipients.
This PR adds support to define patterns containing local part.
You need to write patterns with local part with the special delimiter @, in other words, patterns without @ are treated as just domain patterns for backward compatibility.

How to verify the fixed issue:

Try to define patterns with local part and verify that matched recipients are detected as expected.

The steps to verify:

  1. Assume that you have a GMail account and gmail.com is not an internal domain.
  2. Add a pattern with local part matching to your address to the list of internal domains. For example, if your address is [email protected], you should add piro*@gmail.com.
  3. Compose a new message to multiple recipients including both internal and external domains.
  4. Add your address matching to the pattern you've added, to the list of recipients.
  5. Try to send the message.

Expected result:

Your address matching to the pattern is detected as an internal domain recipient in the confirmation dialog.

.map(
pattern => pattern.toLowerCase()
.replace(/^(-?)@/, '$1') // "@example.com" => "example.com"
.replace(/^(-?)(?![^@]+@)/, '$1?*@') // "example.com" => "?*@example.com"
Copy link
Contributor

@HashidaTKS HashidaTKS May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is example.com really replaced to ?*@example.com ?
/^(-?)(?![^@]+@)/ seems to require @, but example.com has no @.

Also, is ?*@example.com a valid regexp pattern?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Sorry if I misunderstood.)

Copy link
Member Author

@piroor piroor May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(?![^@]+@) is a negative lookahead assertion, so the regexp means "at the start position of the string, there is a - or not, and next to that, there is no part matching to [^@]+@ (any local part)." So it will match to example.com but won't match to [email protected]. As the result, the replacement works as "normalize all input patterns to a full address form containing local part, and fixup missing local part as ?*."

And, patterns at here should be wildcard syntax not regexp. They will be converted to regexp patterns by post operations.

これは否定先読みを使った正規表現で、簡単に言うと、括弧の中の文字列「が無い」場合にのみマッチします。なので、この置換操作は「ローカルパートが無ければ?*@という文字列を付け足して、全項目をローカルパートありのパターンに正規化する」という結果になります。また、ここではまだワイルドカード構文のままでよくて、この後の操作で正規表現への変換を行うようになっています。(先の操作で補った?*@は、..*@に変換され、どんなローカルパートでも受け入れるという正規表現になります。この場合の正規表現は普通は.+@と書くところだとは思いますが、この操作の時点ではワイルドカード構文で表現する必要があるということで、こうなっています。)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(?![^@]+@) is a negative lookahead assertion, so the regexp means "at the start position of the string, there is a - or not, and next to that, there is no part matching to [^@]+@ (any local part)." So it will match to example.com but won't match to [email protected]. As the result, the replacement works as "normalize all input patterns to a full address form containing local part, and fixup missing local part as ?*."

Ah, OK, I understand. it seems good, thanks.

And, patterns at here should be wildcard syntax not regexp. They will be converted to regexp patterns by post operations.

Do we need the first ? for wildcard syntax?
I mean it seems that we can use *@example.com instead of ?*@exmple.com.

Copy link
Member Author

@piroor piroor May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, *@exmple.com (to be converted to .*@example.com) is also acceptable. I was just afraid about invalid addresses like @example.com (missing local part.) Such invalid addresses will be rejected by Thunderbird itself, so we don't need to take care them here, so I agree it should be *@.

Copy link
Contributor

@HashidaTKS HashidaTKS May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, if ?*@exmple.com works fine, I think it was already confirmed by executing tests, I have no objection about using ?*.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a change to simplify the normalized result.

@HashidaTKS
Copy link
Contributor

HashidaTKS commented May 28, 2024

The code looks good to me and I have tested this on Thunderbird, it seems to work fine.

@HashidaTKS HashidaTKS merged commit 3cb06e4 into master May 28, 2024
1 check passed
@kenhys kenhys deleted the matching-to-local-part branch August 5, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants