Do not accidentally leak errors from phonenumbers #4447

ref DEV-1528
authgear · Jul 12, 2024 · 8036abf · 8036abf
2 parents 4091c71 + f5d3118
commit 8036abf
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 4 deletions.
diff --git a/pkg/util/phone/legal_and_valid_parser.go b/pkg/util/phone/legal_and_valid_parser.go
@@ -18,6 +18,7 @@ func (p *legalAndValidParser) ParseInputPhoneNumber(phone string) (e164 string,
 	}
 	num, err := phonenumbers.Parse(phone, "")
 	if err != nil {
+		err = ErrNotInE164Format
 		return
 	}
 	isPhoneValid := phonenumbers.IsValidNumber(num)

diff --git a/pkg/util/phone/legal_and_valid_parser_test.go b/pkg/util/phone/legal_and_valid_parser_test.go
@@ -25,11 +25,20 @@ func TestLegalAndValidParser(t *testing.T) {
 			tooShort := "+85222"
 			So(parser.CheckE164(tooShort), ShouldBeError, "invalid phone number")
 
-			// Hong Kong number starting with 7
-			So(parser.CheckE164("+85270123456"), ShouldBeNil)
+			plus := "+"
+			So(parser.CheckE164(plus), ShouldBeError, "not in E.164 format")
+
+			plusCountryCode := "+852"
+			So(parser.CheckE164(plusCountryCode), ShouldBeError, "not in E.164 format")
 
 			nonsense := "a"
-			So(parser.CheckE164(nonsense), ShouldNotBeNil)
+			So(parser.CheckE164(nonsense), ShouldBeError, "not in E.164 format")
+
+			empty := ""
+			So(parser.CheckE164(empty), ShouldBeError, "not in E.164 format")
+
+			// Hong Kong number starting with 7
+			So(parser.CheckE164("+85270123456"), ShouldBeNil)
 		})
 
 		Convey("ParseCountryCallingCodeAndNationalNumber", func() {

diff --git a/pkg/util/phone/legal_parser.go b/pkg/util/phone/legal_parser.go
@@ -18,6 +18,7 @@ func (p *legalParser) ParseInputPhoneNumber(phone string) (e164 string, err erro
 	}
 	num, err := phonenumbers.Parse(phone, "")
 	if err != nil {
+		err = ErrNotInE164Format
 		return
 	}
 	e164 = phonenumbers.Format(num, phonenumbers.E164)

diff --git a/pkg/util/phone/legal_parser_test.go b/pkg/util/phone/legal_parser_test.go
@@ -25,8 +25,17 @@ func TestLegalParser(t *testing.T) {
 			tooShort := "+85222"
 			So(parser.CheckE164(tooShort), ShouldBeNil)
 
+			plus := "+"
+			So(parser.CheckE164(plus), ShouldBeError, "not in E.164 format")
+
+			plusCountryCode := "+852"
+			So(parser.CheckE164(plusCountryCode), ShouldBeError, "not in E.164 format")
+
 			nonsense := "a"
-			So(parser.CheckE164(nonsense), ShouldNotBeNil)
+			So(parser.CheckE164(nonsense), ShouldBeError, "not in E.164 format")
+
+			empty := ""
+			So(parser.CheckE164(empty), ShouldBeError, "not in E.164 format")
 		})
 	})
 }