diff --git a/user/user.go b/user/user.go index 14b0b41..4478a41 100644 --- a/user/user.go +++ b/user/user.go @@ -56,13 +56,17 @@ type IDMap struct { Count int64 } -func parseLine(line []byte, v ...any) { - parseParts(bytes.Split(line, []byte(":")), v...) +func parseLine(line []byte, v ...any) (ok bool, _ error) { + line = bytes.TrimSpace(line) + if len(line) == 0 { + return false, nil + } + return parseParts(bytes.Split(line, []byte(":")), v...) } -func parseParts(parts [][]byte, v ...any) { +func parseParts(parts [][]byte, v ...any) (ok bool, _ error) { if len(parts) == 0 { - return + return false, nil } for i, p := range parts { @@ -78,10 +82,24 @@ func parseParts(parts [][]byte, v ...any) { case *string: *e = string(p) case *int: - // "numbers", with conversion errors ignored because of some misbehaving configuration files. - *e, _ = strconv.Atoi(string(p)) + if len(p) != 0 { + n, ok, err := parseNumeric(string(p)) + if err != nil { + return true, fmt.Errorf("parsing integer field %d: %w", i, err) + } + if !ok { + return true, fmt.Errorf("parsing integer field %d: %q is not numeric", i, p) + } + *e = n + } case *int64: - *e, _ = strconv.ParseInt(string(p), 10, 64) + if len(p) != 0 { + n, err := strconv.ParseInt(string(p), 10, 64) + if err != nil { + return true, fmt.Errorf("parsing integer field %d: %w", i, err) + } + *e = n + } case *[]string: // Comma-separated lists. if len(p) != 0 { @@ -94,6 +112,7 @@ func parseParts(parts [][]byte, v ...any) { panic(fmt.Sprintf("parseLine only accepts {*string, *int, *int64, *[]string} as arguments! %#v is not a pointer!", e)) } } + return true, nil } func ParsePasswdFile(path string) ([]User, error) { @@ -126,6 +145,7 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { for s.Scan() { line := bytes.TrimSpace(s.Bytes()) if len(line) == 0 { + // Skip empty lines, and don't consider them. continue } @@ -135,7 +155,11 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) { // root:x:0:0:root:/root:/bin/bash // adm:x:3:4:adm:/var/adm:/bin/false p := User{} - parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell) + ok, err := parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell) + if err != nil || !ok { + // Skip malformed and empty lines, and don't consider them. + continue + } if filter == nil || filter(p) { out = append(out, p) @@ -194,7 +218,11 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) { // root:x:0:root // adm:x:4:root,adm,daemon p := Group{} - parseLine(line, &p.Name, &p.Pass, &p.Gid, &p.List) + ok, err := parseLine(line, &p.Name, &p.Pass, &p.Gid, &p.List) + if err != nil || !ok { + // Skip malformed and empty lines, and don't consider them. + continue + } if filter == nil || filter(p) { out = append(out, p) @@ -508,7 +536,11 @@ func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) { // see: man 5 subuid p := SubID{} - parseLine(line, &p.Name, &p.SubID, &p.Count) + ok, err := parseLine(line, &p.Name, &p.SubID, &p.Count) + if err != nil || !ok { + // Skip malformed and empty lines, and don't consider them. + continue + } if filter == nil || filter(p) { out = append(out, p) @@ -556,7 +588,11 @@ func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) { // see: man 7 user_namespaces p := IDMap{} - parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count) + ok, err := parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count) + if err != nil || !ok { + // Skip malformed and empty lines, and don't consider them. + continue + } if filter == nil || filter(p) { out = append(out, p) diff --git a/user/user_test.go b/user/user_test.go index e5f599a..a37894f 100644 --- a/user/user_test.go +++ b/user/user_test.go @@ -21,42 +21,69 @@ func TestParseLine(t *testing.T) { d int ) - parseLine([]byte(""), &a, &b) + _, err := parseLine([]byte(""), &a, &b) + if err != nil { + t.Fatal(err) + } if a != "" || b != "" { t.Fatalf("a and b should be empty ('%v', '%v')", a, b) } - parseLine([]byte("a"), &a, &b) + _, err = parseLine([]byte("a"), &a, &b) + if err != nil { + t.Fatal(err) + } if a != "a" || b != "" { t.Fatalf("a should be 'a' and b should be empty ('%v', '%v')", a, b) } - parseLine([]byte("bad boys:corny cows"), &a, &b) + _, err = parseLine([]byte("bad boys:corny cows"), &a, &b) + if err != nil { + t.Fatal(err) + } if a != "bad boys" || b != "corny cows" { t.Fatalf("a should be 'bad boys' and b should be 'corny cows' ('%v', '%v')", a, b) } - parseLine([]byte(""), &c) + ok, err := parseLine([]byte(""), &c) + if err != nil { + t.Fatal(err) + } + if ok { + t.Fatalf("ok should be false for empty lines") + } if len(c) != 0 { t.Fatalf("c should be empty (%#v)", c) } - parseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c) + _, err = parseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c) + if err != nil { + t.Fatal(err) + } if a != "g" || b != "h" || len(c) != 3 || c[0] != "i" || c[1] != "j" || c[2] != "k" { t.Fatalf("a should be 'g', b should be 'h', and c should be ['i','j','k'] ('%v', '%v', '%#v')", a, b, c) } - parseLine([]byte("::::::::::"), &a, &b, &c) + _, err = parseLine([]byte("::::::::::"), &a, &b, &c) + if err != nil { + t.Fatal(err) + } if a != "" || b != "" || len(c) != 0 { t.Fatalf("a, b, and c should all be empty ('%v', '%v', '%#v')", a, b, c) } - parseLine([]byte("not a number"), &d) + _, err = parseLine([]byte("not a number"), &d) + if err == nil { + t.Fatal("expected an error") + } if d != 0 { t.Fatalf("d should be 0 (%v)", d) } - parseLine([]byte("b:12:c"), &a, &d, &b) + _, err = parseLine([]byte("b:12:c"), &a, &d, &b) + if err != nil { + t.Fatal(err) + } if a != "b" || b != "c" || d != 12 { t.Fatalf("a should be 'b' and b should be 'c', and d should be 12 ('%v', '%v', %v)", a, b, d) } @@ -548,12 +575,6 @@ this is just some garbage data } func TestGetAdditionalGroups(t *testing.T) { - type foo struct { - groups []string - expected []int - hasError bool - } - groupContent := ` root:x:0:root adm:x:43: @@ -564,79 +585,84 @@ adm:x:4343:root,adm-duplicate toolarge:x:2147483648: this is just some garbage data ` + largeGroup() - tests := []foo{ + tests := []struct { + doc string + groups []string + expected []int + hasError bool + }{ { - // empty group + doc: "empty group", groups: []string{}, expected: []int{}, }, { - // single group + doc: "single group", groups: []string{"adm"}, expected: []int{43}, }, { - // numeric group miss must continue checking remaining groups + doc: "numeric group miss must continue checking remaining groups", groups: []string{"10001", "adm"}, expected: []int{43, 10001}, }, { - // multiple groups + doc: "multiple groups", groups: []string{"adm", "grp"}, expected: []int{43, 1234}, }, { - // invalid group + doc: "invalid group", groups: []string{"adm", "grp", "not-exist"}, expected: nil, hasError: true, }, { - // group with numeric id + doc: "group with numeric id", groups: []string{"43"}, expected: []int{43}, }, { - // group with unknown numeric id + doc: "group with unknown numeric id", groups: []string{"adm", "10001"}, expected: []int{43, 10001}, }, { - // groups specified twice with numeric and name + doc: "groups specified twice with numeric and name", groups: []string{"adm", "43"}, expected: []int{43}, }, { - // groups with too small id + doc: "groups with negative id", groups: []string{"-1"}, expected: nil, hasError: true, }, { - // groups with too large id + doc: "groups with too large id", groups: []string{strconv.FormatInt(1<<31, 10)}, expected: nil, hasError: true, }, { - // group with very long list of users + doc: "group with very long list of users", groups: []string{"largegroup"}, expected: []int{1000}, }, { - // numeric group must not resolve as group name + doc: "maxID + 1 group must not resolve as group name", groups: []string{"2147483648"}, expected: nil, hasError: true, }, { - // numeric group must not resolve as group name + doc: "maxInt64+1 group must not resolve as group name", groups: []string{"9223372036854775808"}, expected: nil, hasError: true, }, { - // group entry with out-of-range gid + doc: "group entry with out-of-range gid", groups: []string{"toolarge"}, expected: nil, hasError: true, @@ -644,8 +670,7 @@ this is just some garbage data } for _, tc := range tests { - name := strings.Join(tc.groups, ",") - t.Run(name, func(t *testing.T) { + t.Run(tc.doc, func(t *testing.T) { group := strings.NewReader(groupContent) gids, err := GetAdditionalGroups(tc.groups, group) @@ -665,17 +690,18 @@ this is just some garbage data func TestGetAdditionalGroupsNumeric(t *testing.T) { tests := []struct { + doc string groups []string expected []int hasError bool }{ { - // numeric groups only + doc: "numeric groups only", groups: []string{"1234", "5678"}, expected: []int{1234, 5678}, }, { - // numeric and alphabetic + doc: "numeric and alphabetic", groups: []string{"1234", "fake"}, expected: nil, hasError: true, @@ -683,8 +709,7 @@ func TestGetAdditionalGroupsNumeric(t *testing.T) { } for _, tc := range tests { - name := strings.Join(tc.groups, ",") - t.Run(name, func(t *testing.T) { + t.Run(tc.doc, func(t *testing.T) { gids, err := GetAdditionalGroups(tc.groups, nil) if tc.hasError && err == nil { t.Fatalf("Parse(%#v) expects error but has none", tc)