Skip to content

Fixed illegal reflective access by not relying on a sun.net.dns class. (#8318)#8319

Merged
normanmaurer merged 2 commits intonetty:4.1from
mattayres:4.1
Sep 26, 2018
Merged

Fixed illegal reflective access by not relying on a sun.net.dns class. (#8318)#8319
normanmaurer merged 2 commits intonetty:4.1from
mattayres:4.1

Conversation

@mattayres
Copy link
Copy Markdown
Contributor

Motivation

Applications should not depend on internal packages with Java 9 and later. This cause a warning now, but will break in future versions of Java.

Modification

This change adds methods to UnixResolverDnsServerAddressStreamProvider (following after #6844) that parse /etc/resolv.conf for domain and search entries. Then DnsNameResolver does not need to rely on sun.net.dns.ResolverConfiguration to do this.

Result

Fixes #8318. Furthermore, at least in my testing with Java 11, this also makes multiple search entries work properly (previously I was only getting the first entry).

@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@normanmaurer
Copy link
Copy Markdown
Member

@netty-bot test this please

Copy link
Copy Markdown
Member

@normanmaurer normanmaurer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question / suggestion. Otherwise LGTM


@SuppressWarnings("unchecked")
List<String> list = (List<String>) nameservers.invoke(instance);
List<String> list = UnixResolverDnsServerAddressStreamProvider.parseEtcResolverSearchDomains();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattayres What about non unix like systems (for example windows) ? Maybe we can use PlatformDependent.isWindows() and only use this if it returns false ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@normanmaurer That's a good point. I've made the change you suggested.

@normanmaurer
Copy link
Copy Markdown
Member

@netty-bot test this please

@normanmaurer
Copy link
Copy Markdown
Member

@mattayres can you sign our ICLA and ping me once done:

https://netty.io/s/icla

@mattayres
Copy link
Copy Markdown
Contributor Author

@normanmaurer signed ICLA.

@normanmaurer normanmaurer merged commit ba594bc into netty:4.1 Sep 26, 2018
@normanmaurer
Copy link
Copy Markdown
Member

@mattayres thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants