Skip to content

Commit 9448830

Browse files
user: parseLine, parseParts: add error-return
Return an error when failing to parse numeric values instead of silently discarding them. Co-authored-by: Mickael Emirkanian <mickael.emirkanian@docker.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent 55196b1 commit 9448830

2 files changed

Lines changed: 66 additions & 19 deletions

File tree

user/user.go

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ type IDMap struct {
5656
Count int64
5757
}
5858

59-
func parseLine(line []byte, v ...any) {
60-
parseParts(bytes.Split(line, []byte(":")), v...)
59+
func parseLine(line []byte, v ...any) error {
60+
return parseParts(bytes.Split(line, []byte(":")), v...)
6161
}
6262

63-
func parseParts(parts [][]byte, v ...any) {
63+
func parseParts(parts [][]byte, v ...any) error {
6464
if len(parts) == 0 {
65-
return
65+
return nil
6666
}
6767

6868
for i, p := range parts {
@@ -78,10 +78,24 @@ func parseParts(parts [][]byte, v ...any) {
7878
case *string:
7979
*e = string(p)
8080
case *int:
81-
// "numbers", with conversion errors ignored because of some misbehaving configuration files.
82-
*e, _ = strconv.Atoi(string(p))
81+
if len(p) != 0 {
82+
n, ok, err := parseNumeric(string(p))
83+
if err != nil {
84+
return fmt.Errorf("parsing integer field %d: %w", i, err)
85+
}
86+
if !ok {
87+
return fmt.Errorf("parsing integer field %d: %q is not numeric", i, p)
88+
}
89+
*e = n
90+
}
8391
case *int64:
84-
*e, _ = strconv.ParseInt(string(p), 10, 64)
92+
if len(p) != 0 {
93+
n, err := strconv.ParseInt(string(p), 10, 64)
94+
if err != nil {
95+
return fmt.Errorf("parsing integer field %d: %w", i, err)
96+
}
97+
*e = n
98+
}
8599
case *[]string:
86100
// Comma-separated lists.
87101
if len(p) != 0 {
@@ -94,6 +108,7 @@ func parseParts(parts [][]byte, v ...any) {
94108
panic(fmt.Sprintf("parseLine only accepts {*string, *int, *int64, *[]string} as arguments! %#v is not a pointer!", e))
95109
}
96110
}
111+
return nil
97112
}
98113

99114
func ParsePasswdFile(path string) ([]User, error) {
@@ -135,7 +150,9 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) {
135150
// root:x:0:0:root:/root:/bin/bash
136151
// adm:x:3:4:adm:/var/adm:/bin/false
137152
p := User{}
138-
parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell)
153+
if err := parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell); err != nil {
154+
return nil, err
155+
}
139156

140157
if filter == nil || filter(p) {
141158
out = append(out, p)
@@ -194,7 +211,9 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
194211
// root:x:0:root
195212
// adm:x:4:root,adm,daemon
196213
p := Group{}
197-
parseLine(line, &p.Name, &p.Pass, &p.Gid, &p.List)
214+
if err := parseLine(line, &p.Name, &p.Pass, &p.Gid, &p.List); err != nil {
215+
return nil, err
216+
}
198217

199218
if filter == nil || filter(p) {
200219
out = append(out, p)
@@ -539,7 +558,9 @@ func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) {
539558

540559
// see: man 5 subuid
541560
p := SubID{}
542-
parseLine(line, &p.Name, &p.SubID, &p.Count)
561+
if err := parseLine(line, &p.Name, &p.SubID, &p.Count); err != nil {
562+
return nil, err
563+
}
543564

544565
if filter == nil || filter(p) {
545566
out = append(out, p)
@@ -587,7 +608,9 @@ func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) {
587608

588609
// see: man 7 user_namespaces
589610
p := IDMap{}
590-
parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count)
611+
if err := parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count); err != nil {
612+
return nil, err
613+
}
591614

592615
if filter == nil || filter(p) {
593616
out = append(out, p)

user/user_test.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,42 +21,66 @@ func TestParseLine(t *testing.T) {
2121
d int
2222
)
2323

24-
parseLine([]byte(""), &a, &b)
24+
err := parseLine([]byte(""), &a, &b)
25+
if err != nil {
26+
t.Fatal(err)
27+
}
2528
if a != "" || b != "" {
2629
t.Fatalf("a and b should be empty ('%v', '%v')", a, b)
2730
}
2831

29-
parseLine([]byte("a"), &a, &b)
32+
err = parseLine([]byte("a"), &a, &b)
33+
if err != nil {
34+
t.Fatal(err)
35+
}
3036
if a != "a" || b != "" {
3137
t.Fatalf("a should be 'a' and b should be empty ('%v', '%v')", a, b)
3238
}
3339

34-
parseLine([]byte("bad boys:corny cows"), &a, &b)
40+
err = parseLine([]byte("bad boys:corny cows"), &a, &b)
41+
if err != nil {
42+
t.Fatal(err)
43+
}
3544
if a != "bad boys" || b != "corny cows" {
3645
t.Fatalf("a should be 'bad boys' and b should be 'corny cows' ('%v', '%v')", a, b)
3746
}
3847

39-
parseLine([]byte(""), &c)
48+
err = parseLine([]byte(""), &c)
49+
if err != nil {
50+
t.Fatal(err)
51+
}
4052
if len(c) != 0 {
4153
t.Fatalf("c should be empty (%#v)", c)
4254
}
4355

44-
parseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c)
56+
err = parseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c)
57+
if err != nil {
58+
t.Fatal(err)
59+
}
4560
if a != "g" || b != "h" || len(c) != 3 || c[0] != "i" || c[1] != "j" || c[2] != "k" {
4661
t.Fatalf("a should be 'g', b should be 'h', and c should be ['i','j','k'] ('%v', '%v', '%#v')", a, b, c)
4762
}
4863

49-
parseLine([]byte("::::::::::"), &a, &b, &c)
64+
err = parseLine([]byte("::::::::::"), &a, &b, &c)
65+
if err != nil {
66+
t.Fatal(err)
67+
}
5068
if a != "" || b != "" || len(c) != 0 {
5169
t.Fatalf("a, b, and c should all be empty ('%v', '%v', '%#v')", a, b, c)
5270
}
5371

54-
parseLine([]byte("not a number"), &d)
72+
err = parseLine([]byte("not a number"), &d)
73+
if err == nil {
74+
t.Fatal("expected an error")
75+
}
5576
if d != 0 {
5677
t.Fatalf("d should be 0 (%v)", d)
5778
}
5879

59-
parseLine([]byte("b:12:c"), &a, &d, &b)
80+
err = parseLine([]byte("b:12:c"), &a, &d, &b)
81+
if err != nil {
82+
t.Fatal(err)
83+
}
6084
if a != "b" || b != "c" || d != 12 {
6185
t.Fatalf("a should be 'b' and b should be 'c', and d should be 12 ('%v', '%v', %v)", a, b, d)
6286
}

0 commit comments

Comments
 (0)