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

pkg/types: fix unwanted unescape in NewURLsMap #3635

Merged
merged 1 commit into from
Oct 5, 2015

Conversation

yichengq
Copy link
Contributor

@yichengq yichengq commented Oct 3, 2015

We use url.ParseQuery to parse names-to-urls string, but it has side
effect that unescape the string. If the initial-cluster string has ipv6
which contains %25, it will unescape it to % and make further url
parse failed.

Fix it by writing the parse process ourselves.

@xiang90
Copy link
Contributor

xiang90 commented Oct 4, 2015

Test failed.

}
for name, urls := range v {
if len(urls) == 0 || urls[0] == "" {
urlsm := make(map[string][]string)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we simply copy https://golang.org/src/net/url/url.go?s=18971:19022#L680 and remove the unnecessary parts?

@yichengq
Copy link
Contributor Author

yichengq commented Oct 5, 2015

fix tests, and reference url.Parse to implement the function. PTAL.

// See the License for the specific language governing permissions and
// limitations under the License.

// +build go1.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Let us refer the original bug report in golang itself. Tell people why we only test this after go 1.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added.

@xiang90
Copy link
Contributor

xiang90 commented Oct 5, 2015

LGTM

We use url.ParseQuery to parse names-to-urls string, but it has side
effect that unescape the string. If the initial-cluster string has ipv6
which contains `%25`, it will unescape it to `%` and make further url
parse failed.

Fix it by modifiying the parse process.

Go1.4 doesn't support literal IPv6 address w/ zone in
URI(golang/go#6530), so we only enable tests
in Go1.5+.
yichengq added a commit that referenced this pull request Oct 5, 2015
pkg/types: fix unwanted unescape in NewURLsMap
@yichengq yichengq merged commit 522ee6a into etcd-io:master Oct 5, 2015
@yichengq yichengq deleted the parse-ipv6 branch October 5, 2015 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants