Thunderbird does not handle a possible "NO / BAD" answer to a non-literal+ append command

NEW
Assigned to

Status

MailNews Core
Networking: IMAP
4 years ago
a year ago

People

(Reporter: thomas.cataldo, Assigned: thomas.cataldo)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Created attachment 8501085 [details] [diff] [review]
Patch for the nsImapProtocol.cpp issue

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.101 Safari/537.36

Steps to reproduce:

Tried to append a Draft with a big (bigger than a configured server ratio) attachment, the server refuses with <tag> NO message is too big.


Actual results:

Thunderbird pops a notification with the "message is too big" text, but proceeds with the literal upload. Of course the server parser is not expecting a literal at this point.


Expected results:

The upload should have been aborted.
Tb's behavior when NO to append, when BAD to fllowing data send, is knwn in Bug 693353.
Setting dependncy to this bug.
Blocks: 693353
(Assignee)

Comment 2

4 years ago
Thanks for the link to the other bug. I don't know how I could speed up the review process for my patch.
Attachment #8501085 - Attachment mime type: text/x-patch → text/plain
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

4 years ago
Put review flag on patch and ask one of peers/owners of thunderbird module
https://wiki.mozilla.org/Modules/Thunderbird
(Assignee)

Updated

4 years ago
Attachment #8501085 - Flags: review?(mozilla)

Comment 4

4 years ago
Comment on attachment 8501085 [details] [diff] [review]
Patch for the nsImapProtocol.cpp issue

David is no longer active in reviews
Attachment #8501085 - Flags: review?(mozilla) → review?(mkmelin+mozilla)

Updated

4 years ago
Attachment #8501085 - Attachment is patch: true

Updated

4 years ago
Assignee: nobody → thomas.cataldo

Comment 5

4 years ago
Comment on attachment 8501085 [details] [diff] [review]
Patch for the nsImapProtocol.cpp issue

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

Passing to Irving. With my very limited knowledge of this, the patch looks ok though
Attachment #8501085 - Flags: review?(mkmelin+mozilla) → review?(irving)
Comment on attachment 8501085 [details] [diff] [review]
Patch for the nsImapProtocol.cpp issue

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

Thanks for the patch. I'll r+ this for landing, but if you're interested it would be great to have an automated test case for this. You can see an example of a test for IMAP append in http://dxr.mozilla.org/comm-central/source/mailnews/imap/test/unit/test_saveImapDraft.js.
Attachment #8501085 - Flags: review?(irving) → review+
(Assignee)

Comment 7

4 years ago
Thanks for the review. Would be happy to add a test case but this one requires carefully crafted server responses, eg:
C: A0 login bla bla 
S: A0 OK Complete.
C: A1 APPEND Drafts {6666666666}
S: A1 NO message is too big.

Then ensure that the client does not send anything after that.

I don't really see how to craft this dialog from saveImapDraft.js.
Bug 47311 was found. It looks pretty old problem.
Setting dependency for ease of tracking.
Depends on: 47311
(Assignee)

Comment 9

a year ago
I don't think my patch depends #47311 but it fixes it too.
You need to log in before you can comment on or make changes to this bug.