1931258 - Reading a message from past the end of an mbox file doesn't cause an errror.

Status: RESOLVED FIXED

(MailNews Core :: General, defect)

Description

[Ben Campbell]

Just discovered an extra little quirk to address following on from Bug 1926810
If you Seek() past the end of a filestream, that's not an error.
And if you call Read() while positioned past the end, that doesn't cause an error either. It just returns an EOF.

But we want the mbox to return an error for such cases.

Comment 1

[Ben Campbell]

Attachment: Bug 1931258 - Make sure reading messages from beyond the end of an mbox file throws a read error. r=#thunderbird-reviewers

nsFileInputStream will happily let you Seek() past the end, and Read() from there, all without causing an error.
The Read() just returns 0 bytes of data (indicating EOF).

So this patch just tweaks the mbox parser to require that there is a
message at the given position. Previously there was an exception to allow
nsMsgBrkMBoxStore::AsyncScan() to handle the corner case of an empty mbox
file. But AsyncScan() works fine with this patch, so we're OK.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ca7ad388b86e
Make sure reading messages from beyond the end of an mbox file throws a read error. r=darktrojan,kaie

Comment 3

[Kai Engert [:KaiE:]]

Attachment: 1931258-128.patch

Comment 4

[Geoff Lankow (:darktrojan)]

Comment on attachment 9437643 [details]
Bug 1931258 - Make sure reading messages from beyond the end of an mbox file throws a read error. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): bug 1929105, sort of (actually by me writing a stricter test there)
User impact if declined: errors reading from mboxes go undetected
Testing completed (on c-c, etc.): landed last week
Risk to taking this patch (and alternatives if risky): reasonably low

Comment 5

[Corey Bryant]

Comment on attachment 9437643 [details]
Bug 1931258 - Make sure reading messages from beyond the end of an mbox file throws a read error. r=#thunderbird-reviewers

[Triage Comment]
Approved for beta

Comment 6

[Corey Bryant]

Thunderbird 133.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/bfda4ea4e2f7

Comment 7

[Toby Pilling [:tobyp]]

Please request uplift to 128.5.0

Comment 8

[Ben Campbell]

Comment on attachment 9437670 [details] [diff] [review]
1931258-128.patch

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Makes sure attempts to read messages from beyond end of mbox file actually cause an error. This is important for catching bad storeTokens in the database.
• User impact if declined: errors reading from mboxes go undetected
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Changes mostly isolated to internal MboxMsgInputStream state.

Comment 9

[Magnus Melin [:mkmelin]]

You should use the approval-comm-esr128 flag (and leave that flag set so drivers can decide about it)

Comment 10

[Ben Campbell]

Comment on attachment 9437670 [details] [diff] [review]
1931258-128.patch

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: bad messages in mboxes go undetected
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): low

Comment 11

[Corey Bryant]

Comment on attachment 9437670 [details] [diff] [review]
1931258-128.patch

[Triage Comment]
Approved for esr128

Comment 12

[Corey Bryant]

Thunderbird 128.5.0esr:
https://hg.mozilla.org/releases/comm-esr128/rev/d1e13e29f65e