fix: Unmarshal should fail if required fields are missing

.golangci.yml

@@ -5,6 +5,7 @@ run:
   timeout: 10m
   skip-files:
     - ".*\\.pb.*\\.go"
+    - ".*_test\\.go"
 
 # output configuration options
 output:

Makefile

@@ -137,7 +137,8 @@ clean-generated:
 
 .PHONY: lint
 lint: check-golangci-lint-install
-	@golangci-lint run
+	@golangci-lint run ./...
+	@cd example && golangci-lint run ./...
 
 .PHONY: check-golangci-lint-install
 check-golangci-lint-install:

cmd/protoc-gen-fastmarshal/funcs.go

@@ -203,6 +203,7 @@ func addProtoFunctions(fm template.FuncMap, protoFile *protogen.File) template.F
 	fm["getAdditionalImports"] = getAdditionalImports(protoFile)
 	fm["getImportPrefix"] = getImportPrefix(protoFile)
 	fm["mapFieldGoType"] = mapFieldGoType(protoFile)
+	fm["hasRequiredFields"] = hasRequiredFields(protoFile)
 	return fm
 }
 
@@ -405,3 +406,46 @@ func mapFieldGoType(protoFile *protogen.File) func(*protogen.Field) string {
 		return fmt.Sprintf("map[%s]%s", ktype, vtype)
 	}
 }
+
+// msgHasRequiredField returns true if the specified message has at least 1 field marked required and
+// false if not
+func msgHasRequiredField(m *protogen.Message) bool {
+	if m.Desc.Syntax() == protoreflect.Proto3 {
+		return false
+	}
+	for _, f := range m.Fields {
+		if f.Desc.Cardinality() == protoreflect.Required {
+			return true
+		}
+	}
+	return false
+}
+
+// hasRequiredFields returns true if at least one field in the specified message is marked required
+// and false if not.
+//
+// If m is nil, this function returns true if *any* message in the Protobuf file has a required field
+// and false if not.
+func hasRequiredFields(protoFile *protogen.File) func(*protogen.Message) bool {
+	anyMessageHasRequiredFields := false
+	if protoFile.Desc.Syntax() == protoreflect.Proto2 {
+		for _, m := range allMessages(protoFile)() {
+			anyMessageHasRequiredFields = anyMessageHasRequiredFields || msgHasRequiredField(m)
+		}
+	}
+
+	return func(m *protogen.Message) bool {
+		if m == nil {
+			return anyMessageHasRequiredFields
+		}
+		if m.Desc.Syntax() == protoreflect.Proto3 {
+			return false
+		}
+		for _, f := range m.Fields {
+			if f.Desc.Cardinality() == protoreflect.Required {
+				return true
+			}
+		}
+		return false
+	}
+}

cmd/protoc-gen-fastmarshal/templates/permessage.go.tmpl

@@ -7,7 +7,8 @@
 package {{ .ProtoDesc.GoPackageName }}
 
 import (
-    "fmt"
+    "fmt"{{if and (eq $protoSyntax "proto2") (hasRequiredFields nil)}}
+    "strings"{{end}}
     "github.com/CrowdStrike/csproto"
     {{range (.Message | getAdditionalImports)}}{{.}}
     {{end}}
@@ -114,7 +115,39 @@ func (m *{{ .Message.GoIdent.GoName}}) Unmarshal(p []byte) error {
             {{- end -}}
             }
         }
