Implement "Header Protection for Cryptographically Protected Email" #7386
Conversation
iequidoo
force-pushed
the
iequidoo/use-em-dash-after-subject
branch
from
51b2395 to
9bc2aee
Compare
iequidoo
force-pushed
the
iequidoo/header-protection
branch
from
4264ed2 to
23c0d0a
Compare
iequidoo
force-pushed
the
iequidoo/header-protection
branch
2 times, most recently
from
c5ff81e to
ff46db6
Compare
iequidoo
force-pushed
the
iequidoo/header-protection
branch
3 times, most recently
from
a187251 to
12621ac
Compare
iequidoo
force-pushed
the
iequidoo/header-protection
branch
from
12621ac to
be769cd
Compare
This comment was marked as resolved.
This comment was marked as resolved.
iequidoo
force-pushed
the
iequidoo/header-protection
branch
2 times, most recently
from
4b44365 to
b083abb
Compare
iequidoo
force-pushed
the
iequidoo/header-protection
branch
2 times, most recently
from
125d413 to
3398068
Compare
link2xt
left a comment
link2xt
left a comment
There was a problem hiding this comment.
The code overall looks good, but I'd really prefer not to have conditional compilation just for a single test.
src/context.rs
Outdated
| pub(crate) connectivities: parking_lot::Mutex<Vec<ConnectivityStore>>, | ||
|
|
||
| #[cfg(test)] | ||
| #[allow(clippy::type_complexity)] |
There was a problem hiding this comment.
| #[allow(clippy::type_complexity)] | |
| #[expect(clippy::type_complexity)] |
src/context.rs
Outdated
| /// see [`Context::get_connectivity()`]. | ||
| pub(crate) connectivities: parking_lot::Mutex<Vec<ConnectivityStore>>, | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
I'd prefer not to have conditional compilation for tests. Since this is only enabled in tests, the test essentially does not test the same code as the code that is running in the apps. This can be done for one or two tests, but if we took this approach in all tests then all the code would be full of ad-hoc hooks introduced just for tests. See the comment in the test as well.
src/mimefactory.rs
Outdated
| // once new core versions are sufficiently deployed. | ||
| let anonymous_recipients = false; | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
Having an utility function behind #[cfg(test)] is fine, but this is changing the tested function. Now we have no Rust tests that run unmodified code.
src/mimeparser/mimeparser_tests.rs
Outdated
| ); | ||
| msg.set_subject("Dinner plans".to_string()); | ||
| let chat_id = alice.create_chat(bob).await.id; | ||
| *alice.pre_encrypt_mime_cb.lock() = Some(|mut mime| { |
There was a problem hiding this comment.
So this is the only test using this hook. And to make sure it tests what it is expected to test, I cannot look at the message, but need to verify that the logic producing legacy display element behind #[cfg(test)] is correct. Just loading a binary from test-data would be fine and not require maintaining the code to construct a message with a legacy display element that we don't actually want to be able to construct.
In a way, this tests is not a mimeparser test, but a test of interoperability between the conditionally compiled mimefactory code and mimeparser.
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
For other reviewers: this is apparently removed because the whole test_utils is conditionally compiled.
iequidoo
force-pushed
the
iequidoo/header-protection
branch
2 times, most recently
from
4396d16 to
c50c2a6
Compare
|
Replaced conditional compilation with |
iequidoo
force-pushed
the
iequidoo/header-protection
branch
2 times, most recently
from
d6e58b3 to
c042288
Compare
…7130) This is a part of implementation of https://www.rfc-editor.org/rfc/rfc9788 "Header Protection for Cryptographically Protected Email".
Omit Legacy Display Elements from "text/plain" and "text/html" (implement 4.5.3.{2,3} of
https://www.rfc-editor.org/rfc/rfc9788 "Header Protection for Cryptographically Protected Email").
…defined in RFC 9788) (#7130) And enable it by default as the standard Header Protection is backward-compatible. Also this tests extra IMF header removal when a message has standard Header Protection since now we can send such messages.
iequidoo
force-pushed
the
iequidoo/header-protection
branch
from
c042288 to
c9a7237
Compare
This implements the most important parts of https://www.rfc-editor.org/rfc/rfc9788.html. Need to recheck if anything else needs implementation.
Close #7130