[WIP] user: skip malformed and empty lines

user/user.go

@@ -56,32 +56,49 @@ 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, fields ...any) (ok bool, _ error) {
 	if len(parts) == 0 {
-		return
+		return false, nil
 	}
 
-	for i, p := range parts {
-		// Ignore cases where we don't have enough fields to populate the arguments.
-		// Some configuration files like to misbehave.
-		if len(v) <= i {
-			break
+	for i, v := range fields {
+		var p []byte
+		if i < len(parts) {
+			// Ignore cases where we don't have enough fields to populate the arguments.
+			// Some configuration files like to misbehave.
+			// Depending on the field-type, missing fields can be either ignored,
+			// (e.g. empty string) or produce an error (missing/empty numeric field).
+			p = parts[i]
 		}
 
 		// Use the type of the argument to figure out how to parse it, scanf() style.
 		// This is legit.
-		switch e := v[i].(type) {
+		switch e := v.(type) {
 		case *string:
 			*e = string(p)
 		case *int:
-			// "numbers", with conversion errors ignored because of some misbehaving configuration files.
-			*e, _ = strconv.Atoi(string(p))
+			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)
+			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 +111,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 +144,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 +154,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 +217,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)
@@ -501,7 +528,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)
@@ -549,7 +580,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)

user/user_test.go

@@ -18,48 +18,85 @@ func TestParseLine(t *testing.T) {
 	var (
 		a, b string
 		c    []string
-		d    int
+		d, e 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)
 	}
+
+	// partial line, missing numeric value
+	_, err = parseLine([]byte("a:b:3"), &a, &b, &d, &e)
+	expErr := `parsing integer field 3: "" is not numeric`
+	if err == nil || err.Error() != expErr {
+		t.Fatalf("expected '%s', got: %v", expErr, err)
+	}
+	if a != "a" || b != "b" || d != 3 || e != 0 {
+		t.Fatalf("expected 'a', 'b', 3, 0, got ('%v', '%v', '%v', '%v')", a, b, d, e)
+	}
 }
 
 func TestParsePasswdFilter(t *testing.T) {
@@ -71,8 +108,8 @@ this is just some garbage data
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
-	if len(users) != 3 {
-		t.Fatalf("Expected 3 users, got %v", len(users))
+	if len(users) != 2 {
+		t.Fatalf("Expected 2 users, got %v", len(users))
 	}
 	if users[0].Uid != 0 || users[0].Name != "root" {
 		t.Fatalf("Expected users[0] to be 0 - root, got %v - %v", users[0].Uid, users[0].Name)
@@ -91,8 +128,8 @@ this is just some garbage data
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
-	if len(groups) != 4 {
-		t.Fatalf("Expected 4 groups, got %v", len(groups))
+	if len(groups) != 3 {
+		t.Fatalf("Expected 3 groups, got %v", len(groups))
 	}
 	if groups[0].Gid != 0 || groups[0].Name != "root" || len(groups[0].List) != 1 {
 		t.Fatalf("Expected groups[0] to be 0 - root - 1 member, got %v - %v - %v", groups[0].Gid, groups[0].Name, len(groups[0].List))