Closed Bug 883464 Opened 11 years ago Closed 11 years ago

[email/IMAP] Have imap.js stop entering IDLE state, avoid spamming non-IDLE servers with NOOPs.

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
1.2 FC (16sep)

People

(Reporter: asuth, Assigned: mcav)

Details

Attachments

(1 file)

imap.js will issue a NOOP command every 10 seconds when we are logged into IMAP servers like Yahoo that don't support IDLE.  On servers that support IDLE, it will re-IDLE every 29 minutes.

RFC 3501 5.1 "Autologout timer" (http://tools.ietf.org/html/rfc3501#section-5.4) calls for a 30 minute no-activity logout regardless of the use of IDLE.

Because we currently don't use IDLE data right now, we should not be entering IDLE mode, and we definitely should not be sending all those NOOPs to yahoo.
data-point: yahoo.com just logged me out after roughly 5 minutes and 30 seconds of inactivity.  ('time' said it was 5m47s between when I started the (manual) login process and when I got disconnected.)
Assignee: nobody → mcav
Status: NEW → ASSIGNED
Attached file pull-request.html
This removes the IDLE/NOOP ping timeout. There are still references to the parser being in an "idle" state, which deals with receiving an unsolicited message (line 517ish of imap.js). I left those in because it looks like that also tracks whether or not we're receiving a command.
Attachment #798065 - Flags: review?(bugmail)
fyi: The base commit for your pull request is somewhat old (base commit was from Monday, new patch from Friday).  My own approach is to keep my master up-to-date with origin/master and fork off of that.  Especially for smaller patches like this, I do tend to also update master and then rebase on top of that before pushing the pull request.  That way the Travis runs and reviewer runs are maximally realistic, etc.  I've cherry-picked your commit locally for review purposes.
Comment on attachment 798065 [details]
pull-request.html

This looks good.  Thank you.  r=asuth

Here's my understanding of what we're doing/what I checked:

- We're removing the logic that triggers the IDLE or NOOP commands.  The logic was located in the "hey, we're all done with this request, let's shift it off and call its callback and stuff" region of the mega-function.

- Our isUnsolicited logic will never return true, but this doesn't matter because we never consumed the generated data anyways.

- We still set isIdle, but nothing cares.  (The isUnsolicited logic checks it, but the ext.idle.state stuff will never change.)

- The noop/idle timers and associated logic has been removed.

- Since we really didn't intend to be using NOOP/IDLE, we had no unit test coverage of this, so there's really no unit tests to update other than the change in timer resets that occur.  (This might potentially fix one or two of our suspicious intermittent failures since the IDLE commands could conceivably be interacting.)

- In terms of user impact, our connections may time-out and be disconnected by the server somewhat more frequently now, but overall we expect a bandwidth/cpu/battery win.  Probably the most noticeable difference would be for servers like dovecot which have a setting like imap_idle_notify_interval that periodically sends an unsolicited string for NAT/other keep-alive purposes every ~2 minutes.


Please only land after you've addressed the gaia commit glitch from the IMAP security bug.  (Which I've totally done before, btw :)
Attachment #798065 - Flags: review?(bugmail) → review+
After fixing the IMAP security bug...

Landed in gelam: https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/874bcb75baf432354c76d2d56afa3789842e3136

Landed in gaia: https://github.com/mozilla-b2g/gaia/commit/104f044438a5b4a9b719eb8141a3affea91b32be
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2 FC (16sep)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: