[PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers
Aleks Efimov
aleksej.efimov at oracle.com
Thu Jan 9 00:54:08 UTC 2020
Got the testing results: the CI is happy with the last patch - no JNDI
test failures observed
Kind Regards,
Aleksei
On 08/01/2020 18:23, Aleks Efimov wrote:
> Hi Anuraag,
>
> I've uploaded your latest patch to the following location:
> http://cr.openjdk.java.net/~aefimov/anuraaga/7006496/04
>
> The local Windows build and the acquired configuration are good.
>
> I will run you patch through our CI system and will update this thread
> once I get the results.
>
> Kind Regards,
> Aleksei
>
> On 08/01/2020 15:50, Anuraag Agrawal wrote:
>> Hi Chris,
>>
>> Ah ok - I was worried it might be a backwards incompatible change,
>> but I guess these are internal classes anyways. I have updated the
>> patch, removing the exception handling logic so the errors propagate up.
>>
>> 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..c3ebe09776 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 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 && 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
>> 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 Thu, Jan 9, 2020 at 12:27 AM Chris Hegarty
>> <chris.hegarty at oracle.com <mailto:chris.hegarty at oracle.com>> wrote:
>>
>>
>>> On 8 Jan 2020, at 05:12, Anuraag Agrawal <anuraaga at gmail.com
>>> <mailto:anuraaga at gmail.com>> wrote:
>>>
>>> Hi Chris,
>>>
>>> Happy new year - sorry for the long delay. I have made an
>>> updated patch fixing the comment (missing comma), bumping
>>> MAX_STR_LEN, and tweaking the exception behavior.
>>>
>>> Basically, as you pointed out loadDNSConfig itself does not
>>> throw any exception. The getAdapters function will throw a)
>>> OutOfMemoryError if native memory allocation failed or b) Error
>>> if the Windows SDK function randomly failed. I have gone ahead
>>> and added code to check the exception thrown when getAdapters
>>> failed and propagate it if it is OOME or clear it otherwise.
>>> This should be the same behavior as the old version of
>>> loadDNSConfig.
>>
>> ...
>>
>>> - if (adapterP) {
>>> - free(adapterP);
>>> + free(adapters);
>>> + } else {
>>> + th = (*env)->ExceptionOccurred(env);
>>> + if (th != NULL) {
>>> + // We only throw errors if not able to allocate memory,
>>> otherwise
>>> + // ignore ones related to Windows API usage itself.
>>> + (*env)->ExceptionClear(env);
>>> + if ((*env)->IsInstanceOf(env, th, outOfMemoryErrCl)) {
>>> + (*env)->Throw(env, th);
>>> + return STS_ERROR;
>>> + }
>>> + }
>>
>> I think that it would be preferable to NOT clear the pending
>> exception, and just allow it to propagate up.
>>
>> -Chris.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20200109/3b267378/attachment-0001.htm>
More information about the net-dev
mailing list