1674013 - Unable to fix corrupted MS-Exchange message if opened from a saved local file

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect, P2)

Description

[Alessandro Castellani [:aleca]]

If a locally saved corrupted MS-Exchange is opened via File > Open > Saved Message... the console throws this error when clicking on the Repair message button.

JavaScript error: chrome://openpgp/content/modules/fixExchangeMsg.jsm, line 38: TypeError: can't access property "getUriForMsg", hdr.folder is null

A temporary workaround is for the user to copy this message inside a folder in TB (Archive, Inbox, etc.)

Comment 1

[Alessandro Castellani [:aleca]]

Kai, can you reproduce this issue?

Comment 2

[Magnus Melin [:mkmelin]]

The repair assumes a folder, which it then writes back to as well. Since there's no folder for a message opened from file, it can't work (without significant rewriting to do something almost completely different.)

Comment 3

[Kai Engert (:KaiE:)]

It might be sufficient to introduce a better error message - tell the user we cannot repair files opened this way and instruct to copy.

Comment 4

[Alessandro Castellani [:aleca]]

I can take care of this, so I can also update the test to cover this scenario and prevent the JS error.

Comment 5

[Alessandro Castellani [:aleca]]

Attachment: 1674013-broken-exchange.diff

This patch prevents the repair issue altogether by detecting if the message is inside a folder or not.
If not, we return a notification with the proper information for a successful repair.
No need to run the repair if we already know will fail.

Since there are some strings changes, we should probably not uplift this to 78, which I guess is fine since this is not a super sever issue and we didn't have any report of it so far.

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9186118 [details] [diff] [review]
1674013-broken-exchange.diff

Review of attachment 9186118 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/locales/en-US/messenger/openpgp/openpgp-frontend.ftl
@@ +34,5 @@
>  openpgp-search-signature-key =
>      .label = Discover…
>  
>  # Don't translate the terms "OpenPGP" and "MS-Exchange"
> +openpgp-broken-exchange-opened = This is an OpenPGP message that was apparently corrupted by MS-Exchange and opened from a local file. Copy the message inside a folder by using the Menu item "Message > Copy to" if you want to try an automatic repair.

We should avoid giving instructions on which menu patch to take, as that could easily change over time.
What we usually do when we know the action can't be done, is we just disable the button. I think that could be reasonable for this case as well.

::: mail/locales/en-US/messenger/openpgp/openpgp.ftl
@@ +576,2 @@
>  failed-decrypt = Error - decryption failed
> +fix-broken-exchange-msg-failed = Unable to repair the selected message.

Usually l10n key should be updated, but I guess this just improves the language and doesn't change the message.

But "selected message" is a bit weird. How about "Could not repair this message." or "Repairing this message did not work."

Comment 7

[Alessandro Castellani [:aleca]]

What about "Unable to repair this message."?
I think it's better to keep a simple present verb instead of Could or Repairing...did not.

We should avoid giving instructions on which menu patch to take, as that could easily change over time.
What we usually do when we know the action can't be done, is we just disable the button. I think that could be reasonable for this case as well.

Sure, but I think we should let the user know why we can't repair it and avoid a dead-end interaction.
What about this?
"This is an OpenPGP message that was apparently corrupted by MS-Exchange and it can't be repaired because it was opened from a local file. Copy the message into a mail folder to try an automatic repair."

Comment 8

[Alessandro Castellani [:aleca]]

Attachment: 1674013-broken-exchange.diff

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/90eb59ff31f2
Improve message feedback when trying to fix corrupted MS-Exchange of opened local files. r=mkmelin