If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Some IMAP messages are truncated in offline-store. ("This body part will be downloaded on demand.")

RESOLVED FIXED in Thunderbird 3.0b4

Status

Thunderbird
Message Reader UI
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: durumdara, Assigned: Bienvenu)

Tracking

(Blocks: 1 bug, {dataloss})

Trunk
Thunderbird 3.0b4
dataloss
Dependency tree / graph
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [no l10n impact])

Attachments

(6 attachments, 4 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; hu; rv:1.9.0.4) Gecko/2008102920 Firefox/3.0.4
Build Identifier: 3.01b

I got a message with empty body in IMAP. I checked it with Bat, and I saw that message other parts are lost.
Only a little header I got.
'''
From - Tue Dec 09 11:40:25 2008
X-Mozilla-Status: 0001
X-Mozilla-Status2: 00000000
'''

I don't want to delete this message, but how to I got it as whole message?
Any sync operation needed to sync. it!




Reproducible: Didn't try

Steps to Reproduce:
1.
2.
3.
durumdara, can you attach the message to the bug as .eml file? (sanitize personal information if desired)
Component: General → Message Reader UI
QA Contact: general → message-reader
Version: unspecified → Trunk
(Reporter)

Comment 2

9 years ago
(In reply to comment #1)
> durumdara, can you attach the message to the bug as .eml file? (sanitize
> personal information if desired)

Sorry, but after this accident I changed my IMAP settings to "not store mails here", and I deleted all locally stored email. After this I got my message fully...

The chunk mail contained only these lines:
From: XXX
To: XXX

I cannot delete it, because then tdb is fully delete from the server... :-(

I think that sync function need to check the "sizes", and if they are different, tdb must reload the locally stored version from the server... Or some menu needed to "Reload chunk mail again"...

Because here is only one possibility to get original mail: if we remove all of locally stored mails, and resync again...

This function (store mails locally too) is very good , but I think this is dangerous if some size checks are not realized in tdb.
(Reporter)

Comment 3

9 years ago
(In reply to comment #1)
> durumdara, can you attach the message to the bug as .eml file? (sanitize
> personal information if desired)

So: I got this problem again, but the solution is very easy.
It is seems to be index problem. When I exit tdb, delete the inbox.msf (the "inbox" file is kept), and restart tdb, the tdb is reindex the message good.

The problem is caused by "Save attachment(s)" function.
Interesting, but sometimes the save is not finished... And later the index corrupted.

For example:
I've got a message with 3 attachments. The msg full size is 7 MB.
When I do "Save all", the two little attachments are saved good.
But the third element is never finished (until 3 minutes).
When I tried to close/open this message, I saw that this message have ONLY ONE ATTACHMENT NOW. So the TDB lost two attachments in the view.

When I delete the *.msf, and restart tdb, the messages/attachments are fully visible without any tricks.
Adding beckley and bienvenu, since they're most familiar with attachment issues.

Comment 5

9 years ago
(In reply to comment #3)
> The problem is caused by "Save attachment(s)" function.

durumdara, are you sure this happens with "Save" attachments?  Seems like this only has potential to happen with "Detach" or "Delete" attachments, as those would cause the message to get rewritten.
(Reporter)

Comment 6

9 years ago
(In reply to comment #5)
> (In reply to comment #3)
> > The problem is caused by "Save attachment(s)" function.
> 
> durumdara, are you sure this happens with "Save" attachments?  Seems like this
> only has potential to happen with "Detach" or "Delete" attachments, as those
> would cause the message to get rewritten.

I fully sure to I do this. I got these DB file attachments (zip files) (and I need to save them to open, and repair the tables).
When I clicked to "Save as", the TDB asked me for the three filenames. I defined them all, and I wait for the result... But the biggest zip file never created by tdb...
And after this the index file corrupted, so I don't see, only one of the zip files - the smallest...

I've got same problem before, and I determined to I use only IMAP to avoid the "attachment lost". But later I need some files, and I revert to "store mails locally too" option.
(Reporter)

Comment 7

9 years ago
The problem is possible caused by deadlock when more thread or window want to access/modify the index...

Yesterday I saw this bug also:
It was 10 opened mail in my screen. One new mail arrived, and a new again (1 minute later)
In this period I tried to open an old mail attachment (a pps file), and to see a new mail, and to save attachments of this mail.

The new mail window never response after "save attachments", and later I saw empty mail - I lost all informations about it. 
The index is damaged after I need to kill tdb, because the exe remaining in the process list 3 minutes later I close TDB with normal way.

It is sadly but all of my "Labels" and other things are lost... :-(
I must delete the index file to see my important last mail...

So I think that TDB need some index repair tool, or indexes possible should be sqlite databases, or labels and other things are must be stored in other way (XML, etc).
(Reporter)

Comment 8

8 years ago
Again I got this error in two times, but "WITHOUT SAVE"!

Yesterday I got 2 mails in morning. Later I when I want to open them, I see they are lost... 
I started a search in the full account, and TDB found some trunk mails with date 1970, none of subject...

When I deleted the msf files, the mails returned.

Today I got 4 of big mails. The last was visible, but when I clicked on it, it is lost all of properties...

I also deleted the msf-s, and I got these mails...

So this is an index error.

OR: with this account I use HMAILSERVER. Possible HMAILSERVER with TDB is working correctly???

The partially solution is:
"del *.msf /s" on accounts dir - for reindex...

But this is not normal way, and I lost all of my labels/other specs... :-(
Adding dataloss, as this sounds like a dataloss bug, and qawanted, because we really need a reproducible test case in order to get any traction here.
Keywords: dataloss, qawanted
(In reply to comment #3)
> When I delete the *.msf, and restart tdb, the messages/attachments are fully visible without any tricks.

Next two issues are already known.
(1) If error occurs during download, downloaded data in offline-store becomes partial. (2) Once partial data is created in offline-store, there is no ordinal way to recover form partial data. ("Rebuild-Index" or "Move to other folder then move back" is required.)
I don't know (1) is already fixed or not. AFAIK, (2) still exists.

(Q1) Do you really need to use auto-sync/offline-store? If yes, auto-sync is mandatory for all IMAP folders?
Typical error which can produce problem of (1) is "timeout during download".
(Q2) What is set in next prefs.js setting? (Check via Config Editor)
> mailnews.tcptimeout (default: 100sec if Tb 3, 60sec if Tb 2)
(Q3) Will frequency of problem be reduced by larger timeout value?
(I think mail.server.serverN.tcptimeout can be used but I'm not sure.)
(Q4) Can you produce problem with latest trunk build?
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-1.9.1/
> You can test by UNZIP only, if you use win32.zip build.

If problem occurs frequently, get IMAP log with timestamp using latest trunk build, and check IMAP level flow first. Read some comments relate to logging in meta Bug 402793.
WADA, do you happen to know if either or both of (1) and (2) are already filed as bugs, and, if so, what those bug numbers are?
(In reply to comment #11)

I saw description about (1) and (2) in some bug reports for similar phenomenon to this bug, but I don't remember bug numbers. I don't know specific bug for specific issue of (1)/(2) exists or not.
bienvenu, I don't suppose you'd happen to know of existing bugs on (1) and/or (2).  They kinda sound like they should block...
(Assignee)

Comment 14

8 years ago
dmose, I don't know of reproducible steps to cause the issue, no. I've fixed problems in the past in this area once I had reproducible steps...
(Assignee)

Comment 15

8 years ago
One way to detect the corruption and ameliorate this is to compare the offline msg size vs the online message size and if they don't match, clear the Offline flag from the message, which would make us re-fetch it from the server. The thing that's always slightly worrying about that idea is the line ending issue - if the server doesn't give us CRLF (it's supposed to, but gMail, for example, doesn't always do so), or we don't use CRLF in the local store, the count could get off by 1 byte per line.

The code doesn't mark a message as offline unless we think we've successfully downloaded the whole thing, so somehow there's a situation where the fetch url says everything was fetched, but it was not. Or less likely, we got the whole message but somehow part of it wasn't written to the offline store.
Should be easy enough to test against both exact length and either count the lines or just use some fuzz factor heuristic.
(Assignee)

Comment 17

8 years ago
(In reply to comment #16)
> Should be easy enough to test against both exact length and either count the
> lines or just use some fuzz factor heuristic.

a fuzz factor heuristic could fall down in the case that seems to be problematic - messages w/ very large attachments (hence my worry). Line counting isn't foolproof since the server doesn't give us a line count. The one true thing we know is how big the server says the message is - everything else is a calculation on our part that could go wrong. I don't want to overthink this, but I don't want to refetch large messages when we don't have to - that would be painful.

I did write some code to sanity check the offline message size at download time - I should go find that code...
Two pattern was reproduced by test mail of Bug 501253, with Tb latest-trunk.
  Bug 501253 Comment #4 : Partial mail data in offline-store file.
  Bug 501253 Comment #3 : Corrupted mail data in offline-store file.
                          Similar to Comment #0 case.
Created attachment 385968 [details]
NSPR Log of  Bug 501253 Comment #3(Corrupted mail data)

(0) Folder X-01 contains 1 * 10 KB mail only. Offline-use=On. IDLE=Disabled
(1) Window-1 : Click X-01 (1 mail only)
(2) Window-2 : Copy test mail of Bug 501253 from Inbox to X-01
(3) Go Window-1, double-click the copied mail => Window-3 opens(Stand-alone)
(4) Wait for a while => Loading to message pane of three pane ends.
(5) Loading at Window-3 doesn't end (progress meter is still moving)
(6) At this step, file size is; XXX=39KB, XXX.msf=0KB
(7) Close windows, terminate Tb, keep back up XXX(39KB) and XXX.msf(6KB)
Created attachment 385969 [details]
offline-store file(X-01)
Created attachment 385970 [details]
msf file(X-01.msf)
Fetch for part of [1.1.1] (base64 encoded HTML part?) was executed 5 times.
{ (2 per a window) * (2 windows) } + { 1 for retry by error detection } ?
Or due to corruption of offline-store data? (no data for HTML in offline-store because HTML part data is overlaid)

> 00000387   3.71171188 [5008]  792[172a1c0]: 3acf800:imap.gmail.com:S-Test-Offline-1/X-01:SendData: 20 UID fetch 310 (BODY.PEEK[1.1.1])
> 00001621   5.28210354 [5008]  792[172a1c0]: 3acf800:imap.gmail.com:S-Test-Offline-1/X-01:SendData: 24 UID fetch 310 (BODY.PEEK[1.1.1])
> 00002845   6.72838354 [5008]  792[172a1c0]: 3acf800:imap.gmail.com:S-Test-Offline-1/X-01:SendData: 28 UID fetch 310 (BODY.PEEK[1.1.1])
> 00004069   7.91637421 [5008]  792[172a1c0]: 3acf800:imap.gmail.com:S-Test-Offline-1/X-01:SendData: 32 UID fetch 310 (BODY.PEEK[1.1.1])
> 00005350 233.26986694 [5008] 5764[413d080]: 6a10c00:imap.gmail.com:S-Test-Offline-1/X-01:SendData: 18 UID fetch 310 (BODY.PEEK[1.1.1])
Created attachment 385987 [details]
Offline-store file(X-01) which has two mail data(one partial, aother is complete)

As IDLE was seen in previous log, I tested with IDLE=enabled. Double-click of mail was executed slightly later than test of comment #19. (Sorry but I tested without NSPR logging.)
Interesting offline-store file was obtained.

(Mail data B) First mail data in offline-store.
   Call mail data of Comment #19 "Mail data A".
   Note: Only this mail data was obtained when test of Bug 501253 Comment #4. 
> --Boundary_(ID_SjJJqft3zJ3V7JlRrinaFA)
> X-Mozilla-IMAP-Part: 2
> Content-type: application/pdf;
> name="UTF-8?B?SXplbnBlIC0gTm90YSBUw6ljbmljYSA0MCAtIFZ1bG5l?=
> =?UTF-8?B?cmFiaWxpZGFkIGVuIE9wZW5TU0wgeSBEZWJpYW4ucGQ=?=	=?UTF-8?B?Zg==?="
> Content-transfer-encoding: base64
> Content-disposition: attachment;
> filename="UTF-8?B?SXplbnBlIC0gTm90YSBUw6ljbmljYSA0MCAtIFZ1bG5l?=
> =?UTF-8?B?cmFiaWxpZGFkIGVuIE9wZW5TU0wgeSBEZWJpYW4ucGQ=?=	=?UTF-8?B?Zg==?="
> Content-description: =?UTF-8?B?SXplbnBlIC0gTm90YSBUw6ljbmljYSA0MCAtIFZ1bG5l?=
> =?UTF-8?B?cmFiaWxpZGFkIGVuIE9wZW5TU0wgeSBEZWJpYW4ucGQ=?=	=?UTF-8?B?Zg==?=
> 
> This body part will be downloaded on demand.
--Boundary_(ID_SjJJqft3zJ3V7JlRrinaFA)

(Mail data C) Second mail mail data in offline-store:
   Complete data including PDF part.

Next can be said?
(1) Final mail data of (A) or (B) or (C) depends on interrupted timing.
(2) "Second/Complete mail data is obtained or not" (recovered or not)
    depends on timing of interruption of first try of mail download.
As I wrote Bug 498473 Comment #8 and Bug 501253 Comment #5, I couldn't generate partial/corrupted offline-store file data with 2009/6/29 build by utilizing problem of Bug 498473.
Depends on: 501253
Blocks: 501253
No longer depends on: 501253
FYI.
As I wrote in Bug 468637 Comment #10, I could produce partial mail data in
offline-store using test mail attached to Bug 501253, without utilizing known/already-WORKSFORME bug such as Bug 498473.
(Assignee)

Comment 26

8 years ago
I'm going to let this represent the various offline store corruption bugs we've seen, and put in b4.
Assignee: nobody → bienvenu
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0b4
Oh, David has took this bug.
Setting some bugs in dependency for ease of tracking/search(meta bug like usage for a while).
No longer blocks: 501253
Depends on: 468637, 507842, 499408, 501253
Mail structure of test mail attached to Bug 501253 is as follows.
  multipart/mixed
  part-1 : multipart/related
           part-1-1 : multipart/alternative
                      part-1-1-1 : text/plain  (base64)
                      part-1-1-2 : text/html   (base64)
                                   <img=...> points outer part-1-2 
           part-1-2 : image/jpeg
  part-2 : application/pdf
  part-3 : text/plain
Quirks for such structure may be relevant to problem.
FYI.
Bug 501253 Comment #14 is phenomenon when offline-use=Off. I think it demonstrates Tb's behaviour with mail.imap.mime_parts_on_demand=true for above mail structure.
(It also indicates some issues in new Disk Cache feature.)
Depends on: 506877
Adding "This body part will be downloaded on demand." to bug summary for ease of search. Sorry to Threaded View lovers.
Summary: Some messages are downloaded as trunk (any parts lost) → Some messages are downloaded as trunk (any parts lost. "This body part will be downloaded on demand." in offline-store.)
Depends on: 444473
Depends on: 246415
Depends on: 341225
(Assignee)

Updated

8 years ago
Duplicate of this bug: 499408
Summary: Some messages are downloaded as trunk (any parts lost. "This body part will be downloaded on demand." in offline-store.) → Some IMAP messages are truncated in offline-store. ("This body part will be downloaded on demand.")
(Assignee)

Comment 32

8 years ago
Created attachment 394184 [details] [diff] [review]
wip

this catches it when it happens by comparing the size in the offline store vs the size on the server. I can make it so we don't mark messages that don't match in the db, but we'll still get data in the offline store unnecessarily, even though it will be ignored.

I've caught this in the act and what's happening is that you click on a large message with images and a non-inline attachment. We download the body structure and start fetching the images. Since we're idle, auto sync kicks in, and marks the folder as downloading for offline use, even though the url hasn't really started running. Then, the image fetch imap url finishes, and thinks we've been downloading the whole message, and marks it for offline use. I believe that the download autosync url gets queued, but is still marking the folder for offline use pre-maturely. I thought this patch would fix it, but it doesn't. I'll need to get to a windows box to really debug this correctly.
(Assignee)

Comment 33

8 years ago
Created attachment 394882 [details] [diff] [review]
sanity check message size in offline store

this calculates the expected size of the message in the offline store, adjusting for native line endings and the data we add to the message. I'll definitely use this for debugging, and I'll probably also use it to make sure we don't get any bogus messages in the offline store.
(Assignee)

Comment 34

8 years ago
Created attachment 394970 [details] [diff] [review]
fix for offline store corruption issues

While bored out of my mind swimming laps, it occurred to me that instead of trying to make the code work with the existing design, I should move all the state about whether we should store messages offline to the nsIImapUrl, instead of the nsImapMailFolder. This fixes all the race conditions when urls get chained, or interrupted, etc. It's a bit of a scary change, and certainly deserves a mozmill test or two.

Updated

8 years ago
Keywords: qawanted
OS: Windows XP → All
Hardware: x86 → All

Updated

8 years ago
Whiteboard: [no l10n impact]
(Assignee)

Comment 35

8 years ago
Created attachment 395076 [details] [diff] [review]
proposed fix

This moves the knowledge about whether fetch results should be stored in the offline store to the url, from the folder. I've run with this on a few folders with large imap messages and haven't seen any problems. 

This should help SeaMonkey as well, since it's all backend code.

I'm thinking I can test this with xpcshell tests, actually.
Attachment #394184 - Attachment is obsolete: true
Attachment #394882 - Attachment is obsolete: true
Attachment #394970 - Attachment is obsolete: true
Attachment #395076 - Flags: superreview?(neil)
Attachment #395076 - Flags: review?(neil)

Updated

8 years ago
Attachment #395076 - Flags: superreview?(neil)
Attachment #395076 - Flags: superreview+
Attachment #395076 - Flags: review?(neil)
Attachment #395076 - Flags: review?(bugmail)
Comment on attachment 395076 [details] [diff] [review]
proposed fix

I rarely use offline IMAP, maybe asuth would be a better reviewer?

>   ct[24] = 0;
>   result = "From - ";
>   result += ct;
>   result += MSG_LINEBREAK;
>+  m_bytesAddedToLocalMsg = result.Length();
[Isn't this always MSG_LINEBREAK_LEN + 31?]

>   result = "X-Mozilla-Status: 0001";
>   result += MSG_LINEBREAK;
I would be tempted to write this as NS_NAMED_LITERAL_CSTRING(MozillaStatus, "X-Mozilla-Status: 0001" MSG_LINEBREAK);

>-  return m_tempMessageStream->Write(result.get(), result.Length(),
>+  nsresult rv = m_tempMessageStream->Write(result.get(), result.Length(),
>                              &writeCount);
>+  m_bytesAddedToLocalMsg += result.Length();
Is it necessary to add the length afterwards for these two cases? It does seem to mess up the control flow a bit ;-)

>+    m_offlineHeader->GetMessageSize(&messageSize);
>+    messageSize += m_bytesAddedToLocalMsg;
>+    // unix/mac has a one byte line ending, but the imap server returns
>+    // crlf terminated lines.
>+    if (MSG_LINEBREAK_LEN == 1)
>+      messageSize -= m_numOfflineMsgLines;
>+
>+    // We may want to clear the offline flag on the message if the size
>+    // looks wrong.
>+    NS_ASSERTION((PRUint32) curStorePos  >= messageSize,
>+                 "offline message too small");
This should all be #ifdef DEBUG right?

>+  // e.g., we're fetching a message and the folder is configured for offline 
>+  // use and we're not doing mime parts on demand.
>+  attribute boolean storeResultsOffline;
>+  // If we fallback from fetching by parts to fetching the whole message,
>+  // because all the parts were inline, this tells us we should store
>+  // the message offline.
>+  attribute boolean storeOfflineOnFallback;
>+  // Server disconnected first time so we're retrying.
>+  attribute boolean rerunningUrl; 
Nit: couple of trailing spaces. (Apparently there's another one nearer the beginning of the patch that I can't seem to find right now, but then again you managed to remove 5, so you're still ahead of the game!)

>+      if (m_runningUrl)
>+        m_runningUrl->GetStoreResultsOffline(&echoLineToMessageSink);
>     }
>     if (m_imapMessageSink && line && echoLineToMessageSink && !GetPseudoInterrupted())
>+      m_imapMessageSink->ParseAdoptedMsgLine(line, uidOfMessage, m_runningUrl);
I don't quite understand what's going on here - I may have overlooked something, but can't you just pass echoLineToMessageSink rather than getting ParseAdoptedMsgLine to call GetStoreResultsOffline again?
Assuming we're not punting on the unit tests, I'm going to wait on those for my review.
(Assignee)

Comment 38

8 years ago
Comment on attachment 395076 [details] [diff] [review]
proposed fix

yes, I'll request r= once I have some unit tests and have addressed Neil's comments as well.
Attachment #395076 - Flags: review?(bugmail)
Depends on: 512149
Moving bugs in "Depends on:" to "Blocks:".
(Stop meta bug like use. Change back to normal dependency setting.)
(Assignee)

Comment 40

8 years ago
Created attachment 396833 [details] [diff] [review]
fix addressing Neil's comments

Sadly, our imap fake server doesn't do body structure, which makes xpcshell tests rather useless for testing the various permutations of inline and external attachments in messages. Our existing xpcshell tests exercise the basics of downloading messages to offline stores, however. I have written a test that simulates what happens when the user clicks on messages but it's less than exciting since it can't test the complicated cases w/o body structure support. I'll attach it in a minute.
Attachment #395076 - Attachment is obsolete: true
Attachment #396833 - Flags: review?(bugzilla)
+   * @ param aAdoptedMsgLine a string with a lot of message lines,
+                             separated by native line terminators.

that probably won't parse properly.
(Assignee)

Comment 42

8 years ago
Created attachment 396839 [details] [diff] [review]
unit test

this unit test simulates clicking on a message smaller than our mime parts on demand limit, and verifies that the message gets stored in the offline store.

Once our imap fake server supports body structure, this test will be easy to tweak to test that code path as well.

I'm wondering if perhaps we can leverage gloda's MimeMsg abstraction from inside the fake server to generate body structure responses...
Attachment #396839 - Flags: review?(bugzilla)
(Assignee)

Comment 43

8 years ago
(In reply to comment #41)
> +   * @ param aAdoptedMsgLine a string with a lot of message lines,
> +                             separated by native line terminators.
> 
> that probably won't parse properly.

fixed locally, thx for catching that.
Comment on attachment 396833 [details] [diff] [review]
fix addressing Neil's comments

This looks reasonable, though I need to give it some testing. I've got a build going on try server and will hopefully test it out more later or tomorrow.

>+  nsresult rv = m_tempMessageStream->Write(result.get(), result.Length(),
>                              &writeCount);

nit: realign the second line.

>+  // Set to true if we should store the msg(s) for offline use if we can,
>+  // e.g., we're fetching a message and the folder is configured for offline 
>+  // use and we're not doing mime parts on demand.
>+  attribute boolean storeResultsOffline;
>+  // If we fallback from fetching by parts to fetching the whole message,
>+  // because all the parts were inline, this tells us we should store
>+  // the message offline.
>+  attribute boolean storeOfflineOnFallback;

nit: please use /**\n...*/ for multi-line comments

>+  // Server disconnected first time so we're retrying.
>+  attribute boolean rerunningUrl; 

nit: ///

>+    nsCOMPtr<nsIImapService> imapService = do_GetService(NS_IMAPSERVICE_CONTRACTID, &rv);
>+    NS_ENSURE_SUCCESS(rv,rv);

nit: space after comma.
Comment on attachment 396839 [details] [diff] [review]
unit test

The test failed for me on mac:

...
TEST-PASS | /Users/moztest/comm/main/tb/mozilla/_tests/xpcshell/test_imap/unit/test_imapStoreMsgOffline.js | [anonymous : 238] 0 == 0
Doing test 3
TEST-PASS | /Users/moztest/comm/main/tb/mozilla/_tests/xpcshell/test_imap/unit/test_imapStoreMsgOffline.js | [select2ndMsg : 175] 128 != 0
TEST-PASS | /Users/moztest/comm/main/tb/mozilla/_tests/xpcshell/test_imap/unit/test_imapStoreMsgOffline.js | [anonymous : 34] 0 == 0
###!!! ASSERTION: need exacly one CRLF, which must be at end of line: 'PL_strstr(fCurrentLine, CRLF) == fCurrentLine + strlen(fCurrentLine) - 2', file /Users/moztest/comm/main/src/mailnews/imap/src/nsIMAPGenericParser.cpp, line 198

>+    gIMAPService.DisplayMessage(gIMAPInbox.getUriForMsg(msg1),
>+                                            streamListener,
>+                                            null,
>+                                            URLListener,
>+                                            null,
>+                                            url);

nit: these should align with the start of gIMAPInbox (several places in this file).
Attachment #396839 - Flags: review?(bugzilla) → review-
(Assignee)

Comment 46

8 years ago
oh yeah, fake server doesn't do the right thing with line endings. It needs to convert naked LF-terminated lines in messages put in the server into CRLF-terminated lines.
(Assignee)

Comment 47

8 years ago
hmm, this test works fine for me on the mac. I used patch to apply the patch, and not hg qimport/qpush - I'll try that as well and see if somehow there's some difference with the line endings.
(Assignee)

Comment 48

8 years ago
the test files look ok for me on the mac - each line is crlf terminated, and I don't see the assertion. Can you have a quick look at the test files on your system? I think it's external-attach-test that's causing the issue...
(Assignee)

Comment 49

8 years ago
I tried this on a couple different trees on my Mac and didn't see that imap parser assertion.
Comment on attachment 396839 [details] [diff] [review]
unit test

I downloaded this without using qimportbz and it worked this time.
Attachment #396839 - Flags: review- → review+
Comment on attachment 396833 [details] [diff] [review]
fix addressing Neil's comments

This has run fine with the tests I've tried. Let's get it out in the wild for some wider testing.

r=Standard8
Attachment #396833 - Flags: review?(bugzilla) → review+
(Assignee)

Comment 52

8 years ago
(In reply to comment #50)
> (From update of attachment 396839 [details] [diff] [review])
> I downloaded this without using qimportbz and it worked this time.

ah, interesting.
(Assignee)

Comment 53

8 years ago
fix checked in, w/ comments addressed
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Duplicate of this bug: 506877
Checkin ID.
> http://hg.mozilla.org/comm-central/log?rev=468595
> 8f4aadd44e10: fix various causes of corruption of offline imap store, r=standard8, sr=neil, 468595, plus limited unit test
>               Sat, 29 Aug 2009 12:14:45 -0700 - rev 3450
As I wrote in Bug 501253 Comment #21, if test mail attached to Bug 501253, problem of "This body part will be downloaded on demand" and some other problems still occurred, although "corruption of mail data in offline-store file" was not observed.
I'll open separate bug for the complex/not-so-well-structured/base64/html mail case.
I've opened Bug 516211 for problem of Comment #56.

Comment 58

8 years ago
I wonder if it wouldn't make sense to close all the old dependent bugs?
I assume most of them should be resolved/different now that this landed.
(In reply to comment #58)
> I wonder if it wouldn't make sense to close all the old dependent bugs?
> I assume most of them should be resolved/different now that this landed.

wada pinged in each bug on 2009-09-11. I prefer my bug to remain open till I can test. Perhaps other reporters may feel the same.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED

Updated

8 years ago
No longer blocks: 246415
You need to log in before you can comment on or make changes to this bug.