Updated to support Stalwart Mail server #22
Conversation
There was a problem hiding this comment.
Hi, thanks for your contribution!
- We have unit tests for all regexes here: https://github.com/GermanCoding/Roundcube_TLS_Icon/blob/master/test/TlsIconTest.php. For new regexes, please add new test cases with example Stalwart Received-Headers - including edge cases, if possible. This allows us to both test the code and also have a nice way of understanding the regex. Currently I don't see why the existing Postfix regex doesn't work, so having an example is always great.
- We should also update the README if a new regex is added, but I can do that if necessary.
| @@ -5,6 +5,7 @@ class tls_icon extends rcube_plugin | |||
| const POSTFIX_TLS_REGEX = "/\(using (TLS.*)\) \(/im"; | |||
| const POSTFIX_LOCAL_REGEX = "/\([a-zA-Z]*, from userid [0-9]*\)/im"; | |||
| const SENDMAIL_TLS_REGEX = "/\(version=(TLS.*)\)(\s+for|;)/im"; | |||
| const STALWART_TLS_REGEX = "/\(using (TLSv1\.[23] with cipher .+)\) by .+ \(Stalwart SMTP\)/im"; | |||
There was a problem hiding this comment.
This regex appears similar to the Postfix regex. Is there a specific reason why the existing Postfix regex doesn't work for Stalwart?
There was a problem hiding this comment.
Also, I don't like having overly specific regexes. Currently, TLSv1.2 and TLSv1.3 are the only protocol versions commonly used, but to anticipate a future TLSv1.4 or TLSv2.0 I don't think the regex needs to match only TLSv1\.[23].
| @@ -5,6 +5,7 @@ class tls_icon extends rcube_plugin | |||
| const POSTFIX_TLS_REGEX = "/\(using (TLS.*)\) \(/im"; | |||
| const POSTFIX_LOCAL_REGEX = "/\([a-zA-Z]*, from userid [0-9]*\)/im"; | |||
| const SENDMAIL_TLS_REGEX = "/\(version=(TLS.*)\)(\s+for|;)/im"; | |||
| const STALWART_TLS_REGEX = "/\(using (TLSv1\.[23] with cipher .+)\) by .+ \(Stalwart SMTP\)/im"; | |||
There was a problem hiding this comment.
For reference the name seems to be hard coded: https://github.com/stalwartlabs/mail-server/blob/24df9a0352a948f67ad58ab0d7d7423002863d43/crates/smtp/src/inbound/data.rs#L947
But I do not see the point of keeping it in the regex
| const STALWART_TLS_REGEX = "/\(using (TLSv1\.[23] with cipher .+)\) by .+ \(Stalwart SMTP\)/im"; | |
| const STALWART_TLS_REGEX = "/\(using (TLSv1\.[23] with cipher .+)\)/im"; |
The necessary regex to support Stalwart mail server (https://stalw.art) has been added.