From 49ba07d1d64af7413e16dde43ec2934b7e25b824 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Mon, 24 Oct 2016 15:13:16 +0530 Subject: [PATCH] Use net.ParseCIDR instead of custom-built parsers (#3055) Removes avoidable conversion to and from net.IP to string. --- cmd/host-to-ip.go | 51 ----------------------- cmd/host-to-ip_test.go | 85 --------------------------------------- cmd/interface-ips.go | 69 +++++++++++++++++++++++++++++++ cmd/interface-ips_test.go | 31 ++++++++++++++ cmd/server-main.go | 19 +++------ 5 files changed, 105 insertions(+), 150 deletions(-) delete mode 100644 cmd/host-to-ip.go delete mode 100644 cmd/host-to-ip_test.go create mode 100644 cmd/interface-ips.go create mode 100644 cmd/interface-ips_test.go diff --git a/cmd/host-to-ip.go b/cmd/host-to-ip.go deleted file mode 100644 index 36686b9c3..000000000 --- a/cmd/host-to-ip.go +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Minio Cloud Storage, (C) 2016 Minio, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package cmd - -import ( - "fmt" - "net" - "sort" -) - -// byLastOctet implements sort.Interface used in sorting a list -// of ip address by their last octet value. -type byLastOctet []net.IP - -func (n byLastOctet) Len() int { return len(n) } -func (n byLastOctet) Swap(i, j int) { n[i], n[j] = n[j], n[i] } -func (n byLastOctet) Less(i, j int) bool { - return []byte(n[i].To4())[3] < []byte(n[j].To4())[3] -} - -// sortIPsByOctet - returns a reverse sorted list of hosts based on the last octet value. -func sortIPsByOctet(ips []string) error { - var nips []net.IP - for _, ip := range ips { - nip := net.ParseIP(ip) - if nip == nil { - return fmt.Errorf("Unable to parse invalid ip %s", ip) - } - nips = append(nips, nip) - } - // Reverse sort ips by their last octet. - sort.Sort(sort.Reverse(byLastOctet(nips))) - for i, nip := range nips { - ips[i] = nip.String() - } - return nil -} diff --git a/cmd/host-to-ip_test.go b/cmd/host-to-ip_test.go deleted file mode 100644 index 85e75b4f2..000000000 --- a/cmd/host-to-ip_test.go +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Minio Cloud Storage, (C) 2016 Minio, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package cmd - -import ( - "fmt" - "testing" -) - -// Tests sorted list generated from hosts to ip. -func TestHostToIP(t *testing.T) { - // Collection of test cases to validate last octet sorting. - testCases := []struct { - ips []string - sortedIPs []string - err error - shouldPass bool - }{ - { - // List of ip addresses that need to be sorted. - ips: []string{ - "129.95.30.40", - "5.24.69.2", - "19.20.203.5", - "1.2.3.4", - "127.0.0.1", - "19.20.21.22", - "5.220.100.50", - }, - // Numerical sorting result based on the last octet. - sortedIPs: []string{ - "5.220.100.50", - "129.95.30.40", - "19.20.21.22", - "19.20.203.5", - "1.2.3.4", - "5.24.69.2", - "127.0.0.1", - }, - err: nil, - shouldPass: true, - }, - { - ips: []string{ - "localhost", - }, - sortedIPs: []string{}, - err: fmt.Errorf("Unable to parse invalid ip localhost"), - shouldPass: false, - }, - } - - // Tests the correct sorting behavior of getIPsFromHosts. - for j, testCase := range testCases { - err := sortIPsByOctet(testCase.ips) - if !testCase.shouldPass && testCase.err.Error() != err.Error() { - t.Fatalf("Test %d: Expected error %s, got %s", j+1, testCase.err, err) - } - if testCase.shouldPass && err != nil { - t.Fatalf("Test %d: Expected error %s", j+1, err) - } - if testCase.shouldPass { - for i, ip := range testCase.ips { - if ip == testCase.sortedIPs[i] { - continue - } - t.Errorf("Test %d expected to pass but failed. Wanted ip %s, but got %s", j+1, testCase.sortedIPs[i], ip) - } - } - } -} diff --git a/cmd/interface-ips.go b/cmd/interface-ips.go new file mode 100644 index 000000000..991328a92 --- /dev/null +++ b/cmd/interface-ips.go @@ -0,0 +1,69 @@ +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "fmt" + "net" + "sort" +) + +// byLastOctetValue implements sort.Interface used in sorting a list +// of ip address by their last octet value. +type byLastOctetValue []net.IP + +func (n byLastOctetValue) Len() int { return len(n) } +func (n byLastOctetValue) Swap(i, j int) { n[i], n[j] = n[j], n[i] } +func (n byLastOctetValue) Less(i, j int) bool { + return []byte(n[i].To4())[3] < []byte(n[j].To4())[3] +} + +// getInterfaceIPv4s is synonymous to net.InterfaceAddrs() +// returns net.IP IPv4 only representation of the net.Addr. +// Additionally the returned list is sorted by their last +// octet value. +// +// [The logic to sort by last octet is implemented to +// prefer CIDRs with higher octects, this in-turn skips the +// localhost/loopback address to be not preferred as the +// first ip on the list. Subsequently this list helps us print +// a user friendly message with appropriate values]. +func getInterfaceIPv4s() ([]net.IP, error) { + addrs, err := net.InterfaceAddrs() + if err != nil { + return nil, fmt.Errorf("Unable to determine network interface address. %s", err) + } + // Go through each return network address and collate IPv4 addresses. + var nips []net.IP + for _, addr := range addrs { + if addr.Network() == "ip+net" { + var nip net.IP + // Attempt to parse the addr through CIDR. + nip, _, err = net.ParseCIDR(addr.String()) + if err != nil { + return nil, fmt.Errorf("Unable to parse addrss %s, error %s", addr, err) + } + // Collect only IPv4 addrs. + if nip.To4() != nil { + nips = append(nips, nip) + } + } + } + // Sort the list of IPs by their last octet value. + sort.Sort(sort.Reverse(byLastOctetValue(nips))) + return nips, nil +} diff --git a/cmd/interface-ips_test.go b/cmd/interface-ips_test.go new file mode 100644 index 000000000..be87dbd85 --- /dev/null +++ b/cmd/interface-ips_test.go @@ -0,0 +1,31 @@ +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import "testing" + +func TestGetInterfaceIPv4s(t *testing.T) { + ipv4s, err := getInterfaceIPv4s() + if err != nil { + t.Fatalf("Unexpected error %s", err) + } + for _, ip := range ipv4s { + if ip.To4() == nil { + t.Fatalf("Unexpected expecting only IPv4 addresses only %s", ip) + } + } +} diff --git a/cmd/server-main.go b/cmd/server-main.go index 7a37390b0..75a7046b6 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -222,23 +222,14 @@ func getListenIPs(httpServerConf *http.Server) (hosts []string, port string, err return nil, port, fmt.Errorf("Unable to parse host address %s", err) } if host == "" { - var addrs []net.Addr - addrs, err = net.InterfaceAddrs() - if err != nil { - return nil, port, fmt.Errorf("Unable to determine network interface address. %s", err) - } - for _, addr := range addrs { - if addr.Network() == "ip+net" { - hostname := strings.Split(addr.String(), "/")[0] - if ip := net.ParseIP(hostname); ip.To4() != nil { - hosts = append(hosts, hostname) - } - } - } - err = sortIPsByOctet(hosts) + var ipv4s []net.IP + ipv4s, err = getInterfaceIPv4s() if err != nil { return nil, port, fmt.Errorf("Unable reverse sorted ips from hosts %s", err) } + for _, ip := range ipv4s { + hosts = append(hosts, ip.String()) + } return hosts, port, nil } // if host != "" { // Proceed to append itself, since user requested a specific endpoint.