[PATCH] 7006496: Use modern Windows API to retrieve OS DNS servers
Anuraag Agrawal
anuraaga at gmail.com
Wed Jan 8 05:12:49 UTC 2020
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.
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..d8ccf27031 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,13 @@
static jfieldID searchlistID;
static jfieldID nameserversID;
+static jclass outOfMemoryErrCl;
+
+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 +62,33 @@ 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;
+ jthrowable th;
/*
* First see if there is a global suffix list specified.
@@ -112,120 +106,75 @@ 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);
- /*
- * 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);
- strappend(ns, result);
+ ret = WSAAddressToStringA(sockAddr,
+ dnsServer->Address.iSockaddrLength, NULL,
+ result, &dwLen);
+ if (ret == 0) {
+ strappend(ns, result);
+ }
+
+ dnsServer = dnsServer->Next;
}
- /*
- * Finished with this registry key
- */
- RegCloseKey(hKey);
+ // 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);
+ }
+ }
}
- /*
- * Onto the next adapeter
- */
- curr = curr->Next;
+ adapter = adapter->Next;
}
- }
- /*
- * Free the adpater structure
- */
- 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;
+ }
+ }
}
return STS_SL_FOUND & STS_NS_FOUND;
@@ -233,11 +182,16 @@ static int loadConfig(char *sl, char *ns) {
/*
- * Initialize JNI field IDs.
+ * Initialize JNI field IDs and classes.
*/
JNIEXPORT void JNICALL
Java_sun_net_dns_ResolverConfigurationImpl_init0(JNIEnv *env, jclass cls)
{
+ jclass cl = (*env)->FindClass(env, "java/lang/OutOfMemoryError");
+ if (cl != NULL) {
+ outOfMemoryErrCl = (*env)->NewGlobalRef(env, cl);
+ }
+
searchlistID = (*env)->GetStaticFieldID(env, cls, "os_searchlist",
"Ljava/lang/String;");
CHECK_NULL(searchlistID);
@@ -260,7 +214,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 +226,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 2, 2020 at 11:21 PM Chris Hegarty <chris.hegarty at oracle.com>
wrote:
>
>
> On 27 Dec 2019, at 08:44, Anuraag Agrawal <anuraaga at gmail.com> wrote:
>
> ...
> On Tue, Dec 24, 2019 at 11:57 PM Chris Hegarty <chris.hegarty at oracle.com>
> wrote:
>
>>
>> ...
>> I think that this mainly looks good. A few small comments:
>>
>> 1) If getAdapters returns an error ( ret != ERROR_SUCCESS ), then there
>> will be a JNI pending exception, right? so loadDNSConfig0 should not
>> unconditionally throw OOME.
>>
>
> Thanks for noticing this. Previously, any failure to malloc would return
> STS_ERROR and loadDNSConfig0 would throw OOME, while any other type of
> error (e.g., getAdaptersInfo returning an error) is ignored. In the current
> patch, failure to malloc sets OOME, otherwise a generic Error, and then the
> code tries to throw OOME which would probably cause a crash due to throwing
> two exceptions in a row without clearing (it's hard to repro the error case
> so not entirely sure).
>
> To restore the old behavior, I would have to check the result of
> getAdapters and only throw if the exception is OOME, and ignore exceptions
> of type Error which are generated here
>
>
> https://github.com/openjdk/jdk/blob/master/src/java.base/windows/native/libnet/NetworkInterface_winXP.c#L114
>
>
> Alternatively if leaving the exception of getAdapters as is and just
> remove the current JNU_ThrowOutOfMemoryError in loadDNSConfig0, we will
> throw Error when a random failure of GetAdapterAddresses happens. This is
> probably an incompatible change and not acceptable but just want to confirm.
>
> Also it would be possible to skip getAdapters exceptions that are not OOME
> in C or in Java. It would be great to hear what the recommended pattern
> would be.
>
>
> One approach would be to only throw if there is no pending exception, e.g.
>
> 211 } else if (!(*env)->ExceptionOccurred(env)) {
> 212 JNU_ThrowOutOfMemoryError(env, "native memory allocation
> failed");
> 213 }
>
> But looking at this again, I think that the OOM is not needed at all with
> the new code, since loadDNSConfig is not itself doing a malloc ( any more )
> - and this catch-all style is great anyway.
>
> ...
>
>>
>> 3) I wonder if we should bump MAX_STR_LEN, while here?
>>
>
> Seems like a good idea - I don't know what a good value for this is but
> noticed another location uses 1K so have raised it to that in my local copy.
>
>
> That seems fine to me.
>
> -Chris.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20200108/87dd80a4/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: windows-dns.patch
Type: application/octet-stream
Size: 18403 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20200108/87dd80a4/windows-dns-0001.patch>
More information about the net-dev
mailing list