[PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers

Aleks Efimov aleksej.efimov at oracle.com
Mon Dec 2 16:24:22 UTC 2019


Hey Anuraag,

I've looked at the failures and I believe that your second patch 
accidentally removed 'loadDNSconfig0()' native call.
Putting it back to 
src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java:145 
resolves all these failures.

-Aleksei


On 02/12/2019 13:04, Aleks Efimov wrote:
> Hi Anuraag,
>
> I've submited your patch to our CI system and I'm observing a bunch of 
> NPE failures with such stack trace:
> java.lang.NullPointerException
>     at 
> java.base/sun.net.dns.ResolverConfigurationImpl.allocateListForDelimitedString(ResolverConfigurationImpl.java:108)
>     at 
> java.base/sun.net.dns.ResolverConfigurationImpl.stringToList(ResolverConfigurationImpl.java:68)
>     at 
> java.base/sun.net.dns.ResolverConfigurationImpl.loadConfig(ResolverConfigurationImpl.java:149)
>     at 
> java.base/sun.net.dns.ResolverConfigurationImpl.nameservers(ResolverConfigurationImpl.java:172)
>     at 
> jdk.naming.dns/com.sun.jndi.dns.DnsContextFactory.platformServersAvailable(DnsContextFactory.java:94)
>     at 
> jdk.naming.dns/com.sun.jndi.dns.DnsContextFactory.platformServersUsed(DnsContextFactory.java:171)
>     at 
> jdk.naming.dns/com.sun.jndi.dns.DnsContextFactory.getContext(DnsContextFactory.java:83)
>     at 
> jdk.naming.dns/com.sun.jndi.dns.DnsContextFactory.urlToContext(DnsContextFactory.java:120)
>     at 
> jdk.naming.dns/com.sun.jndi.dns.DnsContextFactory.getInitialContext(DnsContextFactory.java:64)
>     at 
> java.naming/javax.naming.spi.NamingManager.getInitialContext(NamingManager.java:730)
>     at 
> java.naming/javax.naming.InitialContext.getDefaultInitCtx(InitialContext.java:305)
>     at 
> java.naming/javax.naming.InitialContext.init(InitialContext.java:236)
>     at 
> java.naming/javax.naming.InitialContext.<init>(InitialContext.java:208)
>     at 
> java.naming/javax.naming.directory.InitialDirContext.<init>(InitialDirContext.java:130)
>     at LookupWithAnyAttrProp.runTest(LookupWithAnyAttrProp.java:57)
>     at TestBase.launch(TestBase.java:82)
>     at TestBase.run(TestBase.java:50)
>     at LookupWithAnyAttrProp.main(LookupWithAnyAttrProp.java:43)
>     at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
> Method)
>     at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>     at 
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>     at java.base/java.lang.reflect.Method.invoke(Method.java:564)
>     at 
> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>     at java.base/java.lang.Thread.run(Thread.java:832)
>
> Few examples of failing tests:
> test/jdk/com/sun/jndi/dns/FedTests/ListSubInterior.java (and others in 
> the same folder)
> test/jdk/com/sun/jndi/dns/FactoryTests/LookupWithAnyAttrProp.java (and 
> others in the same folder)
>
> Test platform is: Windows Server 2012 R2 6.3
>
> With Best Regards,
> Aleksei
>
> On 25/11/2019 05:09, Anuraag Agrawal wrote:
>> Hi Bernd,
>>
>> Thanks for the look and suggestion. I have updated those comments, as 
>> well as added several more to explain the native code aspects better 
>> which I hope helps future readers of the code. Hope it looks better now.
>>
>> Inline patch follows
>>
>> diff --git 
>> a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java 
>> b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
>> index 2250b3158e..18996f0ee0 100644
>> --- 
>> a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
>> +++ 
>> b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
>> @@ -25,9 +25,10 @@
>>
>>  package sun.net.dns;
>>
>> +import java.util.ArrayList;
>>  import java.util.List;
>> -import java.util.LinkedList;
>>  import java.util.StringTokenizer;
>> +import java.util.concurrent.TimeUnit;
>>
>>  /*
>>   * An implementation of sun.net.ResolverConfiguration for Windows.
>> @@ -50,30 +51,69 @@ public class ResolverConfigurationImpl
>>
>>      // Cache timeout (120 seconds) - should be converted into property
>>      // or configured as preference in the future.
>> -    private static final int TIMEOUT = 120000;
>> +    private static final long TIMEOUT_NANOS = 
>> TimeUnit.SECONDS.toNanos(120);
>>
>>      // DNS suffix list and name servers populated by native method
>>      private static String os_searchlist;
>>      private static String os_nameservers;
>>
>>      // Cached lists
>> -    private static LinkedList<String> searchlist;
>> -    private static LinkedList<String> nameservers;
>> +    private static ArrayList<String> searchlist;
>> +    private static ArrayList<String> nameservers;
>>
>> -    // Parse string that consists of token delimited by space or commas
>> -    // and return LinkedHashMap
>> -    private LinkedList<String> stringToList(String str) {
>> -        LinkedList<String> ll = new LinkedList<>();
>> +    // Parse string that consists of token delimited by space
>> +    // and return ArrayList. Refer to ResolverConfigurationImpl.c and
>> +    // strappend to see how the string is created.
>> +    private ArrayList<String> stringToList(String str) {
>> +        ArrayList<String> l = allocateListForDelimitedString(str);
>>
>> -        // comma and space are valid delimiters
>> -        StringTokenizer st = new StringTokenizer(str, ", ");
>> +        // String is delimited by space.
>> +        StringTokenizer st = new StringTokenizer(str, " ");
>>          while (st.hasMoreTokens()) {
>>              String s = st.nextToken();
>> -            if (!ll.contains(s)) {
>> -                ll.add(s);
>> +            if (!l.contains(s)) {
>> +                l.add(s);
>>              }
>>          }
>> -        return ll;
>> +        return l;
>> +    }
>> +
>> +    // Parse string that consists of token delimited by space
>> +    // and return ArrayList.  Refer to ResolverConfigurationImpl.c and
>> +    // strappend to see how the string is created.
>> +    // In addition to splitting the string, converts IPv6 addresses to
>> +    // BSD-style.
>> +    private ArrayList<String> addressesToList(String str) {
>> +        ArrayList<String> l = allocateListForDelimitedString(str);
>> +
>> +        // String is delimited by space
>> +        StringTokenizer st = new StringTokenizer(str, " ");
>> +        while (st.hasMoreTokens()) {
>> +            String s = st.nextToken();
>> +            if (!s.isEmpty()) {
>> +                if (s.indexOf(':') >= 0 && s.charAt(0) != '[') {
>> +                    // Not BSD style
>> +                    s = '[' + s + ']';
>> +                }
>> +                if (!l.contains(s)) {
>> +                    l.add(s);
>> +                }
>> +            }
>> +        }
>> +        return l;
>> +    }
>> +
>> +    private ArrayList<String> allocateListForDelimitedString(String 
>> str) {
>> +        int num = 0;
>> +        for (int i = 0; i < str.length(); i++) {
>> +            char c = str.charAt(i);
>> +            // String is space separated list of items
>> +            if (c == ' ') {
>> +                num++;
>> +            }
>> +        }
>> +        // Actual num is number of delimiters + 1
>> +        return new ArrayList<String>(num + 1);
>>      }
>>
>>      // Load DNS configuration from OS
>> @@ -81,28 +121,33 @@ public class ResolverConfigurationImpl
>>      private void loadConfig() {
>>          assert Thread.holdsLock(lock);
>>
>> -        // if address have changed then DNS probably changed as well;
>> -        // otherwise check if cached settings have expired.
>> -        //
>> +        // A change in the network address of the machine usually 
>> indicates
>> +        // a change in DNS configuration too so we always refresh 
>> the config
>> +        // after such a change.
>>          if (changed) {
>>              changed = false;
>>          } else {
>> +            // Otherwise we refresh if TIMEOUT_NANOS has passed 
>> since last
>> +            // load.
>>              if (lastRefresh >= 0) {
>> -                long currTime = System.currentTimeMillis();
>> -                if ((currTime - lastRefresh) < TIMEOUT) {
>> +                long currTime = System.nanoTime();
>> +                if ((currTime - lastRefresh) < TIMEOUT_NANOS) {
>>                      return;
>>                  }
>>              }
>>          }
>>
>> -        // load DNS configuration, update timestamp, create
>> -        // new HashMaps from the loaded configuration
>> -        //
>> -        loadDNSconfig0();
>> +        // Native code that uses Windows API to find out the DNS server
>> +        // addresses and search suffixes. It builds a 
>> space-delimited string
>> +        // of nameservers and domain suffixes and sets them to the 
>> static
>> +        // os_nameservers and os_searchlist. We then split these 
>> into Java
>> +        // Lists here.
>>
>> -        lastRefresh = System.currentTimeMillis();
>> +        // Record the time of update and refresh the lists of 
>> addresses /
>> +        // domain suffixes.
>> +        lastRefresh = System.nanoTime();
>>          searchlist = stringToList(os_searchlist);
>> -        nameservers = stringToList(os_nameservers);
>> +        nameservers = addressesToList(os_nameservers);
>>          os_searchlist = null;                       // can be GC'ed
>>          os_nameservers = null;
>>      }
>> diff --git 
>> a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c 
>> b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
>> index f2368bafcb..297a1561ef 100644
>> --- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
>> +++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
>> @@ -73,8 +73,8 @@ const int MAX_TRIES = 3;
>>   * for each adapter on the system. Returned in *adapters.
>>   * Buffer is malloc'd and must be freed (unless error returned)
>>   */
>> -static int getAdapters (JNIEnv *env, IP_ADAPTER_ADDRESSES **adapters) {
>> -    DWORD ret, flags;
>> +int getAdapters (JNIEnv *env, int flags, IP_ADAPTER_ADDRESSES 
>> **adapters) {
>> +    DWORD ret;
>>      IP_ADAPTER_ADDRESSES *adapterInfo;
>>      ULONG len;
>>      int try;
>> @@ -87,9 +87,6 @@ static int getAdapters (JNIEnv *env, 
>> IP_ADAPTER_ADDRESSES **adapters) {
>>      }
>>
>>      len = BUFF_SIZE;
>> -    flags = GAA_FLAG_SKIP_DNS_SERVER;
>> -    flags |= GAA_FLAG_SKIP_MULTICAST;
>> -    flags |= GAA_FLAG_INCLUDE_PREFIX;
>>      ret = GetAdaptersAddresses(AF_UNSPEC, flags, NULL, adapterInfo, 
>> &len);
>>
>>      for (try = 0; ret == ERROR_BUFFER_OVERFLOW && try < MAX_TRIES; 
>> ++try) {
>> @@ -240,7 +237,7 @@ static int ipinflen = 2048;
>>   */
>>  int getAllInterfacesAndAddresses (JNIEnv *env, netif **netifPP)
>>  {
>> -    DWORD ret;
>> +    DWORD ret, flags;
>>      MIB_IPADDRTABLE *tableP;
>>      IP_ADAPTER_ADDRESSES *ptr, *adapters=NULL;
>>      ULONG len=ipinflen, count=0;
>> @@ -296,7 +293,11 @@ int getAllInterfacesAndAddresses (JNIEnv *env, 
>> netif **netifPP)
>>          }
>>      }
>>      free(tableP);
>> -    ret = getAdapters (env, &adapters);
>> +
>> +    flags = GAA_FLAG_SKIP_DNS_SERVER;
>> +    flags |= GAA_FLAG_SKIP_MULTICAST;
>> +    flags |= GAA_FLAG_INCLUDE_PREFIX;
>> +    ret = getAdapters (env, flags, &adapters);
>>      if (ret != ERROR_SUCCESS) {
>>          goto err;
>>      }
>> diff --git 
>> a/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c 
>> b/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>> index 13b28044a5..83100aa9ad 100644
>> --- a/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>> +++ b/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>> @@ -30,6 +30,7 @@
>>  #include <iprtrmib.h>
>>  #include <time.h>
>>  #include <assert.h>
>> +#include <winsock2.h>
>>  #include <iphlpapi.h>
>>
>>  #include "jni_util.h"
>> @@ -48,6 +49,8 @@
>>  static jfieldID searchlistID;
>>  static jfieldID nameserversID;
>>
>> +extern int getAdapters(JNIEnv *env, int flags, IP_ADAPTER_ADDRESSES 
>> **adapters);
>> +
>>  /*
>>   * Utility routine to append s2 to s1 with a space delimiter.
>>   *  strappend(s1="abc", "def")  => "abc def"
>> @@ -72,29 +75,19 @@ void strappend(char *s1, char *s2) {
>>  }
>>
>>  /*
>> - * Windows 2000/XP
>> - *
>> - * Use registry approach based on settings described in Appendix C
>> - * of "Microsoft Windows 2000 TCP/IP Implementation Details".
>> - *
>> - * DNS suffix list is obtained from SearchList registry setting. If
>> - * this is not specified we compile suffix list based on the
>> - * per-connection domain suffix.
>> - *
>> - * DNS name servers and domain settings are on a per-connection
>> - * basic. We therefore enumerate the network adapters to get the
>> - * names of each adapter and then query the corresponding registry
>> - * settings to obtain NameServer/DhcpNameServer and Domain/DhcpDomain.
>> + * Use DNS server addresses returned by GetAdaptersAddresses for 
>> currently
>> + * active interfaces.
>>   */
>> -static int loadConfig(char *sl, char *ns) {
>> -    IP_ADAPTER_INFO *adapterP;
>> -    ULONG size;
>> -    DWORD ret;
>> +static int loadConfig(JNIEnv *env, char *sl, char *ns) {
>> +    IP_ADAPTER_ADDRESSES *adapters, *adapter;
>> +    IP_ADAPTER_DNS_SERVER_ADDRESS *dnsServer;
>> +    SOCKADDR *address;
>> +    IP_ADAPTER_DNS_SUFFIX *suffix;
>> +    DWORD ret, flags;
>>      DWORD dwLen;
>>      ULONG ulType;
>>      char result[MAX_STR_LEN];
>>      HANDLE hKey;
>> -    int gotSearchList = 0;
>>
>>      /*
>>       * First see if there is a global suffix list specified.
>> @@ -112,122 +105,58 @@ static int loadConfig(char *sl, char *ns) {
>>              assert(ulType == REG_SZ);
>>              if (strlen(result) > 0) {
>>                  strappend(sl, result);
>> -                gotSearchList = 1;
>>              }
>>          }
>>          RegCloseKey(hKey);
>>      }
>>
>> -    /*
>> -     * Ask the IP Helper library to enumerate the adapters
>> -     */
>> -    size = sizeof(IP_ADAPTER_INFO);
>> -    adapterP = (IP_ADAPTER_INFO *)malloc(size);
>> -    if (adapterP == NULL) {
>> -        return STS_ERROR;
>> -    }
>> -    ret = GetAdaptersInfo(adapterP, &size);
>> -    if (ret == ERROR_BUFFER_OVERFLOW) {
>> -        IP_ADAPTER_INFO *newAdapterP = (IP_ADAPTER_INFO 
>> *)realloc(adapterP, size);
>> -        if (newAdapterP == NULL) {
>> -            free(adapterP);
>> -            return STS_ERROR;
>> -        }
>> -        adapterP = newAdapterP;
>>
>> -        ret = GetAdaptersInfo(adapterP, &size);
>> +    // We only need DNS server addresses so skip everything else.
>> +    flags = GAA_FLAG_SKIP_UNICAST;
>> +    flags |= GAA_FLAG_SKIP_ANYCAST;
>> +    flags |= GAA_FLAG_SKIP_MULTICAST;
>> +    flags |= GAA_FLAG_SKIP_FRIENDLY_NAME;
>> +    ret = getAdapters(env, flags, &adapters);
>> +    if (ret != ERROR_SUCCESS) {
>> +        return STS_ERROR;
>>      }
>>
>> -    /*
>> -     * Iterate through the list of adapters as registry settings are
>> -     * keyed on the adapter name (GUID).
>> -     */
>> -    if (ret == ERROR_SUCCESS) {
>> -        IP_ADAPTER_INFO *curr = adapterP;
>> -        while (curr != NULL) {
>> -            char key[MAX_STR_LEN];
>> -
>> -            sprintf(key,
>> - 
>>  "SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters\\Interfaces\\%s",
>> -                curr->AdapterName);
>> -
>> -            ret = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
>> -                               key,
>> -                               0,
>> -                               KEY_READ,
>> -                               (PHKEY)&hKey);
>> -            if (ret == ERROR_SUCCESS) {
>> -                DWORD enableDhcp = 0;
>> -
>> -                /*
>> -                 * Is DHCP enabled on this interface
>> -                 */
>> -                dwLen = sizeof(enableDhcp);
>> -                ret = RegQueryValueEx(hKey, "EnableDhcp", NULL, &ulType,
>> - (LPBYTE)&enableDhcp, &dwLen);
>> -
>> -                /*
>> -                 * If we don't have the suffix list when get the Domain
>> -                 * or DhcpDomain. If DHCP is enabled then Domain 
>> overides
>> -                 * DhcpDomain
>> -                 */
>> -                if (!gotSearchList) {
>> -                    result[0] = '\0';
>> -                    dwLen = sizeof(result);
>> -                    ret = RegQueryValueEx(hKey, "Domain", NULL, &ulType,
>> - (LPBYTE)&result, &dwLen);
>> -                    if (((ret != ERROR_SUCCESS) || (strlen(result) 
>> == 0)) &&
>> -                        enableDhcp) {
>> -                        dwLen = sizeof(result);
>> -                        ret = RegQueryValueEx(hKey, "DhcpDomain", 
>> NULL, &ulType,
>> -  (LPBYTE)&result, &dwLen);
>> -                    }
>> -                    if (ret == ERROR_SUCCESS) {
>> -                        assert(ulType == REG_SZ);
>> -                        strappend(sl, result);
>> -                    }
>> -                }
>> -
>> -                /*
>> -                 * Get DNS servers based on NameServer or DhcpNameServer
>> -                 * registry setting. If NameServer is set then it 
>> overrides
>> -                 * DhcpNameServer (even if DHCP is enabled).
>> -                 */
>> -                result[0] = '\0';
>> +    adapter = adapters;
>> +    while (adapter != NULL) {
>> +        // Only load config from enabled adapters.
>> +        if (adapter->OperStatus == IfOperStatusUp) {
>> +            dnsServer = adapter->FirstDnsServerAddress;
>> +            while (dnsServer != NULL) {
>> +                address = dnsServer->Address.lpSockaddr;
>>                  dwLen = sizeof(result);
>> -                ret = RegQueryValueEx(hKey, "NameServer", NULL, &ulType,
>> -                                     (LPBYTE)&result, &dwLen);
>> -                if (((ret != ERROR_SUCCESS) || (strlen(result) == 0)) &&
>> -                    enableDhcp) {
>> -                    dwLen = sizeof(result);
>> -                    ret = RegQueryValueEx(hKey, "DhcpNameServer", 
>> NULL, &ulType,
>> -  (LPBYTE)&result, &dwLen);
>> -                }
>> -                if (ret == ERROR_SUCCESS) {
>> -                    assert(ulType == REG_SZ);
>> +                ret = 
>> WSAAddressToStringA(dnsServer->Address.lpSockaddr,
>> +  dnsServer->Address.iSockaddrLength, NULL,
>> +                          result, &dwLen);
>> +                if (ret == 0) {
>>                      strappend(ns, result);
>>                  }
>>
>> -                /*
>> -                 * Finished with this registry key
>> -                 */
>> -                RegCloseKey(hKey);
>> +                dnsServer = dnsServer->Next;
>>              }
>>
>> -            /*
>> -             * Onto the next adapeter
>> -             */
>> -            curr = curr->Next;
>> +            // Add connection-specific search domains in addition to 
>> global one.
>> +            suffix = adapter->FirstDnsSuffix;
>> +            while (suffix != NULL) {
>> +                ret = WideCharToMultiByte(CP_UTF8, 0, 
>> suffix->String, -1,
>> +                    result, sizeof(result), NULL, NULL);
>> +                if (ret != 0) {
>> +                    strappend(sl, result);
>> +                }
>> +
>> +                suffix = suffix->Next;
>> +            }
>>          }
>> -    }
>>
>> -    /*
>> -     * Free the adpater structure
>> -     */
>> -    if (adapterP) {
>> -        free(adapterP);
>> +        adapter = adapter->Next;
>>      }
>>
>> +    free(adapters);
>> +
>>      return STS_SL_FOUND & STS_NS_FOUND;
>>  }
>>
>> @@ -260,7 +189,7 @@ 
>> Java_sun_net_dns_ResolverConfigurationImpl_loadDNSconfig0(JNIEnv 
>> *env, jclass cl
>>      searchlist[0] = '\0';
>>      nameservers[0] = '\0';
>>
>> -    if (loadConfig(searchlist, nameservers) != STS_ERROR) {
>> +    if (loadConfig(env, searchlist, nameservers) != STS_ERROR) {
>>
>>          /*
>>           * Populate static fields in 
>> sun.net.DefaultResolverConfiguration
>>
>> On Sun, Nov 24, 2019 at 11:21 PM Bernd Eckenfels 
>> <ecki at zusammenkunft.net <mailto:ecki at zusammenkunft.net>> wrote:
>>
>>     Hello Anuraag,
>>
>>     The patch looks like a good idea (especially the additional
>>     cleanup for nanos and arraylist) Just a minor thing I noticed
>>     there are two comments talking about „comma and space“ but the
>>     code below will only tokenize by space. Imguess the comment needs
>>     to be fixed, for the next review round.
>>
>>     Gruss
>>     Bernd
>>     -- 
>>     http://bernd.eckenfels.net
>>     ------------------------------------------------------------------------
>>     *Von:* net-dev <net-dev-bounces at openjdk.java.net
>>     <mailto:net-dev-bounces at openjdk.java.net>> im Auftrag von Anuraag
>>     Agrawal <anuraaga at gmail.com <mailto:anuraaga at gmail.com>>
>>     *Gesendet:* Sonntag, November 24, 2019 10:47 AM
>>     *An:* net-dev at openjdk.java.net <mailto:net-dev at openjdk.java.net>
>>     *Betreff:* [PATCH] 7006496: Use modern Windows API to retrieve OS
>>     DNS servers
>>     Hello,
>>
>>     While investigating DNS resolution failure using Netty
>>     (https://github.com/netty/netty/issues/9796), I came across an
>>     old JDK bug where on Windows, the list of OS name servers
>>     includes those for disabled interfaces
>>     (https://bugs.openjdk.java.net/browse/JDK-7006496). This is
>>     problematic since it will often include values from a previously
>>     registered network that is not available anymore, hence
>>     resolution requests would be guaranteed to fail.
>>
>>     The current approach reads DNS name servers from the registry,
>>     where it isn't possible to read whether an adapter is actually
>>     enabled or not. However, Windows Vista+ (minimum JDK8+ supported
>>     Windows) include an API that can be used to examine whether an
>>     adapter is currently enabled, and also return its DNS servers. So
>>     I've created this patch to update the name server lookup to use
>>     this technique. I can confirm that now only name servers from
>>     enabled adapters are loaded. A side effect is the previous
>>     approach could only read IPv4 - now IPv6 DNS servers are also
>>     loaded. The unix implementation seems to do this too, so I don't
>>     think this should cause issues but I can filter to only IPv4 to
>>     be more conservative if desired.
>>
>>     I have also cleaned some low hanging fruit in what seems to be
>>     very old code
>>
>>     - Use ArrayList instead of LinkedList since it can be accurately
>>     preallocated and is not mutated
>>     - Use nanoTime instead of currentTimeMillis for refresh timer to
>>     not cause problems during OS time changes.
>>     - Simplify tokenizer to only tokenize on space, there is never a
>>     comma separated list passed to it as we create the list in our
>>     JNI code
>>
>>     This is my first patch for OpenJDK - happy to accept any feedback.
>>
>>     Thanks for the consideration.
>>
>>     Inlined patch follows (also attached but I've had attached patch
>>     scrubbed when submitting to jmh-dev before...)
>>
>>     diff --git
>>     a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
>>     b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
>>     index 2250b3158e..dde871cf19 100644
>>     ---
>>     a/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
>>     +++
>>     b/src/java.base/windows/classes/sun/net/dns/ResolverConfigurationImpl.java
>>     @@ -25,9 +25,10 @@
>>
>>      package sun.net.dns;
>>
>>     +import java.util.ArrayList;
>>      import java.util.List;
>>     -import java.util.LinkedList;
>>      import java.util.StringTokenizer;
>>     +import java.util.concurrent.TimeUnit;
>>
>>      /*
>>       * An implementation of sun.net.ResolverConfiguration for Windows.
>>     @@ -50,30 +51,65 @@ public class ResolverConfigurationImpl
>>
>>          // Cache timeout (120 seconds) - should be converted into
>>     property
>>          // or configured as preference in the future.
>>     -    private static final int TIMEOUT = 120000;
>>     +    private static final long TIMEOUT_NANOS =
>>     TimeUnit.SECONDS.toNanos(120);
>>
>>          // DNS suffix list and name servers populated by native method
>>          private static String os_searchlist;
>>          private static String os_nameservers;
>>
>>          // Cached lists
>>     -    private static LinkedList<String> searchlist;
>>     -    private static LinkedList<String> nameservers;
>>     +    private static ArrayList<String> searchlist;
>>     +    private static ArrayList<String> nameservers;
>>
>>     -    // Parse string that consists of token delimited by space or
>>     commas
>>     -    // and return LinkedHashMap
>>     -    private LinkedList<String> stringToList(String str) {
>>     -        LinkedList<String> ll = new LinkedList<>();
>>     +    // Parse string that consists of token delimited by space
>>     +    // and return ArrayList
>>     +    private ArrayList<String> stringToList(String str) {
>>     +        ArrayList<String> l = allocateListForDelimitedString(str);
>>
>>              // comma and space are valid delimiters
>>     -        StringTokenizer st = new StringTokenizer(str, ", ");
>>     +        StringTokenizer st = new StringTokenizer(str, " ");
>>              while (st.hasMoreTokens()) {
>>                  String s = st.nextToken();
>>     -            if (!ll.contains(s)) {
>>     -                ll.add(s);
>>     +            if (!l.contains(s)) {
>>     +                l.add(s);
>>                  }
>>              }
>>     -        return ll;
>>     +        return l;
>>     +    }
>>     +
>>     +    // Parse string that consists of token delimited by space
>>     +    // and return ArrayList. Converts IPv6 addresses to BSD-style.
>>     +    private ArrayList<String> addressesToList(String str) {
>>     +        ArrayList<String> l = allocateListForDelimitedString(str);
>>     +
>>     +        // comma and space are valid delimiters
>>     +        StringTokenizer st = new StringTokenizer(str, " ");
>>     +        while (st.hasMoreTokens()) {
>>     +            String s = st.nextToken();
>>     +            if (!s.isEmpty()) {
>>     +                if (s.indexOf(':') >= 0 && s.charAt(0) != '[') {
>>     +                    // Not BSD style
>>     +                    s = '[' + s + ']';
>>     +                }
>>     +                if (!l.contains(s)) {
>>     +                    l.add(s);
>>     +                }
>>     +            }
>>     +        }
>>     +        return l;
>>     +    }
>>     +
>>     +    private ArrayList<String>
>>     allocateListForDelimitedString(String str) {
>>     +        int num = 0;
>>     +        for (int i = 0; i < str.length(); i++) {
>>     +            char c = str.charAt(i);
>>     +            // String is space separated list of items
>>     +            if (c == ' ') {
>>     +                num++;
>>     +            }
>>     +        }
>>     +        // Actual num is number of delimiters + 1
>>     +        return new ArrayList<String>(num + 1);
>>          }
>>
>>          // Load DNS configuration from OS
>>     @@ -88,8 +124,8 @@ public class ResolverConfigurationImpl
>>                  changed = false;
>>              } else {
>>                  if (lastRefresh >= 0) {
>>     -                long currTime = System.currentTimeMillis();
>>     -                if ((currTime - lastRefresh) < TIMEOUT) {
>>     +                long currTime = System.nanoTime();
>>     +                if ((currTime - lastRefresh) < TIMEOUT_NANOS) {
>>                          return;
>>                      }
>>                  }
>>     @@ -100,9 +136,9 @@ public class ResolverConfigurationImpl
>>              //
>>              loadDNSconfig0();
>>
>>     -        lastRefresh = System.currentTimeMillis();
>>     +        lastRefresh = System.nanoTime();
>>              searchlist = stringToList(os_searchlist);
>>     -        nameservers = stringToList(os_nameservers);
>>     +        nameservers = addressesToList(os_nameservers);
>>              os_searchlist = null;   // can be GC'ed
>>              os_nameservers = null;
>>          }
>>     diff --git
>>     a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
>>     b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
>>     index f2368bafcb..297a1561ef 100644
>>     --- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
>>     +++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
>>     @@ -73,8 +73,8 @@ const int MAX_TRIES = 3;
>>       * for each adapter on the system. Returned in *adapters.
>>       * Buffer is malloc'd and must be freed (unless error returned)
>>       */
>>     -static int getAdapters (JNIEnv *env, IP_ADAPTER_ADDRESSES
>>     **adapters) {
>>     -    DWORD ret, flags;
>>     +int getAdapters (JNIEnv *env, int flags, IP_ADAPTER_ADDRESSES
>>     **adapters) {
>>     +    DWORD ret;
>>          IP_ADAPTER_ADDRESSES *adapterInfo;
>>          ULONG len;
>>          int try;
>>     @@ -87,9 +87,6 @@ static int getAdapters (JNIEnv *env,
>>     IP_ADAPTER_ADDRESSES **adapters) {
>>          }
>>
>>          len = BUFF_SIZE;
>>     -    flags = GAA_FLAG_SKIP_DNS_SERVER;
>>     -    flags |= GAA_FLAG_SKIP_MULTICAST;
>>     -    flags |= GAA_FLAG_INCLUDE_PREFIX;
>>          ret = GetAdaptersAddresses(AF_UNSPEC, flags, NULL,
>>     adapterInfo, &len);
>>
>>          for (try = 0; ret == ERROR_BUFFER_OVERFLOW && try <
>>     MAX_TRIES; ++try) {
>>     @@ -240,7 +237,7 @@ static int ipinflen = 2048;
>>       */
>>      int getAllInterfacesAndAddresses (JNIEnv *env, netif **netifPP)
>>      {
>>     -    DWORD ret;
>>     +    DWORD ret, flags;
>>          MIB_IPADDRTABLE *tableP;
>>          IP_ADAPTER_ADDRESSES *ptr, *adapters=NULL;
>>          ULONG len=ipinflen, count=0;
>>     @@ -296,7 +293,11 @@ int getAllInterfacesAndAddresses (JNIEnv
>>     *env, netif **netifPP)
>>              }
>>          }
>>          free(tableP);
>>     -    ret = getAdapters (env, &adapters);
>>     +
>>     +    flags = GAA_FLAG_SKIP_DNS_SERVER;
>>     +    flags |= GAA_FLAG_SKIP_MULTICAST;
>>     +    flags |= GAA_FLAG_INCLUDE_PREFIX;
>>     +    ret = getAdapters (env, flags, &adapters);
>>          if (ret != ERROR_SUCCESS) {
>>              goto err;
>>          }
>>     diff --git
>>     a/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>>     b/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>>     index 13b28044a5..83100aa9ad 100644
>>     --- a/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>>     +++ b/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>>     @@ -30,6 +30,7 @@
>>      #include <iprtrmib.h>
>>      #include <time.h>
>>      #include <assert.h>
>>     +#include <winsock2.h>
>>      #include <iphlpapi.h>
>>
>>      #include "jni_util.h"
>>     @@ -48,6 +49,8 @@
>>      static jfieldID searchlistID;
>>      static jfieldID nameserversID;
>>
>>     +extern int getAdapters(JNIEnv *env, int flags,
>>     IP_ADAPTER_ADDRESSES **adapters);
>>     +
>>      /*
>>       * Utility routine to append s2 to s1 with a space delimiter.
>>       *  strappend(s1="abc", "def")  => "abc def"
>>     @@ -72,29 +75,19 @@ void strappend(char *s1, char *s2) {
>>      }
>>
>>      /*
>>     - * Windows 2000/XP
>>     - *
>>     - * Use registry approach based on settings described in Appendix C
>>     - * of "Microsoft Windows 2000 TCP/IP Implementation Details".
>>     - *
>>     - * DNS suffix list is obtained from SearchList registry setting. If
>>     - * this is not specified we compile suffix list based on the
>>     - * per-connection domain suffix.
>>     - *
>>     - * DNS name servers and domain settings are on a per-connection
>>     - * basic. We therefore enumerate the network adapters to get the
>>     - * names of each adapter and then query the corresponding registry
>>     - * settings to obtain NameServer/DhcpNameServer and
>>     Domain/DhcpDomain.
>>     + * Use DNS server addresses returned by GetAdaptersAddresses for
>>     currently
>>     + * active interfaces.
>>       */
>>     -static int loadConfig(char *sl, char *ns) {
>>     -    IP_ADAPTER_INFO *adapterP;
>>     -    ULONG size;
>>     -    DWORD ret;
>>     +static int loadConfig(JNIEnv *env, char *sl, char *ns) {
>>     +    IP_ADAPTER_ADDRESSES *adapters, *adapter;
>>     +    IP_ADAPTER_DNS_SERVER_ADDRESS *dnsServer;
>>     +    SOCKADDR *address;
>>     +    IP_ADAPTER_DNS_SUFFIX *suffix;
>>     +    DWORD ret, flags;
>>          DWORD dwLen;
>>          ULONG ulType;
>>          char result[MAX_STR_LEN];
>>          HANDLE hKey;
>>     -    int gotSearchList = 0;
>>
>>          /*
>>           * First see if there is a global suffix list specified.
>>     @@ -112,122 +105,58 @@ static int loadConfig(char *sl, char *ns) {
>>                  assert(ulType == REG_SZ);
>>                  if (strlen(result) > 0) {
>>                      strappend(sl, result);
>>     -                gotSearchList = 1;
>>                  }
>>              }
>>              RegCloseKey(hKey);
>>          }
>>
>>     -    /*
>>     -     * Ask the IP Helper library to enumerate the adapters
>>     -     */
>>     -    size = sizeof(IP_ADAPTER_INFO);
>>     -    adapterP = (IP_ADAPTER_INFO *)malloc(size);
>>     -    if (adapterP == NULL) {
>>     -        return STS_ERROR;
>>     -    }
>>     -    ret = GetAdaptersInfo(adapterP, &size);
>>     -    if (ret == ERROR_BUFFER_OVERFLOW) {
>>     -        IP_ADAPTER_INFO *newAdapterP = (IP_ADAPTER_INFO
>>     *)realloc(adapterP, size);
>>     -        if (newAdapterP == NULL) {
>>     -            free(adapterP);
>>     -            return STS_ERROR;
>>     -        }
>>     -        adapterP = newAdapterP;
>>
>>     -        ret = GetAdaptersInfo(adapterP, &size);
>>     +    // We only need DNS server addresses so skip everything else.
>>     +    flags = GAA_FLAG_SKIP_UNICAST;
>>     +    flags |= GAA_FLAG_SKIP_ANYCAST;
>>     +    flags |= GAA_FLAG_SKIP_MULTICAST;
>>     +    flags |= GAA_FLAG_SKIP_FRIENDLY_NAME;
>>     +    ret = getAdapters(env, flags, &adapters);
>>     +    if (ret != ERROR_SUCCESS) {
>>     +        return STS_ERROR;
>>          }
>>
>>     -    /*
>>     -     * Iterate through the list of adapters as registry settings are
>>     -     * keyed on the adapter name (GUID).
>>     -     */
>>     -    if (ret == ERROR_SUCCESS) {
>>     -        IP_ADAPTER_INFO *curr = adapterP;
>>     -        while (curr != NULL) {
>>     -            char key[MAX_STR_LEN];
>>     -
>>     -            sprintf(key,
>>     -
>>      "SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters\\Interfaces\\%s",
>>     -                curr->AdapterName);
>>     -
>>     -            ret = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
>>     -                               key,
>>     -                               0,
>>     -                               KEY_READ,
>>     -                               (PHKEY)&hKey);
>>     -            if (ret == ERROR_SUCCESS) {
>>     -                DWORD enableDhcp = 0;
>>     -
>>     -                /*
>>     -                 * Is DHCP enabled on this interface
>>     -                 */
>>     -                dwLen = sizeof(enableDhcp);
>>     -                ret = RegQueryValueEx(hKey, "EnableDhcp", NULL,
>>     &ulType,
>>     - (LPBYTE)&enableDhcp, &dwLen);
>>     -
>>     -                /*
>>     -                 * If we don't have the suffix list when get the
>>     Domain
>>     -                 * or DhcpDomain. If DHCP is enabled then Domain
>>     overides
>>     -                 * DhcpDomain
>>     -                 */
>>     -                if (!gotSearchList) {
>>     -                    result[0] = '\0';
>>     -                    dwLen = sizeof(result);
>>     -                    ret = RegQueryValueEx(hKey, "Domain", NULL,
>>     &ulType,
>>     - (LPBYTE)&result, &dwLen);
>>     -                    if (((ret != ERROR_SUCCESS) ||
>>     (strlen(result) == 0)) &&
>>     -                        enableDhcp) {
>>     -                        dwLen = sizeof(result);
>>     -                        ret = RegQueryValueEx(hKey,
>>     "DhcpDomain", NULL, &ulType,
>>     -  (LPBYTE)&result, &dwLen);
>>     -                    }
>>     -                    if (ret == ERROR_SUCCESS) {
>>     -                        assert(ulType == REG_SZ);
>>     -                        strappend(sl, result);
>>     -                    }
>>     -                }
>>     -
>>     -                /*
>>     -                 * Get DNS servers based on NameServer or
>>     DhcpNameServer
>>     -                 * registry setting. If NameServer is set then
>>     it overrides
>>     -                 * DhcpNameServer (even if DHCP is enabled).
>>     -                 */
>>     -                result[0] = '\0';
>>     +    adapter = adapters;
>>     +    while (adapter != NULL) {
>>     +        // Only load config from enabled adapters.
>>     +        if (adapter->OperStatus == IfOperStatusUp) {
>>     +            dnsServer = adapter->FirstDnsServerAddress;
>>     +            while (dnsServer != NULL) {
>>     +                address = dnsServer->Address.lpSockaddr;
>>                      dwLen = sizeof(result);
>>     -                ret = RegQueryValueEx(hKey, "NameServer", NULL,
>>     &ulType,
>>     - (LPBYTE)&result, &dwLen);
>>     -                if (((ret != ERROR_SUCCESS) || (strlen(result)
>>     == 0)) &&
>>     -                    enableDhcp) {
>>     -                    dwLen = sizeof(result);
>>     -                    ret = RegQueryValueEx(hKey,
>>     "DhcpNameServer", NULL, &ulType,
>>     -  (LPBYTE)&result, &dwLen);
>>     -                }
>>     -                if (ret == ERROR_SUCCESS) {
>>     -                    assert(ulType == REG_SZ);
>>     +                ret =
>>     WSAAddressToStringA(dnsServer->Address.lpSockaddr,
>>     +  dnsServer->Address.iSockaddrLength, NULL,
>>     +                          result, &dwLen);
>>     +                if (ret == 0) {
>>                          strappend(ns, result);
>>                      }
>>
>>     -                /*
>>     -                 * Finished with this registry key
>>     -                 */
>>     -                RegCloseKey(hKey);
>>     +                dnsServer = dnsServer->Next;
>>                  }
>>
>>     -            /*
>>     -             * Onto the next adapeter
>>     -             */
>>     -            curr = curr->Next;
>>     +            // Add connection-specific search domains in
>>     addition to global one.
>>     +            suffix = adapter->FirstDnsSuffix;
>>     +            while (suffix != NULL) {
>>     +                ret = WideCharToMultiByte(CP_UTF8, 0,
>>     suffix->String, -1,
>>     +                    result, sizeof(result), NULL, NULL);
>>     +                if (ret != 0) {
>>     +                    strappend(sl, result);
>>     +                }
>>     +
>>     +                suffix = suffix->Next;
>>     +            }
>>              }
>>     -    }
>>
>>     -    /*
>>     -     * Free the adpater structure
>>     -     */
>>     -    if (adapterP) {
>>     -        free(adapterP);
>>     +        adapter = adapter->Next;
>>          }
>>
>>     +    free(adapters);
>>     +
>>          return STS_SL_FOUND & STS_NS_FOUND;
>>      }
>>
>>     @@ -260,7 +189,7 @@
>>     Java_sun_net_dns_ResolverConfigurationImpl_loadDNSconfig0(JNIEnv
>>     *env, jclass cl
>>          searchlist[0] = '\0';
>>          nameservers[0] = '\0';
>>
>>     -    if (loadConfig(searchlist, nameservers) != STS_ERROR) {
>>     +    if (loadConfig(env, searchlist, nameservers) != STS_ERROR) {
>>
>>              /*
>>               * Populate static fields in
>>     sun.net.DefaultResolverConfiguration
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20191202/9a696fec/attachment-0001.html>


More information about the net-dev mailing list