Bugzilla

When I collected the mbox handling into one place, I screwed up the architecture of the [`MboxParser`](https://searchfox.org/comm-central/search?path=&q=MboxParser) and `MboxMsgInputStream` classes.

I started with `MboxMsgInputStream`, and made it own an mbox parser. This works great for reading single messages (which is the usual case).
But for the cases where we need to scan through the entire mbox, it becomes a complete pain and lots of hoop-jumping is required - we have to keep the same inputstream for multiple messages, and artificially kick it to continue onto the next message. We can't use a new inputstream for each message because we want to keep the state (and buffered data) of the underlying mbox parser owned by the stream.

The hoop jumping was tolerable originally, but as things changed it got more and more complicated.
(eg Bug 1926810 changed the parser so it would always fail if there wasn't a "From " marker at the read position. This caused problems with valid, empty mbox files - see Bug 1935124)

How to clean it up:

The mbox parser needs to be broken out from the `MboxMsgInputStream`.

The changes would all be confined inside `nsMsgBrkMBoxStore`, so nothing outside `nsIMsgPluggableStore` needs to be affected.

There are only two places where `MboxMsgInputStream` (and `MboxParser`) is created, so that's where I'd start from:
https://searchfox.org/comm-central/search?q=new+MboxMsgInputStream&path=&case=false&regexp=false

When I collected the mbox handling into one place, I screwed up the architecture of the [`MboxParser`](https://searchfox.org/comm-central/search?path=&q=MboxParser) and `MboxMsgInputStream` classes.

I started with `MboxMsgInputStream`, and made it own an mbox parser. This works great for reading single messages (which is the usual case).
But for the cases where we need to scan through the entire mbox (compaction, local folder repair), it becomes a complete pain and lots of hoop-jumping is required to handle special cases. We have to keep the same inputstream for multiple messages, and artificially kick it to continue onto the next message. We can't use a new inputstream for each message because we want to keep the state (and buffered data) of the underlying mbox parser owned by the stream.

The hoop jumping was tolerable originally, but as things changed it got more and more complicated.
(eg Bug 1926810 changed the parser so it would always fail if there wasn't a "From " marker at the read position. This caused problems with valid, empty mbox files - see Bug 1935124)

How to clean it up:

The mbox parser needs to be broken out from the `MboxMsgInputStream`.

The changes would all be confined inside `nsMsgBrkMBoxStore`, so nothing outside `nsIMsgPluggableStore` needs to be affected.

There are only two places where `MboxMsgInputStream` (and `MboxParser`) is created, so that's where I'd start from:
https://searchfox.org/comm-central/search?q=new+MboxMsgInputStream&path=&case=false&regexp=false

When I collected the mbox handling into one place, I screwed up the architecture of the [`MboxParser`](https://searchfox.org/comm-central/search?path=&q=MboxParser) and `MboxMsgInputStream` classes.

I started with `MboxMsgInputStream`, and made it own an mbox parser. This works great for reading single messages (which is the usual case).
But when we need to scan through the entire mbox (compaction, local folder repair), it becomes a complete pain and lots of hoop-jumping is required to handle special cases. We have to keep the same inputstream for multiple messages, and artificially kick it to continue onto the next message. We can't use a new inputstream for each message because we want to keep the state (and buffered data) of the underlying mbox parser owned by the stream.

The hoop jumping was tolerable originally, but as things changed it got more and more complicated.
(eg Bug 1926810 changed the parser so it would always fail if there wasn't a "From " marker at the read position. This caused problems with valid, empty mbox files - see Bug 1935124)

How to clean it up:

The mbox parser needs to be broken out from the `MboxMsgInputStream`.

The changes would all be confined inside `nsMsgBrkMBoxStore`, so nothing outside `nsIMsgPluggableStore` needs to be affected.

There are only two places where `MboxMsgInputStream` (and `MboxParser`) is created, so that's where I'd start from:
https://searchfox.org/comm-central/search?q=new+MboxMsgInputStream&path=&case=false&regexp=false