Opened 11 years ago

Closed 11 years ago

#2807 defect closed fixed (fixed)

twisted.mail.relaymanager.MXCalculator._cbMX raises unhandled IndexError for response with no answers

Reported by: Jean-Paul Calderone Owned by:
Priority: highest Milestone:
Component: names Keywords:
Cc: therve Branch:
Author:

Description

If a hostname has no MX records, getMX will return a Deferred with an empty answers list. _cbMX doesn't handle this case and raises an IndexError when it tries to get the 0th element at the end of the method. _ebMX doesn't handle this exception, so it falls through to user code, which isn't expecting it (it's not documented).

According to RFC 974, in the section headed Interpreting the List of MX RRs:

   It is possible that the list of MXs in the response to the query will
   be empty.  This is a special case.  If the list is empty, mailers
   should treat it as if it contained one RR, an MX RR with a preference
   value of 0, and a host name of REMOTE.  (I.e., REMOTE is its only
   MX).  In addition, the mailer should do no further processing on the
   list, but should attempt to deliver the message to REMOTE.

Change History (7)

comment:1 Changed 11 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted
Priority: highhighest

Done in mxcalc-indexerror-2807

comment:2 Changed 11 years ago by therve

Cc: therve added
Keywords: review removed
Owner: set to Jean-Paul Calderone

test_successWithoutResults is not totally clear (at least to me), because it uses the successful path of _ebMX. Maybe another test with fallbackToDomain set to false would clarify this. Also, please remove trailing whitespaces of relaymanager.

That's it!

comment:3 Changed 11 years ago by Jean-Paul Calderone

Keywords: review added
Owner: changed from Jean-Paul Calderone to therve

Can you clarify this a bit?

test_sucessWithoutResults definitely only exercises one codepath through _ebMX. If one sets fallbackToDomain to False and tries this, one will get an IOError. Is this the case you'd like to see a test for? Or do you think that even with fallbackToDomain set to false, getMX should be returning a Record_MX with the address from the A record of the hostname?

Either could seem reasonable to me, I just want to know which you're asking for.

comment:4 Changed 11 years ago by therve

Keywords: review removed
Owner: changed from therve to Jean-Paul Calderone

It was the former, but it's not totally justified: my problem was that the diff showed a new failure, whereas your test was a success test, but looking at the code more it totally makes sense, so if you feel like adding a new test, great, otherwise please merge.

comment:5 Changed 11 years ago by Jean-Paul Calderone

Okay, great. Since I didn't really touch any code in _ebMX, I don't feel a strong urge to add another test covering that case, so I think I'll just merge.

comment:6 Changed 11 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [21279]) Merge mxcalc-indexerror-2807

Author: exarkun Reviewer: therve Fixes #2807

In MXCalculator, handle the case where an MX lookup succeeds but there are no answers. The defines behavior for this case, now MXCalculator properly implements it.

comment:7 Changed 7 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.