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

Aleks Efimov aleksej.efimov at oracle.com
Thu Jan 9 17:17:15 UTC 2020


Hi Anuuraag,

Latest webrev: http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/05/
Looks fine to me
CI is also happy

Best Regards,
Aleksei

On 09/01/2020 15:22, Anuraag Agrawal wrote:
> Hi all,
>
> Apologies for the spam. I made one more tweak to follow up the 
> previous, and removed the default value of -1 for lastRefresh. Now 
> that there is no semantic meaning to -1, I think unspecified default 
> (effectively zero) is more appropriate and easy to reason about.
>
> 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..aefe44e9b3 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,9 @@
>
>  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.
> @@ -42,38 +42,64 @@ public class ResolverConfigurationImpl
>      // Resolver options
>      private final Options opts;
>
> -    // Addresses have changed
> -    private static boolean changed = false;
> +    // Addresses have changed. We default to true to make sure we
> +    // resolve the first time it is requested.
> +    private static boolean changed = true;
>
>      // Time of last refresh.
> -    private static long lastRefresh = -1;
> +    private static long lastRefresh;
>
>      // 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;
> -
> -    // 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<>();
> -
> -        // comma and space are valid delimiters
> -        StringTokenizer st = new StringTokenizer(str, ", ");
> -        while (st.hasMoreTokens()) {
> -            String s = st.nextToken();
> -            if (!ll.contains(s)) {
> -                ll.add(s);
> +    private static ArrayList<String> searchlist;
> +    private static ArrayList<String> nameservers;
> +
> +    // Parse string that consists of token delimited by comma
> +    // and return ArrayList. Refer to ResolverConfigurationImpl.c and
> +    // strappend to see how the string is created.
> +    private ArrayList<String> stringToList(String str) {
> +        // String is delimited by comma.
> +        String[] tokens = str.split(",");
> +        ArrayList<String> l = new ArrayList<>(tokens.length);
> +        for (String s : tokens) {
> +            if (!s.isEmpty() && !l.contains(s)) {
> +                l.add(s);
>              }
>          }
> -        return ll;
> +        l.trimToSize();
> +        return l;
> +    }
> +
> +    // Parse string that consists of token delimited by comma
> +    // 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) {
> +        // String is delimited by comma
> +        String[] tokens = str.split(",");
> +        ArrayList<String> l = new ArrayList<>(tokens.length);
> +
> +        for (String s : tokens) {
> +            if (!s.isEmpty()) {
> +                if (s.indexOf(':') >= 0 && s.charAt(0) != '[') {
> +                    // Not BSD style
> +                    s = '[' + s + ']';
> +                }
> +                if (!s.isEmpty() && !l.contains(s)) {
> +                    l.add(s);
> +                }
> +            }
> +        }
> +        l.trimToSize();
> +        return l;
>      }
>
>      // Load DNS configuration from OS
> @@ -81,28 +107,34 @@ 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 {
> -            if (lastRefresh >= 0) {
> -                long currTime = System.currentTimeMillis();
> -                if ((currTime - lastRefresh) < TIMEOUT) {
> -                    return;
> -                }
> +            // Otherwise we refresh if TIMEOUT_NANOS has passed since 
> last
> +            // load.
> +            long currTime = System.nanoTime();
> +            // lastRefresh will always have been set once because we 
> start with
> +            // changed = true.
> +            if ((currTime - lastRefresh) < TIMEOUT_NANOS) {
> +                return;
>              }
>          }
>
> -        // load DNS configuration, update timestamp, create
> -        // new HashMaps from the loaded configuration
> -        //
> +        // Native code that uses Windows API to find out the DNS server
> +        // addresses and search suffixes. It builds a comma-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.
>          loadDNSconfig0();
>
> -        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..e830b8e70b 100644
> --- a/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
> +++ b/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All rights 
> reserved.
>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   *
>   * This code is free software; you can redistribute it and/or modify it
> @@ -24,17 +24,15 @@
>   */
>
>  #include <stdlib.h>
> -#include <windows.h>
>  #include <stdio.h>
>  #include <stddef.h>
> -#include <iprtrmib.h>
>  #include <time.h>
>  #include <assert.h>
> -#include <iphlpapi.h>
>
> +#include "net_util.h"
>  #include "jni_util.h"
>
> -#define MAX_STR_LEN         256
> +#define MAX_STR_LEN         1024
>
>  #define STS_NO_CONFIG       0x0             /* no configuration found */
>  #define STS_SL_FOUND        0x1             /* search list found */
> @@ -48,9 +46,11 @@
>  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"
> + * Utility routine to append s2 to s1 with a comma delimiter.
> + *  strappend(s1="abc", "def")  => "abc,def"
>   *  strappend(s1="", "def")     => "def
>   */
>  void strappend(char *s1, char *s2) {
> @@ -60,41 +60,32 @@ void strappend(char *s1, char *s2) {
>          return;
>
>      len = strlen(s1)+1;
> -    if (s1[0] != 0)                         /* needs space character */
> +    if (s1[0] != 0)                         /* needs comma character */
>          len++;
>      if (len + strlen(s2) > MAX_STR_LEN)     /* insufficient space */
>          return;
>
>      if (s1[0] != 0) {
> -        strcat(s1, " ");
> +        strcat(s1, ",");
>      }
>      strcat(s1, 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;
> +    WCHAR *suffix;
> +    DWORD ret, flags;
>      DWORD dwLen;
>      ULONG ulType;
>      char result[MAX_STR_LEN];
>      HANDLE hKey;
> -    int gotSearchList = 0;
> +    SOCKADDR *sockAddr;
> +    struct sockaddr_in6 *sockAddrIpv6;
>
>      /*
>       * First see if there is a global suffix list specified.
> @@ -112,128 +103,74 @@ 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);
> +    adapter = adapters;
> +    while (adapter != NULL) {
> +        // Only load config from enabled adapters.
> +        if (adapter->OperStatus == IfOperStatusUp) {
> +            dnsServer = adapter->FirstDnsServerAddress;
> +            while (dnsServer != NULL) {
> +                sockAddr = dnsServer->Address.lpSockaddr;
> +                if (sockAddr->sa_family == AF_INET6) {
> +                    sockAddrIpv6 = (struct sockaddr_in6 *)sockAddr;
> +                    if (sockAddrIpv6->sin6_scope_id != 0) {
> +                        // An address with a scope is either 
> link-local or
> +                        // site-local, which aren't valid for DNS 
> queries so
> +                        // we can skip them.
> +                        dnsServer = dnsServer->Next;
> +                        continue;
>                      }
>                  }
>
> -                /*
> -                 * 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';
>                  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(sockAddr,
> +  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->DnsSuffix;
> +            if (suffix != NULL) {
> +                ret = WideCharToMultiByte(CP_UTF8, 0, suffix, -1,
> +                    result, sizeof(result), NULL, NULL);
> +                if (ret != 0) {
> +                    strappend(sl, result);
> +                }
> +            }
>          }
> -    }
>
> -    /*
> -     * Free the adpater structure
> -     */
> -    if (adapterP) {
> -        free(adapterP);
> +        adapter = adapter->Next;
>      }
>
> +    free(adapters);
> +
>      return STS_SL_FOUND & STS_NS_FOUND;
>  }
>
>
>  /*
> - * Initialize JNI field IDs.
> + * Initialize JNI field IDs and classes.
>   */
>  JNIEXPORT void JNICALL
>  Java_sun_net_dns_ResolverConfigurationImpl_init0(JNIEnv *env, jclass cls)
> @@ -260,7 +197,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
> @@ -272,8 +209,6 @@ 
> Java_sun_net_dns_ResolverConfigurationImpl_loadDNSconfig0(JNIEnv *env, 
> jclass cl
>          obj = (*env)->NewStringUTF(env, nameservers);
>          CHECK_NULL(obj);
>          (*env)->SetStaticObjectField(env, cls, nameserversID, obj);
> -    } else {
> -        JNU_ThrowOutOfMemoryError(env, "native memory allocation 
> failed");
>      }
>  }
>
>
>
> On Fri, Jan 10, 2020 at 12:17 AM Anuraag Agrawal <anuraaga at gmail.com 
> <mailto:anuraaga at gmail.com>> wrote:
>
>     Hi Daniel,
>
>     Thanks for the review!
>
>     On Thu, Jan 9, 2020 at 9:04 PM Daniel Fuchs
>     <daniel.fuchs at oracle.com <mailto:daniel.fuchs at oracle.com>> wrote:
>
>         Hi,
>
>         Two small remarks:
>
>         ResolverConfigurationImpl.java:
>
>            71             if (!l.contains(s)) {
>
>         Maybe empty strings should be skipped here too.
>
>
>     Makes sense, added the check.
>
>
>         I believe it would be more correct to change this line:
>
>           117             if (lastRefresh >= 0) {
>
>         to
>
>           117             if (lastRefresh != -1) {
>
>         as System.nanoTime() can return negative values.
>
>
>     Had never realized nanoTime can return negative values, I have to
>     check all my other code for bugs now ;) I have gone ahead and
>     removed this check altogether, instead defaulting "changed" to
>     true so we always load on the first time through. I think this
>     should be the most precise.
>
>     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..0f161ea4a8 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,9 @@
>
>      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.
>     @@ -42,38 +42,64 @@ public class ResolverConfigurationImpl
>          // Resolver options
>          private final Options opts;
>
>     -    // Addresses have changed
>     -    private static boolean changed = false;
>     +    // Addresses have changed. We default to true to make sure we
>     +    // resolve the first time it is requested.
>     +    private static boolean changed = true;
>
>          // Time of last refresh.
>          private static long lastRefresh = -1;
>
>          // 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;
>     -
>     -    // 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<>();
>     -
>     -        // comma and space are valid delimiters
>     -        StringTokenizer st = new StringTokenizer(str, ", ");
>     -        while (st.hasMoreTokens()) {
>     -            String s = st.nextToken();
>     -            if (!ll.contains(s)) {
>     -                ll.add(s);
>     +    private static ArrayList<String> searchlist;
>     +    private static ArrayList<String> nameservers;
>     +
>     +    // Parse string that consists of token delimited by comma
>     +    // and return ArrayList. Refer to ResolverConfigurationImpl.c and
>     +    // strappend to see how the string is created.
>     +    private ArrayList<String> stringToList(String str) {
>     +        // String is delimited by comma.
>     +        String[] tokens = str.split(",");
>     +        ArrayList<String> l = new ArrayList<>(tokens.length);
>     +        for (String s : tokens) {
>     +            if (!s.isEmpty() && !l.contains(s)) {
>     +                l.add(s);
>                  }
>              }
>     -        return ll;
>     +        l.trimToSize();
>     +        return l;
>     +    }
>     +
>     +    // Parse string that consists of token delimited by comma
>     +    // 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) {
>     +        // String is delimited by comma
>     +        String[] tokens = str.split(",");
>     +        ArrayList<String> l = new ArrayList<>(tokens.length);
>     +
>     +        for (String s : tokens) {
>     +            if (!s.isEmpty()) {
>     +                if (s.indexOf(':') >= 0 && s.charAt(0) != '[') {
>     +                    // Not BSD style
>     +                    s = '[' + s + ']';
>     +                }
>     +                if (!s.isEmpty() && !l.contains(s)) {
>     +                    l.add(s);
>     +                }
>     +            }
>     +        }
>     +        l.trimToSize();
>     +        return l;
>          }
>
>          // Load DNS configuration from OS
>     @@ -81,28 +107,34 @@ 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 {
>     -            if (lastRefresh >= 0) {
>     -                long currTime = System.currentTimeMillis();
>     -                if ((currTime - lastRefresh) < TIMEOUT) {
>     -                    return;
>     -                }
>     +            // Otherwise we refresh if TIMEOUT_NANOS has passed
>     since last
>     +            // load.
>     +            long currTime = System.nanoTime();
>     +            // lastRefresh will always have been set once because
>     we start with
>     +            // changed = true.
>     +            if ((currTime - lastRefresh) < TIMEOUT_NANOS) {
>     +                return;
>                  }
>              }
>
>     -        // load DNS configuration, update timestamp, create
>     -        // new HashMaps from the loaded configuration
>     -        //
>     +        // Native code that uses Windows API to find out the DNS
>     server
>     +        // addresses and search suffixes. It builds a
>     comma-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.
>              loadDNSconfig0();
>
>     -        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..e830b8e70b 100644
>     --- a/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>     +++ b/src/java.base/windows/native/libnet/ResolverConfigurationImpl.c
>     @@ -1,5 +1,5 @@
>      /*
>     - * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All
>     rights reserved.
>     + * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All
>     rights reserved.
>       * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>       *
>       * This code is free software; you can redistribute it and/or
>     modify it
>     @@ -24,17 +24,15 @@
>       */
>
>      #include <stdlib.h>
>     -#include <windows.h>
>      #include <stdio.h>
>      #include <stddef.h>
>     -#include <iprtrmib.h>
>      #include <time.h>
>      #include <assert.h>
>     -#include <iphlpapi.h>
>
>     +#include "net_util.h"
>      #include "jni_util.h"
>
>     -#define MAX_STR_LEN         256
>     +#define MAX_STR_LEN         1024
>
>      #define STS_NO_CONFIG       0x0             /* no configuration
>     found */
>      #define STS_SL_FOUND        0x1             /* search list found */
>     @@ -48,9 +46,11 @@
>      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"
>     + * Utility routine to append s2 to s1 with a comma delimiter.
>     + *  strappend(s1="abc", "def")  => "abc,def"
>       *  strappend(s1="", "def")     => "def
>       */
>      void strappend(char *s1, char *s2) {
>     @@ -60,41 +60,32 @@ void strappend(char *s1, char *s2) {
>              return;
>
>          len = strlen(s1)+1;
>     -    if (s1[0] != 0)                         /* needs space
>     character */
>     +    if (s1[0] != 0)                         /* needs comma
>     character */
>              len++;
>          if (len + strlen(s2) > MAX_STR_LEN)     /* insufficient space */
>              return;
>
>          if (s1[0] != 0) {
>     -        strcat(s1, " ");
>     +        strcat(s1, ",");
>          }
>          strcat(s1, 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;
>     +    WCHAR *suffix;
>     +    DWORD ret, flags;
>          DWORD dwLen;
>          ULONG ulType;
>          char result[MAX_STR_LEN];
>          HANDLE hKey;
>     -    int gotSearchList = 0;
>     +    SOCKADDR *sockAddr;
>     +    struct sockaddr_in6 *sockAddrIpv6;
>
>          /*
>           * First see if there is a global suffix list specified.
>     @@ -112,128 +103,74 @@ 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);
>     +    adapter = adapters;
>     +    while (adapter != NULL) {
>     +        // Only load config from enabled adapters.
>     +        if (adapter->OperStatus == IfOperStatusUp) {
>     +            dnsServer = adapter->FirstDnsServerAddress;
>     +            while (dnsServer != NULL) {
>     +                sockAddr = dnsServer->Address.lpSockaddr;
>     +                if (sockAddr->sa_family == AF_INET6) {
>     +                    sockAddrIpv6 = (struct sockaddr_in6 *)sockAddr;
>     +                    if (sockAddrIpv6->sin6_scope_id != 0) {
>     +                        // An address with a scope is either
>     link-local or
>     +                        // site-local, which aren't valid for DNS
>     queries so
>     +                        // we can skip them.
>     +                        dnsServer = dnsServer->Next;
>     +                        continue;
>                          }
>                      }
>
>     -                /*
>     -                 * 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';
>                      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(sockAddr,
>     +  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->DnsSuffix;
>     +            if (suffix != NULL) {
>     +                ret = WideCharToMultiByte(CP_UTF8, 0, suffix, -1,
>     +                    result, sizeof(result), NULL, NULL);
>     +                if (ret != 0) {
>     +                    strappend(sl, result);
>     +                }
>     +            }
>              }
>     -    }
>
>     -    /*
>     -     * Free the adpater structure
>     -     */
>     -    if (adapterP) {
>     -        free(adapterP);
>     +        adapter = adapter->Next;
>          }
>
>     +    free(adapters);
>     +
>          return STS_SL_FOUND & STS_NS_FOUND;
>      }
>
>
>      /*
>     - * Initialize JNI field IDs.
>     + * Initialize JNI field IDs and classes.
>       */
>      JNIEXPORT void JNICALL
>      Java_sun_net_dns_ResolverConfigurationImpl_init0(JNIEnv *env,
>     jclass cls)
>     @@ -260,7 +197,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
>     @@ -272,8 +209,6 @@
>     Java_sun_net_dns_ResolverConfigurationImpl_loadDNSconfig0(JNIEnv
>     *env, jclass cl
>              obj = (*env)->NewStringUTF(env, nameservers);
>              CHECK_NULL(obj);
>              (*env)->SetStaticObjectField(env, cls, nameserversID, obj);
>     -    } else {
>     -        JNU_ThrowOutOfMemoryError(env, "native memory allocation
>     failed");
>          }
>      }
>
>
>
>         best regards,
>
>         -- daniel
>
>
>         On 09/01/2020 00:54, Aleks Efimov wrote:
>         > Got the testing results: the CI is happy with the last patch
>         - no JNDI
>         > test failures observed
>         >
>         > Kind Regards,
>         > Aleksei
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20200109/28c0cfda/attachment-0001.htm>


More information about the net-dev mailing list