Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid unsafe split when validate hostname which might be ipv6 address #5713

Merged
merged 3 commits into from
Dec 20, 2019

Conversation

fan1emon
Copy link
Contributor

Motivation

Pulsar doesn't support deploy on IPv6 network environment.
This PR just makes an effort to make it.
The error happens if the client connect to brokers by IPv6 address, like fec0:0:0:ffff::1.

  • Wrong format: fec0:0:0:ffff::1:6650
  • Correct format: [fec0:0:0:ffff::1]:6650

Cause the split regex is ':', brackets are needed and the ip:port can't split by ':' directly.

Modifications

validateHostName in ServiceURI

Change-Id: I21c6f161270afc1d279c52d98ddf956abcf64741
Change-Id: I7cc94cca8d6c3f3981c74795013e2ce24a029b76
@fan1emon
Copy link
Contributor Author

The failed tests seem unrelated.

@jiazhai
Copy link
Member

jiazhai commented Nov 23, 2019

run cpp tests
run java8 tests

1 similar comment
@fan1emon
Copy link
Contributor Author

run cpp tests
run java8 tests

@fan1emon
Copy link
Contributor Author

run cpp tests

@@ -122,6 +122,18 @@ public void testUserInfo() {
"/path/to/namespace");
}

@Test
public void testIpv6Uri() {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test case of using ipv6 address without port?

Change-Id: I8b127438886c9c414746a0dae90fc3a5ea50f5c7
@sijie sijie added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Nov 27, 2019
@sijie
Copy link
Member

sijie commented Nov 30, 2019

run integration tests

1 similar comment
@sijie
Copy link
Member

sijie commented Dec 8, 2019

run integration tests

@fan1emon
Copy link
Contributor Author

run integration tests

@sijie sijie added this to the 2.5.0 milestone Dec 20, 2019
@sijie sijie merged commit 880fb56 into apache:master Dec 20, 2019
@sijie sijie mentioned this pull request Jan 14, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
…apache#5713)

### Motivation
Pulsar doesn't support deploy on IPv6 network environment.
This PR just makes an effort to make it.
The error happens if the client connect to brokers by IPv6 address, like fec0:0:0:ffff::1.
- Wrong format: fec0:0:0:ffff::1:6650
- Correct format: [fec0:0:0:ffff::1]:6650

Cause the split regex is ':', brackets are needed and the ip:port can't split by ':' directly.

### Modifications

validateHostName in ServiceURI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants