Add DefaultHostsFileEntriesResolver#addresses to provide all hosts file's entries for a hostname by violetagg · Pull Request #11246 · netty/netty

violetagg · 2021-05-11T18:25:27Z

Motivation:

DefaultHostsFileEntriesResolver should provide all hosts file's entries for a hostname when
DnsNameResolver#resolveAll as opposed to the current implementation where only the first
entry is taken into consideration.

Modification:

Add DefaultHostsFileEntriesResolver#addresses to provide all hosts file's entries for a hostname
Add HostsFileEntriesProvider to provide all hosts file's entries for a hostname and to keep
backwards compatibility for HostsFileEntries and HostsFileParser
DnsNameResolver#resolveAll uses the new DefaultHostsFileEntriesResolver#addresses
BlockHound configuration: replace HostsFileParser#parse with HostsFileEntriesProvider$ParserImpl#parse
as the latter does the parsing
Add junit tests

Result:
Fixes #10834

…le's entries for a hostname Motivation: DefaultHostsFileEntriesResolver should provide all hosts file's entries for a hostname when DnsNameResolver#resolveAll as opposed to the current implementation where only the first entry is taken into consideration Modification: - Add DefaultHostsFileEntriesResolver#addresses to provide all hosts file's entries for a hostname - Add HostsFileEntriesProvider to provide all hosts file's entries for a hostname and to keep backwards compatibility for HostsFileEntries and HostsFileParser - DnsNameResolver#resolveAll uses the new DefaultHostsFileEntriesResolver#addresses - BlockHound configuration: replace HostsFileParser#parse with HostsFileEntriesProvider$ParserImpl#parse as the latter does the parsing - Add junit tests Result: Fixes #10834

violetagg · 2021-05-12T07:39:39Z

@normanmaurer How can I find the test that is not passing the build on CI?

normanmaurer · 2021-05-12T07:51:19Z

@violetagg it failed due some network problems:

