From d0bcd3cd68e33c5b66a49cfe4412e38409c32522 Mon Sep 17 00:00:00 2001
From: Calvin Lee <calvins.lee@utah.edu>
Date: Wed, 27 Jul 2022 08:31:24 -0700
Subject: [PATCH] Add Logging for plist parse errors (#54)

---
 http/mdm/mdm.go | 22 ++++++++++++++++++++--
 mdm/checkin.go  |  3 +++
 mdm/command.go  |  4 ++--
 mdm/mdm.go      | 17 +++++++++++++++++
 4 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/http/mdm/mdm.go b/http/mdm/mdm.go
index 0325280..0002db7 100644
--- a/http/mdm/mdm.go
+++ b/http/mdm/mdm.go
@@ -2,6 +2,7 @@ package mdm
 
 import (
 	"errors"
+	"fmt"
 	"net/http"
 	"strings"
 
@@ -37,12 +38,21 @@ func CheckinHandler(svc service.Checkin, logger log.Logger) http.HandlerFunc {
 		}
 		respBytes, err := service.CheckinRequest(svc, mdmReqFromHTTPReq(r), bodyBytes)
 		if err != nil {
-			logger.Info("msg", "check-in request", "err", err)
+			logs := []interface{}{"msg", "check-in request"}
 			httpStatus := http.StatusInternalServerError
 			var statusErr *service.HTTPStatusError
 			if errors.As(err, &statusErr) {
 				httpStatus = statusErr.Status
+				err = fmt.Errorf("HTTP error: %w", statusErr.Unwrap())
 			}
+			// manualy unwrapping the `StatusErr` is not necessary as `errors.As` manually unwraps
+			var parseErr *mdm.ParseError
+			if errors.As(err, &parseErr) {
+				logs = append(logs, "content", string(parseErr.Content))
+				err = fmt.Errorf("parse error: %w", parseErr.Unwrap())
+			}
+			logs = append(logs, "http_status", httpStatus, "err", err)
+			logger.Info(logs...)
 			http.Error(w, http.StatusText(httpStatus), httpStatus)
 		}
 		w.Write(respBytes)
@@ -61,12 +71,20 @@ func CommandAndReportResultsHandler(svc service.CommandAndReportResults, logger
 		}
 		respBytes, err := service.CommandAndReportResultsRequest(svc, mdmReqFromHTTPReq(r), bodyBytes)
 		if err != nil {
-			logger.Info("msg", "command report results", "err", err)
+			logs := []interface{}{"msg", "command report results"}
 			httpStatus := http.StatusInternalServerError
 			var statusErr *service.HTTPStatusError
 			if errors.As(err, &statusErr) {
 				httpStatus = statusErr.Status
+				err = fmt.Errorf("HTTP error: %w", statusErr.Unwrap())
+			}
+			var parseErr *mdm.ParseError
+			if errors.As(err, &parseErr) {
+				logs = append(logs, "content", string(parseErr.Content))
+				err = fmt.Errorf("parse error: %w", parseErr.Unwrap())
 			}
+			logs = append(logs, "http_status", httpStatus, "err", err)
+			logger.Info(logs...)
 			http.Error(w, http.StatusText(httpStatus), httpStatus)
 		}
 		w.Write(respBytes)
diff --git a/mdm/checkin.go b/mdm/checkin.go
index 425756f..db9a38a 100644
--- a/mdm/checkin.go
+++ b/mdm/checkin.go
@@ -149,6 +149,9 @@ func (w *checkinUnmarshaller) UnmarshalPlist(f func(interface{}) error) error {
 func DecodeCheckin(rawMessage []byte) (message interface{}, err error) {
 	w := &checkinUnmarshaller{raw: rawMessage}
 	err = plist.Unmarshal(rawMessage, w)
+	if err != nil {
+		err = &ParseError{Err: err, Content: rawMessage}
+	}
 	message = w.message
 	return
 }
diff --git a/mdm/command.go b/mdm/command.go
index f7c1ea1..3be9998 100644
--- a/mdm/command.go
+++ b/mdm/command.go
@@ -35,7 +35,7 @@ func DecodeCommandResults(rawResults []byte) (results *CommandResults, err error
 	results = new(CommandResults)
 	err = plist.Unmarshal(rawResults, results)
 	if err != nil {
-		return
+		return nil, &ParseError{Err: err, Content: rawResults}
 	}
 	results.Raw = rawResults
 	if results.Status == "" {
@@ -58,7 +58,7 @@ func DecodeCommand(rawCommand []byte) (command *Command, err error) {
 	command = new(Command)
 	err = plist.Unmarshal(rawCommand, command)
 	if err != nil {
-		return
+		return nil, &ParseError{Err: err, Content: rawCommand}
 	}
 	command.Raw = rawCommand
 	if command.CommandUUID == "" || command.Command.RequestType == "" {
diff --git a/mdm/mdm.go b/mdm/mdm.go
index e3ec762..8f1a68c 100644
--- a/mdm/mdm.go
+++ b/mdm/mdm.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"crypto/x509"
 	"errors"
+	"fmt"
 )
 
 // Enrollment represents the various enrollment-related data sent with requests.
@@ -59,3 +60,19 @@ func (r *Request) Clone() *Request {
 	*r2 = *r
 	return r2
 }
+
+// ParseError represents a failure to parse an MDM structure (usually Apple Plist)
+type ParseError struct {
+	Err     error
+	Content []byte
+}
+
+// Unwrap returns the underlying error of the ParseError
+func (e *ParseError) Unwrap() error {
+	return e.Err
+}
+
+// Error formats the ParseError as a string
+func (e *ParseError) Error() string {
+	return fmt.Sprintf("parse error: %v: raw content: %v", e.Err, string(e.Content))
+}