Hi Aleksei, Thanks for the feedback. I had thought about it but felt that it might be better to leave as much business logic to Java vs native as possible as a general principle. But if it makes more sense to do it native here happy to make the change :)
Thanks, - Anuraag On Thu, Dec 12, 2019, 01:18 Aleks Efimov <aleksej.efi...@oracle.com> wrote: > Hi Anuraag, > > The webrev with the latest patch can be viewed here: > http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/02 > > Thanks for replacing FirstDnsSuffix usages with DnsSuffix. I've double > checked it with native application and can confirm that on my test host > (Microsoft Windows Server 2016 Standard) the DnsSuffix contains correct > information while the FirstDnsSuffix list is empty: > DNS Suffix (From DnsSuffix field): test.domain.com > Number of IP Adapter DNS suffix entries: 0 > > DNS Suffix (From DnsSuffix field): > Number of IP Adapter DNS suffix entries: 0 > > DNS Suffix (From DnsSuffix field): > Number of IP Adapter DNS suffix entries: 0 > > DNS Suffix (From DnsSuffix field): > Number of IP Adapter DNS suffix entries: 0 > > DNS Suffix (From DnsSuffix field): test.domain.com > Number of IP Adapter DNS suffix entries: 0 > > About site-local addresses: Maybe we can move the site-local address check > to native code? i.e. analyze dnsServer->Address.lpSockaddr and filter out > site-local addresses. > > Testing: > Run your latest patch though our CI system - no issues detected. > > -Aleksei > > On 10/12/2019 06:59, Anuraag Agrawal wrote: > > Hi Aleksei, > > Thanks for running the test. I had checked the global search list, but > realize I didn't check connection-specific suffixes which were causing the > issue you see. While the documentation states that FirstDnsSuffix is > populated on Vista+, amazingly it doesn't seem to be > > > https://docs.microsoft.com/en-us/windows/win32/api/iptypes/ns-iptypes-ip_adapter_addresses_lh > > > Chromium has run into the same issue and just doesn't use the field > > > https://cs.chromium.org/chromium/src/net/dns/dns_config_service_win.cc?q=firstdnssuffix&sq=package:chromium&dr=C&l=532 > > So I have gone ahead and switched to using the normal DnsSuffix field. > DnsSuffix and looking up the search list in the registry should generally > give a full list. > > During this testing, other issues I found are that the search list returns > a comma-separated list. So to keep the string list parsing simple, I > changed the delimiter from space to comma. Also, on my machine I would > always have DNS servers like fec0:0:0:ffff:1 on some adapters. At first, I > assumed it is ok to return these, since they are configured by the OS and > should be valid, but I learnt that these are deprecated IPv6 site-local > addresses and should not be used. > > https://en.wikipedia.org/wiki/Unique_local_address > > I used the same check for site-local as Inet6Address, which just sees if > the first bytes are fec0. > > Thanks again - 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..b9a06461ab 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. > @@ -50,30 +50,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; > - > - // 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 (!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) { > + // IPv6 > + if (s.charAt(0) != '[') { > + // Not BSD style > + s = '[' + s + ']'; > + } > + if (s.startsWith("[fec0:")) { > + // Deprecated site-local address. Windows adds > + // such addresses to IPv6 adapters without DNS > + // configured, but these should not actually be > + // used for queries. > + continue; > + } > + } > + if (!l.contains(s)) { > + l.add(s); > + } > + } > + } > + l.trimToSize(); > + return l; > } > > // Load DNS configuration from OS > @@ -81,28 +116,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 { > + // 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 > - // > + // 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..427d7c85eb 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 > @@ -30,6 +30,7 @@ > #include <iprtrmib.h> > #include <time.h> > #include <assert.h> > +#include <winsock2.h> > #include <iphlpapi.h> > > #include "jni_util.h" > @@ -48,8 +49,10 @@ > 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. > + * Utility routine to append s2 to s1 with a comma delimiter. > * strappend(s1="abc", "def") => "abc def" > * strappend(s1="", "def") => "def > */ > @@ -60,41 +63,30 @@ 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; > > /* > * First see if there is a global suffix list specified. > @@ -112,122 +104,55 @@ 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) { > 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->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; > } > > @@ -260,7 +185,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 Tue, Dec 10, 2019 at 1:29 AM Aleks Efimov <aleksej.efi...@oracle.com> > wrote: > >> Hi Anuraag, >> >> The webrev with your latest changes can be found here: >> http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/01 >> >> The copyright changes looks good. >> >> The split by space also looks good with assumption that we will never get >> string with two spaces between IP addresses/DNS suffixes. >> >> I've also performed one test on Windows machine and it shows an issue >> with the retrieval of DNS suffixes: >> Method to call: >> sun.net.dns.ResolverConfiguration.open().searchlist() >> >> Old implementation returns: >> [test.domain.com] >> >> With the latest patch the suffixes list (searchlist) is empty: >> [] >> >> I've collected the following information on the test host that might help >> you to fix the issue: >> >> The host OS: Microsoft Windows Server 2016 Standard >> >> Networking configuration (<---> is for masked addresses/info): >> $ ipconfig /all >> >> Windows IP Configuration >> >> Host Name . . . . . . . . . . . . : host-name-1 >> Primary Dns Suffix . . . . . . . : >> Node Type . . . . . . . . . . . . : Hybrid >> IP Routing Enabled. . . . . . . . : No >> WINS Proxy Enabled. . . . . . . . : No >> DNS Suffix Search List. . . . . . : test.domain.com >> >> Ethernet adapter Ethernet: >> >> Connection-specific DNS Suffix . : test.domain.com >> Description . . . . . . . . . . . : Broadcom NetXtreme-E Virtual >> Function >> Physical Address. . . . . . . . . : <------------> >> DHCP Enabled. . . . . . . . . . . : Yes >> Autoconfiguration Enabled . . . . : Yes >> Link-local IPv6 Address . . . . . : <------------> >> IPv4 Address. . . . . . . . . . . : <------------> >> Subnet Mask . . . . . . . . . . . : <------------> >> Lease Obtained. . . . . . . . . . : <------------> >> Lease Expires . . . . . . . . . . : <------------> >> Default Gateway . . . . . . . . . : <------------> >> DHCP Server . . . . . . . . . . . : <------------> >> DHCPv6 Client DUID. . . . . . . . : <------------> >> DNS Servers . . . . . . . . . . . : <------------> >> <------------> >> NetBIOS over Tcpip. . . . . . . . : Disabled >> >> Tunnel adapter isatap.test.domain.com: >> >> Media State . . . . . . . . . . . : Media disconnected >> Connection-specific DNS Suffix . : test.domain.com >> Description . . . . . . . . . . . : Microsoft ISATAP Adapter #3 >> Physical Address. . . . . . . . . : <------------> >> DHCP Enabled. . . . . . . . . . . : No >> Autoconfiguration Enabled . . . . : Yes >> >> >> - Aleksei >> >> >> On 07/12/2019 09:44, Anuraag Agrawal wrote: >> >> Hi all, >> >> Thanks for the reviews. I've fixed the formatting errors, and I hope I >> fixed the copyright issue. I couldn't find documentation on the format of >> the copyright header, but assume the second number is the year of >> modification to update. I found ResolverConfigurationImpl.c to have an old >> one and updated it to 2019. >> >> I also found the note in the docs that StringTokenizer is deprecated and >> shouldn't be used - I agree it's a good time to replace it. As we have a >> simple split by space, I have gone ahead and used the simpler String.split >> instead of regex, hope that makes sense. While in many code I'd be a little >> concerned with the array allocation, this is only run very infrequently so >> I think it should be ok. >> >> Inline diff 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..47065e805e 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. >> @@ -50,30 +50,55 @@ 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; >> - >> - // 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 space >> + // 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 space. >> + String[] tokens = str.split(" "); >> + ArrayList<String> l = new ArrayList<>(tokens.length); >> + for (String s : tokens) { >> + if (!l.contains(s)) { >> + l.add(s); >> } >> } >> - return ll; >> + l.trimToSize(); >> + 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) { >> + // String is delimited by space >> + 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 (!l.contains(s)) { >> + l.add(s); >> + } >> + } >> + } >> + l.trimToSize(); >> + return l; >> } >> >> // Load DNS configuration from OS >> @@ -81,28 +106,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 { >> + // 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 >> - // >> + // 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. >> 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..85a7935314 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 >> @@ -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 Sat, Dec 7, 2019 at 12:41 AM Alan Bateman <alan.bate...@oracle.com> >> wrote: >> >>> On 06/12/2019 15:34, Aleks Efimov wrote: >>> > : >>> > >>> > I've created webrev with your latest changes - it could be convenient >>> > for other reviewers: >>> > http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/00 >>> > >>> > I will happily sponsor this change once it gets necessary approvals >>> This could be split into two issues but since the code using >>> StringTokenizer is changing significantly then we should replace it with >>> regex while we there. >>> >>> -Alan >>> >> >> >
