Skip to content

Commit 4d8cbdc

Browse files
cpuschmacpuschma
committed
add error handling for NewPostalAddress and enforce non-empty addresses
1 parent 069fdb4 commit 4d8cbdc

File tree

2 files changed

+61
-53
lines changed

2 files changed

+61
-53
lines changed

‎v3/postaladdress.go‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package ldap
22

33
import (
4+
"errors"
45
"fmt"
56
"strings"
67
)
78

9+
var ErrEmptyPostalAddress = errors.New("ldap: postal address cannot be empty")
10+
811
// PostalAddress represents an RFC 4517 Postal Address
912
// A postal address is a sequence of strings of one or more arbitrary UCS
1013
// characters, which form the lines of the address.
@@ -13,15 +16,20 @@ type PostalAddress struct {
1316
}
1417

1518
// NewPostalAddress creates a new PostalAddress by copying non-empty lines from the provided slice of strings.
16-
func NewPostalAddress(lines []string) *PostalAddress {
19+
func NewPostalAddress(lines []string) (*PostalAddress, error) {
1720
copiedLines := make([]string, 0, len(lines))
1821
for _, line := range lines {
1922
if line == "" {
2023
continue
2124
}
2225
copiedLines = append(copiedLines, line)
2326
}
24-
return &PostalAddress{lines: copiedLines}
27+
28+
if len(copiedLines) == 0 {
29+
return nil, ErrEmptyPostalAddress
30+
}
31+
32+
return &PostalAddress{lines: copiedLines}, nil
2533
}
2634

2735
// Lines returns a copy of the address lines as a slice of strings.
@@ -61,11 +69,8 @@ func (p *PostalAddress) Escape() string {
6169
// ParsePostalAddress parses an RFC 4517 escaped postal address string into a PostalAddress object or returns an error.
6270
func ParsePostalAddress(escaped string) (*PostalAddress, error) {
6371
lines := strings.Split(escaped, "$")
64-
var parsedLines []string
65-
const (
66-
escapeLen = 2
67-
totalEscapeLen = 3
68-
)
72+
parsedLines := make([]string, 0, len(lines))
73+
const totalEscapeLen = 3
6974

7075
for _, line := range lines {
7176
if line == "" {
@@ -97,10 +102,15 @@ func ParsePostalAddress(escaped string) (*PostalAddress, error) {
97102
parsedLines = append(parsedLines, builder.String())
98103
}
99104

105+
if len(parsedLines) == 0 {
106+
return nil, ErrEmptyPostalAddress
107+
}
108+
100109
return &PostalAddress{lines: parsedLines}, nil
101110
}
102111

103-
func (p *PostalAddress) Equals(other *PostalAddress) bool {
112+
// Equal compares the current PostalAddress with another PostalAddress and returns true if they are identical.
113+
func (p *PostalAddress) Equal(other *PostalAddress) bool {
104114
if p == other {
105115
return true
106116
}

‎v3/postaladdress_test.go‎

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,18 @@ func TestPostalAddressRoundTrip(t *testing.T) {
3434
assert.NoError(t, err)
3535
assert.Equal(t, str.Expected, escaped.String())
3636

37-
addr := NewPostalAddress([]string{str.Expected})
37+
addr, err := NewPostalAddress([]string{str.Expected})
38+
assert.NoError(t, err)
3839
assert.Equal(t, str.Expected, addr.String(), "PostalAddress.String() should round-trip")
3940
})
4041
}
4142
}
4243

44+
func TestPostalAddressEmptyLines(t *testing.T) {
45+
_, err := NewPostalAddress([]string{""})
46+
assert.Equal(t, err, ErrEmptyPostalAddress)
47+
}
48+
4349
func TestPostalAddressUTF8Handling(t *testing.T) {
4450
testCases := []struct {
4551
name string
@@ -80,7 +86,8 @@ func TestPostalAddressUTF8Handling(t *testing.T) {
8086

8187
for _, tc := range testCases {
8288
t.Run(tc.name, func(t *testing.T) {
83-
addr := NewPostalAddress(tc.lines)
89+
addr, err := NewPostalAddress(tc.lines)
90+
assert.NoError(t, err)
8491
escaped := addr.Escape()
8592
assert.Equal(t, tc.expected, escaped, "UTF-8 characters should be preserved in escaped output")
8693

@@ -92,7 +99,7 @@ func TestPostalAddressUTF8Handling(t *testing.T) {
9299
}
93100
}
94101

95-
func TestPostalAddressEquals(t *testing.T) {
102+
func TestPostalAddressEqual(t *testing.T) {
96103
testCases := []struct {
97104
name string
98105
addr1 *PostalAddress
@@ -108,103 +115,85 @@ func TestPostalAddressEquals(t *testing.T) {
108115
{
109116
name: "first nil",
110117
addr1: nil,
111-
addr2: NewPostalAddress([]string{"line1"}),
118+
addr2: mustNewPostalAddress(t, []string{"line1"}),
112119
expected: false,
113120
},
114121
{
115122
name: "second nil",
116-
addr1: NewPostalAddress([]string{"line1"}),
123+
addr1: mustNewPostalAddress(t, []string{"line1"}),
117124
addr2: nil,
118125
expected: false,
119126
},
120-
{
121-
name: "both empty",
122-
addr1: NewPostalAddress([]string{}),
123-
addr2: NewPostalAddress([]string{}),
124-
expected: true,
125-
},
126127
{
127128
name: "same single line",
128-
addr1: NewPostalAddress([]string{"123 Main St"}),
129-
addr2: NewPostalAddress([]string{"123 Main St"}),
129+
addr1: mustNewPostalAddress(t, []string{"123 Main St"}),
130+
addr2: mustNewPostalAddress(t, []string{"123 Main St"}),
130131
expected: true,
131132
},
132133
{
133134
name: "different single line",
134-
addr1: NewPostalAddress([]string{"123 Main St"}),
135-
addr2: NewPostalAddress([]string{"456 Oak Ave"}),
135+
addr1: mustNewPostalAddress(t, []string{"123 Main St"}),
136+
addr2: mustNewPostalAddress(t, []string{"456 Oak Ave"}),
136137
expected: false,
137138
},
138139
{
139140
name: "same multi-line",
140-
addr1: NewPostalAddress([]string{"123 Main St", "Anytown, CA", "USA"}),
141-
addr2: NewPostalAddress([]string{"123 Main St", "Anytown, CA", "USA"}),
141+
addr1: mustNewPostalAddress(t, []string{"123 Main St", "Anytown, CA", "USA"}),
142+
addr2: mustNewPostalAddress(t, []string{"123 Main St", "Anytown, CA", "USA"}),
142143
expected: true,
143144
},
144145
{
145146
name: "different multi-line content",
146-
addr1: NewPostalAddress([]string{"123 Main St", "Anytown, CA", "USA"}),
147-
addr2: NewPostalAddress([]string{"123 Main St", "Othertown, CA", "USA"}),
147+
addr1: mustNewPostalAddress(t, []string{"123 Main St", "Anytown, CA", "USA"}),
148+
addr2: mustNewPostalAddress(t, []string{"123 Main St", "Othertown, CA", "USA"}),
148149
expected: false,
149150
},
150151
{
151152
name: "different line count",
152-
addr1: NewPostalAddress([]string{"123 Main St", "Anytown, CA"}),
153-
addr2: NewPostalAddress([]string{"123 Main St", "Anytown, CA", "USA"}),
153+
addr1: mustNewPostalAddress(t, []string{"123 Main St", "Anytown, CA"}),
154+
addr2: mustNewPostalAddress(t, []string{"123 Main St", "Anytown, CA", "USA"}),
154155
expected: false,
155156
},
156157
{
157158
name: "same order matters",
158-
addr1: NewPostalAddress([]string{"line1", "line2"}),
159-
addr2: NewPostalAddress([]string{"line2", "line1"}),
159+
addr1: mustNewPostalAddress(t, []string{"line1", "line2"}),
160+
addr2: mustNewPostalAddress(t, []string{"line2", "line1"}),
160161
expected: false,
161162
},
162163
{
163164
name: "whitespace differences",
164-
addr1: NewPostalAddress([]string{"123 Main St"}),
165-
addr2: NewPostalAddress([]string{"123 Main St"}),
165+
addr1: mustNewPostalAddress(t, []string{"123 Main St"}),
166+
addr2: mustNewPostalAddress(t, []string{"123 Main St"}),
166167
expected: false,
167168
},
168169
{
169170
name: "case sensitive",
170-
addr1: NewPostalAddress([]string{"Main Street"}),
171-
addr2: NewPostalAddress([]string{"main street"}),
171+
addr1: mustNewPostalAddress(t, []string{"Main Street"}),
172+
addr2: mustNewPostalAddress(t, []string{"main street"}),
172173
expected: false,
173174
},
174175
{
175176
name: "with special characters",
176-
addr1: NewPostalAddress([]string{"Café René", "$1000\\month"}),
177-
addr2: NewPostalAddress([]string{"Café René", "$1000\\month"}),
177+
addr1: mustNewPostalAddress(t, []string{"Café René", "$1000\\month"}),
178+
addr2: mustNewPostalAddress(t, []string{"Café René", "$1000\\month"}),
178179
expected: true,
179180
},
180181
{
181182
name: "with UTF-8 characters",
182-
addr1: NewPostalAddress([]string{"北京市东城区", "中国 🇨🇳"}),
183-
addr2: NewPostalAddress([]string{"北京市东城区", "中国 🇨🇳"}),
184-
expected: true,
185-
},
186-
{
187-
name: "empty vs nil lines",
188-
addr1: NewPostalAddress([]string{}),
189-
addr2: NewPostalAddress([]string{""}),
190-
expected: true,
191-
},
192-
{
193-
name: "same empty string line",
194-
addr1: NewPostalAddress([]string{""}),
195-
addr2: NewPostalAddress([]string{""}),
183+
addr1: mustNewPostalAddress(t, []string{"北京市东城区", "中国 🇨🇳"}),
184+
addr2: mustNewPostalAddress(t, []string{"北京市东城区", "中国 🇨🇳"}),
196185
expected: true,
197186
},
198187
}
199188

200189
for _, tc := range testCases {
201190
t.Run(tc.name, func(t *testing.T) {
202-
result := tc.addr1.Equals(tc.addr2)
191+
result := tc.addr1.Equal(tc.addr2)
203192
assert.Equal(t, tc.expected, result)
204193

205194
// Test symmetry (except for nil cases where calling on nil would panic)
206195
if tc.addr1 != nil && tc.addr2 != nil {
207-
reverseResult := tc.addr2.Equals(tc.addr1)
196+
reverseResult := tc.addr2.Equal(tc.addr1)
208197
assert.Equal(t, tc.expected, reverseResult, "Equals should be symmetric")
209198
}
210199
})
@@ -222,3 +211,12 @@ func TestParsePostalAddress_Escape(t *testing.T) {
222211
assert.Error(t, err)
223212
})
224213
}
214+
215+
func mustNewPostalAddress(t *testing.T, lines []string) *PostalAddress {
216+
t.Helper()
217+
addr, err := NewPostalAddress(lines)
218+
if err != nil {
219+
t.Fatalf("NewPostalAddress failed: %v", err)
220+
}
221+
return addr
222+
}

0 commit comments

Comments
 (0)