Merge pull request #842 from sakhisheikh/fix/xctattachment-null-userinfo

fix(nskeyedarchiver): tolerate XCUITest objects with absent fields

Author: iSevenDays

ios/nskeyedarchiver/objectivec_classes.go

@@ -327,16 +327,31 @@ type XCTAttachment struct {
 	userInfo              map[string]interface{}
 }
 
+// resolveRef follows a keyed archive reference, returning nil when the key is
+// absent, is not a reference, or points outside the object table. Callers
+// comma-ok assert the result, so an omitted field yields a zero value.
+func resolveRef(object map[string]interface{}, key string, objects []interface{}) interface{} {
+	uid, isRef := object[key].(plist.UID)
+	if !isRef || int(uid) >= len(objects) {
+		return nil
+	}
+	return objects[uid]
+}
+
 func NewXCTAttachment(object map[string]interface{}, objects []interface{}) interface{} {
-	lifetime := object["lifetime"].(uint64)
-	uniformTypeIdentifier := objects[object["uniformTypeIdentifier"].(plist.UID)].(string)
-	fileNameOverride := objects[object["fileNameOverride"].(plist.UID)].(string)
-	timestamp := objects[object["timestamp"].(plist.UID)].(float64)
-	name := objects[object["name"].(plist.UID)].(string)
-	userInfo, _ := extractDictionary(objects[object["userInfo"].(plist.UID)].(map[string]interface{}), objects, 0)
+	lifetime, _ := object["lifetime"].(uint64)
+	uniformTypeIdentifier, _ := resolveRef(object, "uniformTypeIdentifier", objects).(string)
+	fileNameOverride, _ := resolveRef(object, "fileNameOverride", objects).(string)
+	timestamp, _ := resolveRef(object, "timestamp", objects).(float64)
+	name, _ := resolveRef(object, "name", objects).(string)
+
+	// An absent userInfo arrives as the string "$null", not a dictionary.
+	var userInfo map[string]interface{}
+	if raw, isDict := resolveRef(object, "userInfo", objects).(map[string]interface{}); isDict {
+		userInfo, _ = extractDictionary(raw, objects, 0)
+	}
 
-	payloadRaw := objects[object["payload"].(plist.UID)]
-	payload := extractAttachmentPayload(payloadRaw, objects)
+	payload := extractAttachmentPayload(resolveRef(object, "payload", objects), objects)
 
 	return XCTAttachment{
 		lifetime:              lifetime,
@@ -354,7 +369,7 @@ func extractAttachmentPayload(payloadRaw interface{}, objects []interface{}) []u
 	if !byteSliceOk {
 		mapPayload, mapOk := payloadRaw.(map[string]interface{})
 		if mapOk {
-			payloadClassMap, classOk := objects[mapPayload["$class"].(plist.UID)].(map[string]interface{})
+			payloadClassMap, classOk := resolveRef(mapPayload, "$class", objects).(map[string]interface{})
 			if classOk {
 				payloadClass := payloadClassMap["$classname"]
 				if payloadClass == "NSMutableData" || payloadClass == "NSData" {
@@ -772,14 +787,14 @@ type XCTIssue struct {
 }
 
 func NewXCTIssue(object map[string]interface{}, objects []interface{}) interface{} {
-	runtimeIssueSeverity := object["runtimeIssueSeverity"].(uint64)
-	detailedDescriptionRef := object["detailed-description"].(plist.UID)
-	sourceCodeContextRef := object["source-code-context"].(plist.UID)
-	compactDescriptionRef := object["compact-description"].(plist.UID)
+	runtimeIssueSeverity, _ := object["runtimeIssueSeverity"].(uint64)
+	detailedDescription, _ := resolveRef(object, "detailed-description", objects).(string)
+	compactDescription, _ := resolveRef(object, "compact-description", objects).(string)
 
-	detailedDescription := objects[detailedDescriptionRef].(string)
-	compactDescription := objects[compactDescriptionRef].(string)
-	sourceCodeContext := NewXCTSourceCodeContext(objects[sourceCodeContextRef].(map[string]interface{}), objects).(XCTSourceCodeContext)
+	var sourceCodeContext XCTSourceCodeContext
+	if raw, isDict := resolveRef(object, "source-code-context", objects).(map[string]interface{}); isDict {
+		sourceCodeContext, _ = NewXCTSourceCodeContext(raw, objects).(XCTSourceCodeContext)
+	}
 
 	return XCTIssue{RuntimeIssueSeverity: runtimeIssueSeverity, DetailedDescription: detailedDescription, CompactDescription: compactDescription, SourceCodeContext: sourceCodeContext}
 }
@@ -798,8 +813,10 @@ type XCTSourceCodeContext struct {
 }
 
 func NewXCTSourceCodeContext(object map[string]interface{}, objects []interface{}) interface{} {
-	locationRef := object["location"].(plist.UID)
-	location := NewXCTSourceCodeLocation(objects[locationRef].(map[string]interface{}), objects).(XCTSourceCodeLocation)
+	var location XCTSourceCodeLocation
+	if raw, isDict := resolveRef(object, "location", objects).(map[string]interface{}); isDict {
+		location, _ = NewXCTSourceCodeLocation(raw, objects).(XCTSourceCodeLocation)
+	}
 
 	return XCTSourceCodeContext{Location: location}
 }
@@ -810,13 +827,13 @@ type XCTSourceCodeLocation struct {
 }
 
 func NewXCTSourceCodeLocation(object map[string]interface{}, objects []interface{}) interface{} {
-	fileUrlRef := object["file-url"].(plist.UID)
-	relativeRef := objects[fileUrlRef].(map[string]interface{})["NS.relative"].(plist.UID)
-	relativePath := objects[int(relativeRef)].(string)
-	fileUrl := NewNSURL(relativePath)
-	lineNumber := object["line-number"].(uint64)
+	var relativePath string
+	if fileURL, isDict := resolveRef(object, "file-url", objects).(map[string]interface{}); isDict {
+		relativePath, _ = resolveRef(fileURL, "NS.relative", objects).(string)
+	}
+	lineNumber, _ := object["line-number"].(uint64)
 
-	return XCTSourceCodeLocation{FileUrl: fileUrl, LineNumber: lineNumber}
+	return XCTSourceCodeLocation{FileUrl: NewNSURL(relativePath), LineNumber: lineNumber}
 }
 
 func toInterfaceSliceOfTests(testSlice []XCTTestIdentifier) []interface{} {

ios/nskeyedarchiver/xctattachment_test.go

@@ -0,0 +1,136 @@
+package nskeyedarchiver
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"howett.net/plist"
+)
+
+// Attachments with no userInfo encode it as the string "$null". Asserting a
+// dictionary there panicked, and Unarchive turned that into an error that failed
+// the whole XCUITest run.
+func TestNewXCTAttachmentWithNullUserInfo(t *testing.T) {
+	objects := []interface{}{
+		"$null",              // 0
+		"public.plain-text",  // 1 uniformTypeIdentifier
+		"override.txt",       // 2 fileNameOverride
+		float64(770474977.9), // 3 timestamp
+		"Screenshot",         // 4 name
+		[]uint8{0x61, 0x62},  // 5 payload
+	}
+	object := map[string]interface{}{
+		"lifetime":              uint64(1),
+		"uniformTypeIdentifier": plist.UID(1),
+		"fileNameOverride":      plist.UID(2),
+		"timestamp":             plist.UID(3),
+		"name":                  plist.UID(4),
+		"payload":               plist.UID(5),
+		"userInfo":              plist.UID(0), // points at "$null"
+	}
+
+	var decoded interface{}
+	require.NotPanics(t, func() { decoded = NewXCTAttachment(object, objects) })
+
+	attachment, ok := decoded.(XCTAttachment)
+	require.True(t, ok)
+	assert.Equal(t, "public.plain-text", attachment.UniformTypeIdentifier)
+	assert.Equal(t, "Screenshot", attachment.Name)
+	assert.Equal(t, []uint8{0x61, 0x62}, attachment.Payload)
+	assert.Nil(t, attachment.userInfo, "a missing userInfo should decode as empty, not panic")
+}
+
+// A real userInfo dictionary must still be decoded.
+func TestNewXCTAttachmentWithUserInfoDictionary(t *testing.T) {
+	objects := []interface{}{
+		"$null",
+		"public.png",
+		"shot.png",
+		float64(1),
+		"Shot",
+		[]uint8{0x01},
+		map[string]interface{}{"NS.keys": []interface{}{}, "NS.objects": []interface{}{}},
+	}
+	object := map[string]interface{}{
+		"lifetime":              uint64(2),
+		"uniformTypeIdentifier": plist.UID(1),
+		"fileNameOverride":      plist.UID(2),
+		"timestamp":             plist.UID(3),
+		"name":                  plist.UID(4),
+		"payload":               plist.UID(5),
+		"userInfo":              plist.UID(6),
+	}
+
+	var decoded interface{}
+	require.NotPanics(t, func() { decoded = NewXCTAttachment(object, objects) })
+	attachment, ok := decoded.(XCTAttachment)
+	require.True(t, ok)
+	assert.Equal(t, "public.png", attachment.UniformTypeIdentifier)
+	assert.NotNil(t, attachment.userInfo)
+}
+
+// A reference past the end of the object table would index out of range.
+func TestNewXCTAttachmentToleratesMalformedArchives(t *testing.T) {
+	objects := []interface{}{"$null", "public.png"}
+
+	tests := []struct {
+		name   string
+		object map[string]interface{}
+	}{
+		{name: "empty object", object: map[string]interface{}{}},
+		{
+			name: "reference out of range",
+			object: map[string]interface{}{
+				"lifetime": uint64(1),
+				"name":     plist.UID(99),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.NotPanics(t, func() { NewXCTAttachment(tt.object, objects) })
+		})
+	}
+}
+
+// The same "$null" shape reaches XCTIssue and its nested source code context.
+func TestNewXCTIssueWithNullSourceCodeContext(t *testing.T) {
+	objects := []interface{}{"$null", "compact", "detailed"}
+	object := map[string]interface{}{
+		"runtimeIssueSeverity": uint64(1),
+		"compact-description":  plist.UID(1),
+		"detailed-description": plist.UID(2),
+		"source-code-context":  plist.UID(0), // "$null"
+	}
+
+	var decoded interface{}
+	require.NotPanics(t, func() { decoded = NewXCTIssue(object, objects) })
+
+	issue, ok := decoded.(XCTIssue)
+	require.True(t, ok)
+	assert.Equal(t, "compact", issue.CompactDescription)
+	assert.Equal(t, "detailed", issue.DetailedDescription)
+}
+
+func TestNewXCTSourceCodeContextWithNullLocation(t *testing.T) {
+	objects := []interface{}{"$null"}
+
+	require.NotPanics(t, func() {
+		NewXCTSourceCodeContext(map[string]interface{}{"location": plist.UID(0)}, objects)
+	})
+	require.NotPanics(t, func() {
+		NewXCTSourceCodeContext(map[string]interface{}{}, objects)
+	})
+}
+
+func TestNewXCTSourceCodeLocationToleratesMissingFileURL(t *testing.T) {
+	objects := []interface{}{"$null", map[string]interface{}{"NS.relative": plist.UID(0)}}
+
+	require.NotPanics(t, func() {
+		NewXCTSourceCodeLocation(map[string]interface{}{"file-url": plist.UID(0), "line-number": uint64(3)}, objects)
+	})
+	require.NotPanics(t, func() {
+		NewXCTSourceCodeLocation(map[string]interface{}{"file-url": plist.UID(1)}, objects)
+	})
+}