+    }{{if hasRequiredFields .Message }}
+    // verify required fields are assigned
+    if err := m.csprotoCheckRequiredFields(); err != nil {
+        return err
+    }
+    {{end}}
+    return nil
+}
+{{if hasRequiredFields .Message}}
+
+// csprotoCheckRequiredFields is called by Unmarshal() to ensure that all required fields have been
+// populated.
+func (m *{{.Message.GoIdent.GoName}}) csprotoCheckRequiredFields() error {
+    var missingFields []string
+{{ range .Message.Fields }}
+{{ if and (eq (.Desc.Cardinality | string) "required") (eq (.Desc.Syntax | string) "proto2") -}}
+    if m.{{.GoName}} == nil {
+        missingFields = append(missingFields, "{{.GoName}}")
+    }
+{{- end -}}
+{{ end }}
+    if len(missingFields) > 0 {
+        var sb strings.Builder
+        sb.WriteString("cannot unmarshal, one or more required fields missing: ")
+        for i, s := range missingFields {
+            if i > 0 {
+                sb.WriteRune(',')
+            }
+            sb.WriteString(s)
+        }
+        return fmt.Errorf(sb.String())
     }
     return nil
 }
+{{ end }}
 {{end}}

cmd/protoc-gen-fastmarshal/templates/singlefile.go.tmpl

@@ -7,7 +7,8 @@
 package {{ .ProtoDesc.GoPackageName }}
 
 import (
-    "fmt"
+    "fmt"{{if and (eq $protoSyntax "proto2") (hasRequiredFields nil)}}
+    "strings"{{end}}
     "github.com/CrowdStrike/csproto"
     {{range (allMessages | getAdditionalImports)}}{{.}}
     {{end}}
@@ -117,8 +118,40 @@ func (m *{{ .GoIdent.GoName}}) Unmarshal(p []byte) error {
             {{- end -}}
             }
         }
+    }{{if hasRequiredFields . }}
+    // verify required fields are assigned
+    if err := m.csprotoCheckRequiredFields(); err != nil {
+        return err
+    }
+    {{end}}
+    return nil
+}
+{{if hasRequiredFields .}}
+
+// csprotoCheckRequiredFields is called by Unmarshal() to ensure that all required fields have been
+// populated.
+func (m *{{.GoIdent.GoName}}) csprotoCheckRequiredFields() error {
+    var missingFields []string
+{{ range .Fields }}
+{{ if and (eq (.Desc.Cardinality | string) "required") (eq (.Desc.Syntax | string) "proto2") -}}
+    if m.{{.GoName}} == nil {
+        missingFields = append(missingFields, "{{.GoName}}")
+    }
+{{- end -}}
+{{ end }}
+    if len(missingFields) > 0 {
+        var sb strings.Builder
+        sb.WriteString("cannot unmarshal, one or more required fields missing: ")
+        for i, s := range missingFields {
+            if i > 0 {
+                sb.WriteRune(',')
+            }
+            sb.WriteString(s)
+        }
+        return fmt.Errorf(sb.String())
     }
     return nil
 }
 {{ end }}
+{{ end }}
 {{end}}

example/proto2/gogo/gogo_proto2_example.pb.fm.go

@@ -5,6 +5,7 @@ package gogo
 
 import (
 	"fmt"
+	"strings"
 	"github.com/CrowdStrike/csproto"
 )
 
@@ -172,6 +173,42 @@ func (m *BaseEvent) Unmarshal(p []byte) error {
 			}
 		}
 	}
+	// verify required fields are assigned
+	if err := m.csprotoCheckRequiredFields(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// csprotoCheckRequiredFields is called by Unmarshal() to ensure that all required fields have been
+// populated.
+func (m *BaseEvent) csprotoCheckRequiredFields() error {
+	var missingFields []string
+
+	if m.EventID == nil {
+		missingFields = append(missingFields, "EventID")
+	}
+	if m.SourceID == nil {
+		missingFields = append(missingFields, "SourceID")
+	}
+	if m.Timestamp == nil {
+		missingFields = append(missingFields, "Timestamp")
+	}
+	if m.EventType == nil {
+		missingFields = append(missingFields, "EventType")
+	}
+	if len(missingFields) > 0 {
+		var sb strings.Builder
+		sb.WriteString("cannot unmarshal, one or more required fields missing: ")
+		for i, s := range missingFields {
+			if i > 0 {
+				sb.WriteRune(',')
+			}
+			sb.WriteString(s)
+		}
+		return fmt.Errorf(sb.String())
+	}
 	return nil
 }
 
@@ -413,6 +450,34 @@ func (m *TestEvent) Unmarshal(p []byte) error {
 			}
 		}
 	}
+	// verify required fields are assigned
+	if err := m.csprotoCheckRequiredFields(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// csprotoCheckRequiredFields is called by Unmarshal() to ensure that all required fields have been
+// populated.
+func (m *TestEvent) csprotoCheckRequiredFields() error {
+	var missingFields []string
+
+	if m.Embedded == nil {
+		missingFields = append(missingFields, "Embedded")
+	}
+
+	if len(missingFields) > 0 {
+		var sb strings.Builder
+		sb.WriteString("cannot unmarshal, one or more required fields missing: ")
+		for i, s := range missingFields {
+			if i > 0 {
+				sb.WriteRune(',')
+			}
+			sb.WriteString(s)
+		}
+		return fmt.Errorf(sb.String())
+	}
 	return nil
 }
 
@@ -557,6 +622,34 @@ func (m *EmbeddedEvent) Unmarshal(p []byte) error {
 			}
 		}
 	}
+	// verify required fields are assigned
+	if err := m.csprotoCheckRequiredFields(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// csprotoCheckRequiredFields is called by Unmarshal() to ensure that all required fields have been
+// populated.
+func (m *EmbeddedEvent) csprotoCheckRequiredFields() error {
+	var missingFields []string
+
+	if m.ID == nil {
+		missingFields = append(missingFields, "ID")
+	}
+
+	if len(missingFields) > 0 {
+		var sb strings.Builder
+		sb.WriteString("cannot unmarshal, one or more required fields missing: ")
+		for i, s := range missingFields {
+			if i > 0 {
+				sb.WriteRune(',')
+			}
+			sb.WriteString(s)
+		}
+		return fmt.Errorf(sb.String())
+	}
 	return nil
 }
 
@@ -904,6 +997,34 @@ func (m *AllTheThings) Unmarshal(p []byte) error {
 			}
 		}
 	}
+	// verify required fields are assigned
+	if err := m.csprotoCheckRequiredFields(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// csprotoCheckRequiredFields is called by Unmarshal() to ensure that all required fields have been
+// populated.
+func (m *AllTheThings) csprotoCheckRequiredFields() error {
+	var missingFields []string
+
+	if m.ID == nil {
+		missingFields = append(missingFields, "ID")
+	}
+
+	if len(missingFields) > 0 {
+		var sb strings.Builder
+		sb.WriteString("cannot unmarshal, one or more required fields missing: ")
+		for i, s := range missingFields {
+			if i > 0 {
+				sb.WriteRune(',')
+			}
+			sb.WriteString(s)
+		}
+		return fmt.Errorf(sb.String())
+	}
 	return nil
 }
 
@@ -1333,6 +1454,34 @@ func (m *RepeatAllTheThings) Unmarshal(p []byte) error {
 			}
 		}
 	}
+	// verify required fields are assigned
+	if err := m.csprotoCheckRequiredFields(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// csprotoCheckRequiredFields is called by Unmarshal() to ensure that all required fields have been
+// populated.
+func (m *RepeatAllTheThings) csprotoCheckRequiredFields() error {
+	var missingFields []string
+
+	if m.ID == nil {
+		missingFields = append(missingFields, "ID")
+	}
+
+	if len(missingFields) > 0 {
+		var sb strings.Builder
+		sb.WriteString("cannot unmarshal, one or more required fields missing: ")
+		for i, s := range missingFields {
+			if i > 0 {
+				sb.WriteRune(',')
+			}
+			sb.WriteString(s)
+		}
+		return fmt.Errorf(sb.String())
+	}
 	return nil
 }