james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Søren Hilmer ...@tefs.dk>
Subject Re: Regarding BUG 24885: RemoteDelivery only tries one of multiple A record
Date Thu, 11 Dec 2003 08:42:00 GMT
Hi Richard,

Nice solution!
I might do a few small changes like Enumeration -> Iterator.

The only thing (I believe) differing between this and Noel's favorite, is the 
double loop in RemoteDelivery. I think it makes the code clearer, but that is 
a matter of taste.

I like the introduction of the dynamically made prioritylist, on average I 
believe it should be faster than doing the present sort() call in 
findMXRecords().

I will do some testing and hopefully a commit friday (yet another optimistic 
deadline).

This solution however will introduce a new interface to the Mailet API 
(matching the class SMTPHostAddresses) as we of course cannot have 
MailetContext returning an Enumeration/Iterator with element types outside 
the Mailet API, but again as this is an addition it should not break 
backwards compatibility.

--Søren

On Thursday 11 December 2003 01:37, Richard O. Hammer wrote:
> Below you may find code which I have developed and tested to deal with
> this issue.
>
> This solution has a new little type to wrap the data needed in
> RemoteDelivery.  It carries a String hostName and an array of InetAddress.
>
>
> import java.net.InetAddress;
> import org.xbill.DNS.ARecord;
> import org.xbill.DNS.Record;
>
> /** A holder of data generated in the DNSServer and needed in
> RemoteDelivery.  Especially needed for multihomed hosts.
>   */
> public class SMTPHostAddresses {
>    String hostName;
>    InetAddress[] ipAddresses;
>
>    SMTPHostAddresses(Record[] aRecords, String hostName){
>      this.hostName = hostName;
>      if (aRecords == null){
>        ipAddresses = new InetAddress[0];
>      }else{
>        ipAddresses = new InetAddress[aRecords.length];
>        for (int i = 0; i < ipAddresses.length; i++){
>          ipAddresses[i] = ((ARecord)aRecords[i]).getAddress();
>        }
>      }
>    }
>
>    public String getHostname(){
>      return hostName;
>    }
>    public InetAddress[] getAddresses(){
>      return ipAddresses;
>    }
> }
>
>
> The following new method, added to DNSServer, provides the interface
> to the underlying DNS functionality.  This will be used instead of
> DNSServer.findMXRecords(hostname).
>
>
>    /** Performs DNS lookups as needed to find servers which should or
> might
>     * support SMTP.  Returns one SMTPHostAddresses for each such host
> discovered
>     * by DNS.  If no host is found for domainName, the Enumeration
>     * returned will be empty and the first call to hasMoreElements()
> will return
>     * false.
>     * @param domainName the String domain for which SMTP host
> addresses are
>     * sought.
>     * @return an Enumeration in which the Objects returned by
> nextElement()
>     * are instances of SMTPHostAddresses.
>     */
>    public Enumeration getSMTPHostAddresses(final String domainName){
>      return new Enumeration(){
>        private Enumeration mxHosts = new MxSorter(domainName);
>        public boolean hasMoreElements(){
>          return mxHosts.hasMoreElements();
>        }
>        public Object nextElement(){
>          String nextHostname = (String)mxHosts.nextElement();
>          Record[] aRecords = lookup(nextHostname, Type.A);
>          return new SMTPHostAddresses(aRecords, nextHostname);
>        }
>      };
>    }
>
>
> You will notice that this method simply wraps another Enumeration,
> which I will supply below.
>
> But first let me touch upon how this would be used in RemoteDelivery,
> if that is not obvious.  The code immediately below is an untested
> sketch of the structure which I suppose could work in
> RemoteDelivery.deliver().  It contains an outer loop going through the
> Enumeration of SMTPHostAddresses, and an inner loop going through the
> array of InetAddresses in each SMTPHostAddresses.
>
>
>      Enumeration smtpHosts = /* something like */
>                      getMailetContext().getSMTPHostAddresses(host);
>      if(!smtpHosts.hasMoreElements()){
>        //error condition as in RemoteDelivery
>        log("No mail server found for: " + host);
>        StringBuffer exceptionBuffer =
>            new StringBuffer(128)
>            .append("There are no DNS entries for the hostname ")
>            .append(host)
>            .append(".  I cannot determine where to send this message.");
>        return failMessage(
>              mail, new MessagingException(exceptionBuffer.toString()),
> false);
>      }
>      while(smtpHosts.hasMoreElements()){
>        SMTPHostAddresses thisHost =
> (SMTPHostAddresses)smtpHosts.nextElement();
>        String thisHostName = thisHost.getHostname();//if needed for
> logging
>        InetAddress[] addresses = thisHost.getAddresses();
>        for (int addressIndex = 0; addressIndex < addresses.length;
>                   addressIndex++){
>          InetAddress thisAddress = addresses[addressIndex];
>          //attempt SMTP connection at thiAddress
>          //do other work
>        }
>      }
>
>
> I agree with Noel that in some ways a single Enumeration,
> necessitating only a single loop, would be simpler.  But I've already
> got this written, and there are also some ways in which this structure
> seems better.
>
> Now for the biggest block of my code, the inner class MxSorter below
> also goes into DNSServer.  This is the Enumeration that was wrapped
> above in the method DNSServer.getSMTPHostAddresses().
>
>
>    /** A way to get mail hosts to try.  If any MX hosts are found for the
>     * domain name with which this is constructed, then these MX hostnames
>     * are returned in priority sorted order, lowest priority numbers
> coming
>     * first.  And, whenever multiple hosts have the same priority then
> these
>     * are returned in a randomized order within that priority group, as
>     * specified in RFC 2821, Section 5.
>     *
>     * If no MX hosts are found for the domain name, then a DNS search is
>     * performed for an A record.  If an A record is found then
> domainName itself
>     * will be returned by the Enumeration, and it will be the only
> object in
>     * the Enumeration.  If however no A record is found (in addition
> to no MX
>     * record) then the Enumeration constructed will be empty; the
> first call to
>     * its hasMoreElements() will return false.
>     *
>     * This behavior attempts to satisfy the requirements of RFC 2821,
> Section 5.
>     */
>    private class MxSorter implements Enumeration{
>      int priorListPriority = Integer.MIN_VALUE;
>      ArrayList equiPriorityList = new ArrayList();
>      Record[] mxRecords;
>      /* The implementation of this class attempts to achieve efficiency by
>      * performing no more sorting of the rawMxRecords than necessary.
>   In the
>      * large majority of cases the first attempt, made by a client of
> this class
>      * to connect to an SMTP server for a given domain, will succeed.
>   As such,
>      * in most cases only one call will be made to this Enumeration's
>      * nextElement(), and in that majority of cases there will have
> been no need
>      * to sort the array of MX Records.  This implementation would,
> however, be
>      * relatively inefficient in the case where all hosts fail, when
> every
>      * Object is called out of a long Enumeration.
>      */
>
>      MxSorter(String domainName){
>        mxRecords =  lookup(domainName, Type.MX);
>        if (mxRecords == null || mxRecords.length == 0){
>          //no MX records were found, so try to use the domainName
>          Record[] aRecords = lookup(domainName, Type.A);
>          if(aRecords != null && aRecords.length > 0){
>            equiPriorityList.add(domainName);
>          }
>        }
>      }
>
>      /* Sets presentPriorityList to contain all hosts
>       * which have the least priority greater than pastPriority.
>       * When this is called, both (rawMxRecords.length > 0) and
>       * (presentPriorityList.size() == 0), by contract.
>       * In the case where this is called repeatedly, so that
> priorListPriority
>       * has already become the highest of the priorities in the
> rawMxRecords,
>       * then this returns without having added any elements to
>       * presentPriorityList; presentPriorityList.size remains zero.
>       */
>      private void createPriorityList(){
>        int leastPriorityFound = Integer.MAX_VALUE;
>        /* We loop once through the rawMxRecords, finding the lowest
> priority
>         * greater than priorListPriority, and collecting all the hostnames
>         * with that priority into equiPriorityList.
>         */
>        for (int i = 0; i < mxRecords.length; i++) {
>          MXRecord thisRecord = (MXRecord)mxRecords[i];
>          int thisRecordPriority = thisRecord.getPriority();
>          if (thisRecordPriority > priorListPriority) {
>            if (thisRecordPriority < leastPriorityFound) {
>              equiPriorityList.clear();
>              leastPriorityFound = thisRecordPriority;
>              equiPriorityList.add(thisRecord.getTarget().toString());
>            } else if (thisRecordPriority == leastPriorityFound) {
>              equiPriorityList.add(thisRecord.getTarget().toString());
>            }
>          }
>        }
>        priorListPriority = leastPriorityFound;
>      }
>      public boolean hasMoreElements(){
>        if (equiPriorityList.size() > 0){
>          return true;
>        }else if (mxRecords != null && mxRecords.length > 0){
>          createPriorityList();
>          return equiPriorityList.size() > 0;
>        } else{
>          return false;
>        }
>      }
>
>      public Object nextElement(){
>        if (hasMoreElements()){
>          int getIndex = (int)(Math.random()*equiPriorityList.size());
>          Object returnElement = equiPriorityList.get(getIndex);
>          equiPriorityList.remove(getIndex);
>          return returnElement;
>        }else{
>            throw new NoSuchElementException();
>          }
>        }
>    }
>
>
> And finally, if anyone should drop these elements into DNSServer and
> want to test them, here is a main() method for testing which I have
> put into my working copy of DNSServer.
>
>
>    /* For testing.  Displays InetAddresses for SMTP hosts for domain
> names.
>     * Domain names to test may be supplied optionally as arguments to
> main().
>     * But if args is left empty then a hardcoded list of names will be
> tested.
>     * For each domain name given, this lists each hostname and then each
>     * InetAddress for each hostname.  Thus it shows the entire
> sequence of
>     * InetAddresses which may be tried for the given domain.
>     */
>    public static void main(String[] args) throws UnknownHostException{
>      if(args.length == 0){ //if args have not been given, set some
> names to test
>        String[] newArgs = {"nc.rr.com","\r^Illegal.characters",
>            "mailscreen.net","aol.com",
>            "rockland.com","mindspring.com","mx02.mindspring.com.","",
>            "mx2.mail.yahoo.com", "IMpossible.domain-3", "hotmail.com"};
>        args = newArgs;
>      }
>      DNSServer dnsServer = new DNSServer();
>      for (int domainIndex = 0; domainIndex < args.length; domainIndex++){
>        String domainName = args[domainIndex];
>        System.out.println("Results for domain " + domainName);
>        Enumeration hostsForDomain =
> dnsServer.getSMTPHostAddresses(domainName);
>        int hostCount = 0;
>        while (hostsForDomain.hasMoreElements()){
>          SMTPHostAddresses thisHost =
>                (SMTPHostAddresses)hostsForDomain.nextElement();
>          System.out.print(thisHost.getHostname());
>          InetAddress[] addresses = thisHost.getAddresses();
>          for (int addressIndex = 0; addressIndex < addresses.length;
>                   addressIndex++){
>            System.out.print(" " + addresses[addressIndex].toString());
>          }
>          System.out.println();
>          hostCount++;
>        }
>        System.out.println("Done, found " + hostCount + " hosts.\n");
>      }
>    }
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org

-- 
Søren Hilmer, M.Sc.
R&D manager		Phone:	+45 70 27 64 00
TietoEnator IT+		Fax:	+45 70 27 64 40
Ved Lunden 12		Direct:	+45 87 46 64 57
DK-8230 Åbyhøj		Email:	soren.hilmer <at> tietoenator.com



---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message