2021-05-11T18:51:11.3115666Z [INFO] ------------------------------------------------------------------------
2021-05-11T18:51:11.3116421Z [INFO] BUILD FAILURE
2021-05-11T18:51:11.3117366Z [INFO] ------------------------------------------------------------------------
2021-05-11T18:51:11.3118128Z [INFO] Total time:  20:49 min
2021-05-11T18:51:11.3118926Z [INFO] Finished at: 2021-05-11T18:51:11Z
2021-05-11T18:51:11.3119884Z [INFO] ------------------------------------------------------------------------
2021-05-11T18:51:11.3125549Z [ERROR] Failed to execute goal on project netty-codec-dns: Could not resolve dependencies for project io.netty:netty-codec-dns:jar:4.1.64.Final-SNAPSHOT: The following artifacts could not be resolved: org.apache.directory.server:apacheds-core:jar:1.5.7, org.apache.directory.server:apacheds-core-api:jar:1.5.7: Could not transfer artifact org.apache.directory.server:apacheds-core:jar:1.5.7 from/to central (https://repo.maven.apache.org/maven2): transfer failed for https://repo.maven.apache.org/maven2/org/apache/directory/server/apacheds-core/1.5.7/apacheds-core-1.5.7.jar: Connection timed out (Read failed) -> [Help 1]
2021-05-11T18:51:11.3129407Z [ERROR] 
2021-05-11T18:51:11.3130408Z [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
2021-05-11T18:51:11.3131587Z [ERROR] Re-run Maven using the -X switch to enable full debug logging.
2021-05-11T18:51:11.3132323Z [ERROR] 
2021-05-11T18:51:11.3133240Z [ERROR] For more information about the errors and possible solutions, please read the following articles:
2021-05-11T18:51:11.3134751Z [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException
2021-05-11T18:51:11.3135857Z [ERROR] 
2021-05-11T18:51:11.3136650Z [ERROR] After correcting the problems, you can resume the build with the command
2021-05-11T18:51:11.3137718Z [ERROR]   mvn <args> -rf :netty-codec-dns
2021-05-11T18:51:11.4706176Z 1

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java

resolver/src/main/java/io/netty/resolver/DefaultHostsFileEntriesResolver.java

resolver/src/main/java/io/netty/resolver/HostsFileEntriesProvider.java

normanmaurer

one nit... then I think we are ready to go

resolver/src/main/java/io/netty/resolver/DefaultHostsFileEntriesResolver.java

normanmaurer · 2021-05-12T16:09:42Z

/cc @NiteshKant can you check as well ?

normanmaurer

I think this is fine but one question left @violetagg that I am not sure about...

resolver/src/main/java/io/netty/resolver/HostsFileEntriesProvider.java

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java

violetagg · 2021-05-14T07:59:31Z

@normanmaurer @NiteshKant Thanks a lot!

…le's entries for a hostname (#11246) Motivation: DefaultHostsFileEntriesResolver should provide all hosts file's entries for a hostname when DnsNameResolver#resolveAll as opposed to the current implementation where only the first entry is taken into consideration Modification: - Add DefaultHostsFileEntriesResolver#addresses to provide all hosts file's entries for a hostname - Add HostsFileEntriesProvider to provide all hosts file's entries for a hostname and to keep backwards compatibility for HostsFileEntries and HostsFileParser - DnsNameResolver#resolveAll uses the new DefaultHostsFileEntriesResolver#addresses - BlockHound configuration: replace HostsFileParser#parse with HostsFileEntriesProvider$ParserImpl#parse as the latter does the parsing - Add junit tests Result: Fixes #10834

normanmaurer · 2021-05-14T10:51:07Z

@violetagg I need to investigate... with this change I see a test-failure in servicetalk... It seems to return the IPV6 address for localhost when we expect the IPV4. I need to investigate if the problem is with the test or the change here that it may return the IPV6 as first when we want the IPV4.

violetagg · 2021-05-14T10:57:04Z

@violetagg I need to investigate... with this change I see a test-failure in servicetalk... It seems to return the IPV6 address for localhost when we expect the IPV4. I need to investigate if the problem is with the test or the change here that it may return the IPV6 as first when we want the IPV4.

if I can help just tell me ...

normanmaurer · 2021-05-14T12:47:16Z

@violetagg all good... I debugged a bit with the team and we are ok from a netty side..

violetagg · 2021-05-14T12:53:32Z

@violetagg all good... I debugged a bit with the team and we are ok from a netty side..

That's good. Thanks

…le's entries for a hostname (netty#11246) Motivation: DefaultHostsFileEntriesResolver should provide all hosts file's entries for a hostname when DnsNameResolver#resolveAll as opposed to the current implementation where only the first entry is taken into consideration Modification: - Add DefaultHostsFileEntriesResolver#addresses to provide all hosts file's entries for a hostname - Add HostsFileEntriesProvider to provide all hosts file's entries for a hostname and to keep backwards compatibility for HostsFileEntries and HostsFileParser - DnsNameResolver#resolveAll uses the new DefaultHostsFileEntriesResolver#addresses - BlockHound configuration: replace HostsFileParser#parse with HostsFileEntriesProvider$ParserImpl#parse as the latter does the parsing - Add junit tests Result: Fixes netty#10834 * Address feedback

violetagg mentioned this pull request May 11, 2021

AnnotatedConnectException: Connection refused: localhost/127.0.1.1:37655 #10834

Closed

normanmaurer requested changes May 12, 2021

View reviewed changes

Address feedback

dd438dc

normanmaurer requested changes May 12, 2021

View reviewed changes

resolver/src/main/java/io/netty/resolver/HostsFileEntriesProvider.java Outdated Show resolved Hide resolved

resolver/src/main/java/io/netty/resolver/HostsFileEntriesProvider.java Show resolved Hide resolved

Address feedback

89f045f

normanmaurer requested changes May 12, 2021

View reviewed changes

resolver/src/main/java/io/netty/resolver/DefaultHostsFileEntriesResolver.java Outdated Show resolved Hide resolved

Update javadoc

07146e8

normanmaurer requested a review from NiteshKant May 12, 2021 16:09

normanmaurer requested changes May 12, 2021

View reviewed changes

resolver/src/main/java/io/netty/resolver/HostsFileEntriesProvider.java Show resolved Hide resolved

normanmaurer added this to the 4.1.64.Final milestone May 12, 2021

normanmaurer approved these changes May 12, 2021

View reviewed changes

NiteshKant approved these changes May 12, 2021

View reviewed changes

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java Outdated Show resolved Hide resolved

Address feedback

8c271dc

NiteshKant approved these changes May 12, 2021

View reviewed changes

normanmaurer merged commit fa8f7a3 into netty:4.1 May 14, 2021

violetagg deleted the hosts-file-entries branch May 14, 2021 12:55

Uh oh!

Conversation

violetagg commented May 11, 2021

Uh oh!

violetagg commented May 12, 2021

Uh oh!

normanmaurer commented May 12, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

normanmaurer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

normanmaurer commented May 12, 2021

Uh oh!

normanmaurer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

violetagg commented May 14, 2021

Uh oh!

normanmaurer commented May 14, 2021

Uh oh!

violetagg commented May 14, 2021

Uh oh!

normanmaurer commented May 14, 2021

Uh oh!

violetagg commented May 14, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants