Skip to content

Marshaler returning nil creates malformed output #84

@grahammiln

Description

@grahammiln

If a value implementing the Marshaler interface returns a nil replacement value, the resulting encoding is malformed. The value's key is encoded but the value is silently skipped:

<plist version="1.0">
	<dict>
		<key>C</key>
	</dict>
</plist>

The correct behaviour is to return an encoding error. The exception is if the non-nil Marshaler value has the omitempty structure tag and it returns a replacement of nil, then the entire key value pair should be omitted:

type S struct {
	C CustomMarshaler `plist:"C,omitempty"`
}

Below are some tests to help isolate the problem:

type CustomMarshaler struct {
	value interface{}
}

var _ plist.Marshaler = (*CustomMarshaler)(nil)

func (c *CustomMarshaler) MarshalPlist() (interface{}, error) {
	return c.value, nil
}

func TestPlistMarshalerNil(t *testing.T) {

	// Direct non-nil value encodes
	t.Run("string", func(t *testing.T) {
		c := &CustomMarshaler{value: "hello world"}
		b, err := plist.Marshal(c, plist.XMLFormat)
		if err != nil {
			t.Error(err)
		}
		if len(b) == 0 {
			t.Error("expect non-nil")
		}

		// <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
		// <plist version="1.0"><string>hello world</string></plist>
	})

	// Direct nil value correctly returns an error
	t.Run("nil", func(t *testing.T) {
		c := &CustomMarshaler{}
		b, err := plist.Marshal(c, plist.XMLFormat)
		if err == nil {
			t.Error("expect error")
		}
		if len(b) != 0 {
			t.Error("expect nil")
		}
	})

	// Field nil value with omitempty correctly omitted
	t.Run("ptr-omitempty", func(t *testing.T) {
		type Structure struct {
			C *CustomMarshaler `plist:"C,omitempty"`
		}
		s := &Structure{}
		b, err := plist.Marshal(s, plist.XMLFormat)
		if err != nil {
			t.Error(err)
		}
		if len(b) == 0 {
			t.Error("expect non-nil")
		}

		// <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
		// <plist version="1.0"><dict></dict></plist>
	})

	// Non-nil field returning marshaler nil value with omitempty should be omitted
	t.Run("omitempty", func(t *testing.T) {
		type Structure struct {
			C CustomMarshaler `plist:"C,omitempty"`
		}
		s := &Structure{}
		b, err := plist.Marshal(s, plist.XMLFormat)
		if err != nil {
			t.Error(err)
		}
		if len(b) == 0 {
			t.Error("expect non-nil")
		}
		//t.Log(string(b))

		// Unmarshal to prove malformed encoding
		var dst Structure
		if _, err := plist.Unmarshal(b, &dst); err != nil {
			t.Error(err) // plist: error parsing XML property list: missing value in dictionary
		}

		// Get key without value and no error:
		// <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
		// <plist version="1.0"><dict><key>C</key></dict></plist>

		// Expect:
		// <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
		// <plist version="1.0"><dict></dict></plist>
	})
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions