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

common: Improve IsHex and IsHexAddress & add tests #15551

Merged
merged 3 commits into from
Dec 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions common/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
// Package common contains various helper functions.
package common

import (
"encoding/hex"
)
import "encoding/hex"

func ToHex(b []byte) string {
hex := Bytes2Hex(b)
Expand Down Expand Up @@ -55,14 +53,24 @@ func CopyBytes(b []byte) (copiedBytes []byte) {
return
}

func HasHexPrefix(str string) bool {
l := len(str)
return l >= 2 && str[0:2] == "0x"
func hasHexPrefix(str string) bool {
return len(str) >= 2 && str[0] == '0' && (str[1] == 'x' || str[1] == 'X')
}

func IsHex(str string) bool {
l := len(str)
return l >= 4 && l%2 == 0 && str[0:2] == "0x"
func isHexCharacter(c byte) bool {
return ('0' <= c && c <= '9') || ('a' <= c && c <= 'f') || ('A' <= c && c <= 'F')
}

func isHex(str string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the semantics of IsHex/isHex. Previously, it expected a 0x-prefixed string, now it expects no prefix. That's fine, though, since it's internal we know it's not used anywhere where it will break anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

The only use is in IsHexAddress, which already checks for the prefix ;)

if len(str)%2 != 0 {
return false
}
for _, c := range []byte(str) {
if !isHexCharacter(c) {
return false
}
}
return true
}

func Bytes2Hex(d []byte) string {
Expand Down
34 changes: 21 additions & 13 deletions common/bytes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,6 @@ func (s *BytesSuite) TestCopyBytes(c *checker.C) {
c.Assert(res1, checker.DeepEquals, exp1)
}

func (s *BytesSuite) TestIsHex(c *checker.C) {
data1 := "a9e67e"
exp1 := false
res1 := IsHex(data1)
c.Assert(res1, checker.DeepEquals, exp1)

data2 := "0xa9e67e00"
exp2 := true
res2 := IsHex(data2)
c.Assert(res2, checker.DeepEquals, exp2)

}

func (s *BytesSuite) TestLeftPadBytes(c *checker.C) {
val1 := []byte{1, 2, 3, 4}
exp1 := []byte{0, 0, 0, 0, 1, 2, 3, 4}
Expand Down Expand Up @@ -78,6 +65,27 @@ func TestFromHex(t *testing.T) {
}
}

func TestIsHex(t *testing.T) {
tests := []struct {
input string
ok bool
}{
{"", true},
{"0", false},
{"00", true},
{"a9e67e", true},
{"A9E67E", true},
{"0xa9e67e", false},
{"a9e67e001", false},
{"0xHELLO_MY_NAME_IS_STEVEN_@#$^&*", false},
}
for _, test := range tests {
if ok := isHex(test.input); ok != test.ok {
t.Errorf("isHex(%q) = %v, want %v", test.input, ok, test.ok)
}
}
}

func TestFromHexOddLength(t *testing.T) {
input := "0x1"
expected := []byte{1}
Expand Down
9 changes: 3 additions & 6 deletions common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,10 @@ func HexToAddress(s string) Address { return BytesToAddress(FromHex(s)) }
// IsHexAddress verifies whether a string can represent a valid hex-encoded
// Ethereum address or not.
func IsHexAddress(s string) bool {
if len(s) == 2+2*AddressLength && IsHex(s) {
return true
if hasHexPrefix(s) {
s = s[2:]
}
if len(s) == 2*AddressLength && IsHex("0x"+s) {
return true
}
return false
return len(s) == 2*AddressLength && isHex(s)
}

// Get the string representation of the underlying address
Expand Down
24 changes: 24 additions & 0 deletions common/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,30 @@ func TestBytesConversion(t *testing.T) {
}
}

func TestIsHexAddress(t *testing.T) {
tests := []struct {
str string
exp bool
}{
{"0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed", true},
{"5aaeb6053f3e94c9b9a09f33669435e7ef1beaed", true},
{"0X5aaeb6053f3e94c9b9a09f33669435e7ef1beaed", true},
{"0XAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", true},
{"0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", true},
{"0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed1", false},
{"0x5aaeb6053f3e94c9b9a09f33669435e7ef1beae", false},
{"5aaeb6053f3e94c9b9a09f33669435e7ef1beaed11", false},
{"0xxaaeb6053f3e94c9b9a09f33669435e7ef1beaed", false},
}

for _, test := range tests {
if result := IsHexAddress(test.str); result != test.exp {
t.Errorf("IsHexAddress(%s) == %v; expected %v",
test.str, result, test.exp)
}
}
}

func TestHashJsonValidation(t *testing.T) {
var tests = []struct {
Prefix string
Expand Down