IPv6 compatibility fix

[smagellan]

based upon #982

[marcosnils]

@smagellan can you please add a test for this?

[marcosnils]

@smagellan can you also take into consideration the changes in this PR #1099 ?

[smagellan]

@marcosnils Ok, I will introduce test.
I can't realize your comment on PR #1099. Should I include changeset into my PR? Please confirm or correct if I was wrong.

[marcosnils]

I can't realize your comment on PR #1099. Should I include changeset into my PR? Please confirm or correct if I was wrong.

@smagellan Just wanted to make sure that changes proposed in #1099 still apply with this PR as well.Maybe @allanwax can make a quick review and leave his comments here.

@smadasu an example is this change: https://github.com/xetorthio/jedis/pull/1099/files#diff-e8943893bffd99c67d703eabf84d0647R8. Seems like we should include this logic also, right?

[marcosnils]

@smagellan can u please check #1344 ?

[mp911de]

Parsing IPv6 addresses will always find a port, no matter whether the port is specified or not.

[marcosnils]

LGTM. Swall we merge this right away independently of redis current IPv6 issue?

@HeartSaVioR @xetorthio @mp911de @smadasu ?

[allanwax]

It's been a while since I looked at this and the stuff I checked into
git last year was never merged due to formatting issues. I also can't
find the code that needs to be examined. I'm not really a git person.

However, from what I code fragments I've seen in the posts, extracting
the last index of ':' is the proper way to go. In addition, because of
ipv6, I am reposting fragments of the changes that I think should also
be inserted.

public class HostAndPort {
public static final String LOCALHOST_STR ;

static {
    try {
        LOCALHOST_STR = InetAddress.getLocalHost().getAddress();
    }
    catch (Exception e) {
        LOCALHOST_STR = "localhost";
    }
}

This fragment will make sure that the value of local host is whatever
the actual machine's IPV4/V6 address is versus "localhost".

private String convertHost(String host) {
if (host.equals("127.0.0.1") || host.startsWith("localhost") ||
host.equals("0.0.0.0") || host.startsWith("169.254"))
return LOCALHOST_STR;
else if (host.startsWith("::1") ||
host.startsWith("0:0:0:0:0:0:0:1"))
return LOCALHOST_STR;

    return host;
}

This fragment does additional checks for IPV4/V6. '::1' is not the only
way to reference localhost in IPV6.

The code I checked into git has additional functionality.

If someone can send me the code in question, I can look further or the
git reference to clone from.

Allan Wax

On 7/11/2016 3:53 AM, Marcos Nils wrote:

I can't realize your comment on PR #1099
<https://github.com/xetorthio/jedis/pull/1099>. Should I include
changeset into my PR? Please confirm or correct if I was wrong.

@smagellan https://github.com/smagellan Just wanted to make sure that
changes proposed in #1099 #1099
still apply with this PR as well.Maybe @allanwax
https://github.com/allanwax can make a quick review and leave his
comments here.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1342 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AIEV_g9X0IDYxyd2lg6pZIylJYZlFmfcks5qUiC3gaJpZM4JHPtE.

[xetorthio]

@smagellan can you please squash and add a nice commit text?

Also why do you think redis IPv6 support has an issue? As far as I understand it seems like it always appends a color and the port to the host. So the parsing in this PR seems to be totally fine. Am I missing something?

[smagellan]

@xetorthio Done.
Yep, redis's answer can be handled without problems. But rfc5952 recommends another IPv6 formatting ('bracketized') approach, e.g. [2001:db8::1]:80.

[xetorthio]

Are you you pushed it after squashing? I can still see all the commits.

[smagellan]

@xetorthio Can you please check again?

[xetorthio]

I can still see 4 commits instead of 1

[smagellan]

@xetorthio I squashed it again

[xetorthio]

Now it is perfect! Thanks!

I think this is ready to be merged!

[marcosnils]

LGTM!.

[marcosnils]

Merged to master and 2.9 respectively.

Thx!.

[tonyghita]

@marcosnils @smagellan this is breaking functionality for me. getLocalhostQuietly() now uses localAddress = InetAddress.getLocalHost().getHostAddress();, which resolves 'localhost' to the host's external ip (i.e. '10.1.10.254') instead of 127.0.0.1.

This breaks connections to the Redis Sentinel (v3.2.2) on my localhost, since only loopback connections are allowed.

I'm on OS X 10.11.6 FWIW (I think InetAddress.getLocalHost() has system-dependent behavior).

Would it be possible to fix this forward to resolve loopback? Or was there some need to resolve the external ip?

Reverting to Jedis @ 2.8.2 resolves the issue I'm having.

[smagellan]

@tonyghita, can you please open separate issue for this problem?

[tonyghita]

Will do!

[tonyghita]

@smagellan I've created an issue for this at #1367