From 0325de0850646d73d02cd1c43e0ba4ffe79211b2 Mon Sep 17 00:00:00 2001 From: Kory Prince Date: Wed, 9 Nov 2022 18:29:50 -0600 Subject: [PATCH 1/3] added test for parity with macOS's plutil -lint --- decode_test.go | 50 +++++++++++++++++++++++++ testdata/xml/empty-doctype.plist | 8 ++++ testdata/xml/empty-plist.plist | 4 ++ testdata/xml/empty-xml.plist | 8 ++++ testdata/xml/invalid-before-plist.plist | 9 +++++ testdata/xml/invalid-data.plist | 9 +++++ testdata/xml/invalid-end.plist | 9 +++++ testdata/xml/invalid-middle.plist | 9 +++++ testdata/xml/invalid-start.plist | 8 ++++ testdata/xml/malformed-xml.plist | 8 ++++ testdata/xml/no-both.plist | 6 +++ testdata/xml/no-dict-end.plist | 7 ++++ testdata/xml/no-doctype.plist | 7 ++++ testdata/xml/no-plist-end.plist | 7 ++++ testdata/xml/no-plist-version.plist | 8 ++++ testdata/xml/no-xml-tag.plist | 7 ++++ testdata/xml/swapped.plist | 8 ++++ testdata/xml/unescaped-plist.plist | 8 ++++ testdata/xml/unescaped-xml.plist | 8 ++++ testdata/xml/valid.plist | 8 ++++ 20 files changed, 196 insertions(+) create mode 100644 testdata/xml/empty-doctype.plist create mode 100644 testdata/xml/empty-plist.plist create mode 100644 testdata/xml/empty-xml.plist create mode 100644 testdata/xml/invalid-before-plist.plist create mode 100644 testdata/xml/invalid-data.plist create mode 100644 testdata/xml/invalid-end.plist create mode 100644 testdata/xml/invalid-middle.plist create mode 100644 testdata/xml/invalid-start.plist create mode 100644 testdata/xml/malformed-xml.plist create mode 100644 testdata/xml/no-both.plist create mode 100644 testdata/xml/no-dict-end.plist create mode 100644 testdata/xml/no-doctype.plist create mode 100644 testdata/xml/no-plist-end.plist create mode 100644 testdata/xml/no-plist-version.plist create mode 100644 testdata/xml/no-xml-tag.plist create mode 100644 testdata/xml/swapped.plist create mode 100644 testdata/xml/unescaped-plist.plist create mode 100644 testdata/xml/unescaped-xml.plist create mode 100644 testdata/xml/valid.plist diff --git a/decode_test.go b/decode_test.go index bd171a1..a8295d0 100644 --- a/decode_test.go +++ b/decode_test.go @@ -8,6 +8,7 @@ import ( "log" "net/http" "net/http/httptest" + "os/exec" "path/filepath" "reflect" "testing" @@ -513,3 +514,52 @@ func TestDecodeTagSkip(t *testing.T) { t.Error("field decoded when it was tagged as -") } } + +// test parity with plutil -lint on macOS +var xmlParityTestFailures = map[string]struct{}{ + "empty-plist.plist": {}, + "invalid-before-plist.plist": {}, + "invalid-data.plist": {}, + "invalid-middle.plist": {}, + "invalid-start.plist": {}, + "no-dict-end.plist": {}, + "no-plist-end.plist": {}, + "unescaped-plist.plist": {}, + "unescaped-xml.plist": {}, +} + +func TestXMLPlutilParity(t *testing.T) { + type data struct { + Key string `plist:"key"` + } + tests, err := ioutil.ReadDir("testdata/xml/") + if err != nil { + t.Fatalf("could not open testdata/xml: %v", err) + } + + plutil, _ := exec.LookPath("plutil") + + for _, test := range tests { + testPath := filepath.Join("testdata/xml/", test.Name()) + buf, err := ioutil.ReadFile(testPath) + if err != nil { + t.Errorf("could not read test %s: %v", test.Name(), err) + continue + } + v := new(data) + err = Unmarshal(buf, v) + + _, check := xmlParityTestFailures[test.Name()] + if plutil != "" { + check = exec.Command(plutil, "-lint", testPath).Run() != nil + } + + if check && err == nil { + t.Errorf("expected error for test %s but got: nil", test.Name()) + } else if !check && err != nil { + t.Errorf("expected no error for test %s but got: %v", test.Name(), err) + } else if !check && v.Key != "val" { + t.Errorf("expected key=val for test %s but got: key=%s", test.Name(), v.Key) + } + } +} diff --git a/testdata/xml/empty-doctype.plist b/testdata/xml/empty-doctype.plist new file mode 100644 index 0000000..aa4a935 --- /dev/null +++ b/testdata/xml/empty-doctype.plist @@ -0,0 +1,8 @@ + + + + + key + val + + diff --git a/testdata/xml/empty-plist.plist b/testdata/xml/empty-plist.plist new file mode 100644 index 0000000..8b232f6 --- /dev/null +++ b/testdata/xml/empty-plist.plist @@ -0,0 +1,4 @@ + + + + diff --git a/testdata/xml/empty-xml.plist b/testdata/xml/empty-xml.plist new file mode 100644 index 0000000..05a26d1 --- /dev/null +++ b/testdata/xml/empty-xml.plist @@ -0,0 +1,8 @@ + + + + + key + val + + diff --git a/testdata/xml/invalid-before-plist.plist b/testdata/xml/invalid-before-plist.plist new file mode 100644 index 0000000..ad9bd75 --- /dev/null +++ b/testdata/xml/invalid-before-plist.plist @@ -0,0 +1,9 @@ + + +invalid + + + key + val + + diff --git a/testdata/xml/invalid-data.plist b/testdata/xml/invalid-data.plist new file mode 100644 index 0000000..7bb255e --- /dev/null +++ b/testdata/xml/invalid-data.plist @@ -0,0 +1,9 @@ + + + + + key + val +invalid + + diff --git a/testdata/xml/invalid-end.plist b/testdata/xml/invalid-end.plist new file mode 100644 index 0000000..bafbc0c --- /dev/null +++ b/testdata/xml/invalid-end.plist @@ -0,0 +1,9 @@ + + + + + key + val + + +invalid diff --git a/testdata/xml/invalid-middle.plist b/testdata/xml/invalid-middle.plist new file mode 100644 index 0000000..ed4abab --- /dev/null +++ b/testdata/xml/invalid-middle.plist @@ -0,0 +1,9 @@ + +invalid + + + + key + val + + diff --git a/testdata/xml/invalid-start.plist b/testdata/xml/invalid-start.plist new file mode 100644 index 0000000..8ac26f5 --- /dev/null +++ b/testdata/xml/invalid-start.plist @@ -0,0 +1,8 @@ +invalid + + + + key + val + + diff --git a/testdata/xml/malformed-xml.plist b/testdata/xml/malformed-xml.plist new file mode 100644 index 0000000..539ffb2 --- /dev/null +++ b/testdata/xml/malformed-xml.plist @@ -0,0 +1,8 @@ + + + + + key + val + + diff --git a/testdata/xml/no-both.plist b/testdata/xml/no-both.plist new file mode 100644 index 0000000..4fa7ef5 --- /dev/null +++ b/testdata/xml/no-both.plist @@ -0,0 +1,6 @@ + + + key + val + + diff --git a/testdata/xml/no-dict-end.plist b/testdata/xml/no-dict-end.plist new file mode 100644 index 0000000..508e45c --- /dev/null +++ b/testdata/xml/no-dict-end.plist @@ -0,0 +1,7 @@ + + + + + key + val + diff --git a/testdata/xml/no-doctype.plist b/testdata/xml/no-doctype.plist new file mode 100644 index 0000000..f8134b4 --- /dev/null +++ b/testdata/xml/no-doctype.plist @@ -0,0 +1,7 @@ + + + + key + val + + diff --git a/testdata/xml/no-plist-end.plist b/testdata/xml/no-plist-end.plist new file mode 100644 index 0000000..d34f609 --- /dev/null +++ b/testdata/xml/no-plist-end.plist @@ -0,0 +1,7 @@ + + + + + key + val + diff --git a/testdata/xml/no-plist-version.plist b/testdata/xml/no-plist-version.plist new file mode 100644 index 0000000..6ef5b11 --- /dev/null +++ b/testdata/xml/no-plist-version.plist @@ -0,0 +1,8 @@ + + + + + key + val + + diff --git a/testdata/xml/no-xml-tag.plist b/testdata/xml/no-xml-tag.plist new file mode 100644 index 0000000..80047b0 --- /dev/null +++ b/testdata/xml/no-xml-tag.plist @@ -0,0 +1,7 @@ + + + + key + val + + diff --git a/testdata/xml/swapped.plist b/testdata/xml/swapped.plist new file mode 100644 index 0000000..0304264 --- /dev/null +++ b/testdata/xml/swapped.plist @@ -0,0 +1,8 @@ + + + + + key + val + + diff --git a/testdata/xml/unescaped-plist.plist b/testdata/xml/unescaped-plist.plist new file mode 100644 index 0000000..d45ca28 --- /dev/null +++ b/testdata/xml/unescaped-plist.plist @@ -0,0 +1,8 @@ + + + + key + val + + diff --git a/testdata/xml/unescaped-xml.plist b/testdata/xml/unescaped-xml.plist new file mode 100644 index 0000000..116dfe4 --- /dev/null +++ b/testdata/xml/unescaped-xml.plist @@ -0,0 +1,8 @@ + + + + key + val + + diff --git a/testdata/xml/valid.plist b/testdata/xml/valid.plist new file mode 100644 index 0000000..96f10ba --- /dev/null +++ b/testdata/xml/valid.plist @@ -0,0 +1,8 @@ + + + + + key + val + + From 92934b9e6f0b718fa6ef4b00be92d9ea0fae92d1 Mon Sep 17 00:00:00 2001 From: Kory Prince Date: Wed, 9 Nov 2022 21:19:27 -0600 Subject: [PATCH 2/3] return error when extraneous tags/chardata are present or start/end tags are mismatched/missing (parity with plutil) --- xml_parser.go | 85 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 26 deletions(-) diff --git a/xml_parser.go b/xml_parser.go index 146e72b..375d680 100644 --- a/xml_parser.go +++ b/xml_parser.go @@ -1,6 +1,7 @@ package plist import ( + "bytes" "encoding/base64" "encoding/xml" "errors" @@ -22,19 +23,28 @@ func newXMLParser(r io.Reader) *xmlParser { } func (p *xmlParser) parseDocument(start *xml.StartElement) (*plistValue, error) { - if start == nil { - for { - tok, err := p.Token() - if err != nil { - return nil, err - } - if t, ok := tok.(xml.StartElement); ok { - start = &t - break + if start != nil { + return p.parseXMLElement(start) + } + + for { + tok, err := p.Token() + if err != nil { + return nil, err + } + switch el := tok.(type) { + case xml.StartElement: + return p.parseXMLElement(&el) + case xml.ProcInst, xml.Directive: + continue + case xml.CharData: + if len(bytes.TrimSpace(el)) != 0 { + return nil, errors.New("plist: unexpected non-empty xml.CharData") } + default: + return nil, fmt.Errorf("unexpected element: %T", el) } } - return p.parseXMLElement(start) } func (p *xmlParser) parseXMLElement(element *xml.StartElement) (*plistValue, error) { @@ -63,19 +73,32 @@ func (p *xmlParser) parseXMLElement(element *xml.StartElement) (*plistValue, err } func (p *xmlParser) parsePlist(element *xml.StartElement) (*plistValue, error) { + var val *plistValue for { token, err := p.Token() if err != nil { return nil, err } - if el, ok := token.(xml.EndElement); ok && el.Name.Local == "plist" { - break - } - if el, ok := token.(xml.StartElement); ok { - return p.parseXMLElement(&el) + switch el := token.(type) { + case xml.EndElement: + if val == nil { + return nil, errors.New("plist: empty plist tag") + } + return val, nil + case xml.StartElement: + v, err := p.parseXMLElement(&el) + if err != nil { + return v, err + } + val = v + case xml.CharData: + if len(bytes.TrimSpace(el)) != 0 { + return nil, errors.New("plist: unexpected non-empty xml.CharData") + } + default: + return nil, fmt.Errorf("unexpected element: %T", el) } } - return nil, errors.New("plist: Invalid plist") } func (p *xmlParser) parseDict(element *xml.StartElement) (*plistValue, error) { @@ -86,10 +109,10 @@ func (p *xmlParser) parseDict(element *xml.StartElement) (*plistValue, error) { if err != nil { return nil, err } - if el, ok := token.(xml.EndElement); ok && el.Name.Local == "dict" { - break - } - if el, ok := token.(xml.StartElement); ok { + switch el := token.(type) { + case xml.EndElement: + return &plistValue{Dictionary, &dictionary{m: subvalues}}, nil + case xml.StartElement: if el.Name.Local == "key" { var k string if err := p.DecodeElement(&k, &el); err != nil { @@ -106,9 +129,14 @@ func (p *xmlParser) parseDict(element *xml.StartElement) (*plistValue, error) { return nil, err } key = nil + case xml.CharData: + if len(bytes.TrimSpace(el)) != 0 { + return nil, errors.New("plist: unexpected non-empty xml.CharData") + } + default: + return nil, fmt.Errorf("unexpected element: %T", el) } } - return &plistValue{Dictionary, &dictionary{m: subvalues}}, nil } func (p *xmlParser) parseString(element *xml.StartElement) (*plistValue, error) { @@ -134,18 +162,23 @@ func (p *xmlParser) parseArray(element *xml.StartElement) (*plistValue, error) { if err != nil { return nil, err } - if el, ok := token.(xml.EndElement); ok && el.Name.Local == "array" { - break - } - if el, ok := token.(xml.StartElement); ok { + switch el := token.(type) { + case xml.EndElement: + return &plistValue{Array, subvalues}, nil + case xml.StartElement: subv, err := p.parseXMLElement(&el) if err != nil { return nil, err } subvalues = append(subvalues, subv) + case xml.CharData: + if len(bytes.TrimSpace(el)) != 0 { + return nil, errors.New("plist: unexpected non-empty xml.CharData") + } + default: + return nil, fmt.Errorf("unexpected element: %T", el) } } - return &plistValue{Array, subvalues}, nil } func (p *xmlParser) parseReal(element *xml.StartElement) (*plistValue, error) { From cf92745b8eb3979615b37d9c07b3f168ec4154cb Mon Sep 17 00:00:00 2001 From: Kory Prince Date: Mon, 2 Dec 2024 21:46:49 -0600 Subject: [PATCH 3/3] mark test cases as failure by filename --- decode_test.go | 28 +++++++------------ ...-plist.plist => empty-plist.failure.plist} | 0 ...ist => invalid-before-plist.failure.plist} | 0 ...-data.plist => invalid-data.failure.plist} | 0 ...dle.plist => invalid-middle.failure.plist} | 0 ...tart.plist => invalid-start.failure.plist} | 0 ...ct-end.plist => no-dict-end.failure.plist} | 0 ...t-end.plist => no-plist-end.failure.plist} | 0 ...st.plist => unescaped-plist.failure.plist} | 0 ...-xml.plist => unescaped-xml.failure.plist} | 0 10 files changed, 10 insertions(+), 18 deletions(-) rename testdata/xml/{empty-plist.plist => empty-plist.failure.plist} (100%) rename testdata/xml/{invalid-before-plist.plist => invalid-before-plist.failure.plist} (100%) rename testdata/xml/{invalid-data.plist => invalid-data.failure.plist} (100%) rename testdata/xml/{invalid-middle.plist => invalid-middle.failure.plist} (100%) rename testdata/xml/{invalid-start.plist => invalid-start.failure.plist} (100%) rename testdata/xml/{no-dict-end.plist => no-dict-end.failure.plist} (100%) rename testdata/xml/{no-plist-end.plist => no-plist-end.failure.plist} (100%) rename testdata/xml/{unescaped-plist.plist => unescaped-plist.failure.plist} (100%) rename testdata/xml/{unescaped-xml.plist => unescaped-xml.failure.plist} (100%) diff --git a/decode_test.go b/decode_test.go index a8295d0..e0b9d75 100644 --- a/decode_test.go +++ b/decode_test.go @@ -11,6 +11,7 @@ import ( "os/exec" "path/filepath" "reflect" + "strings" "testing" "time" ) @@ -515,19 +516,7 @@ func TestDecodeTagSkip(t *testing.T) { } } -// test parity with plutil -lint on macOS -var xmlParityTestFailures = map[string]struct{}{ - "empty-plist.plist": {}, - "invalid-before-plist.plist": {}, - "invalid-data.plist": {}, - "invalid-middle.plist": {}, - "invalid-start.plist": {}, - "no-dict-end.plist": {}, - "no-plist-end.plist": {}, - "unescaped-plist.plist": {}, - "unescaped-xml.plist": {}, -} - +// TestXMLPlutilParity tests parity with plutil -lint on macOS func TestXMLPlutilParity(t *testing.T) { type data struct { Key string `plist:"key"` @@ -549,16 +538,19 @@ func TestXMLPlutilParity(t *testing.T) { v := new(data) err = Unmarshal(buf, v) - _, check := xmlParityTestFailures[test.Name()] + shouldFail := strings.HasSuffix(test.Name(), ".failure.plist") if plutil != "" { - check = exec.Command(plutil, "-lint", testPath).Run() != nil + plutilFail := exec.Command(plutil, "-lint", testPath).Run() != nil + if shouldFail != plutilFail { + t.Errorf("expected plutil test failure: %v for %s, but got test failure: %v", shouldFail, test.Name(), plutilFail) + } } - if check && err == nil { + if shouldFail && err == nil { t.Errorf("expected error for test %s but got: nil", test.Name()) - } else if !check && err != nil { + } else if !shouldFail && err != nil { t.Errorf("expected no error for test %s but got: %v", test.Name(), err) - } else if !check && v.Key != "val" { + } else if !shouldFail && v.Key != "val" { t.Errorf("expected key=val for test %s but got: key=%s", test.Name(), v.Key) } } diff --git a/testdata/xml/empty-plist.plist b/testdata/xml/empty-plist.failure.plist similarity index 100% rename from testdata/xml/empty-plist.plist rename to testdata/xml/empty-plist.failure.plist diff --git a/testdata/xml/invalid-before-plist.plist b/testdata/xml/invalid-before-plist.failure.plist similarity index 100% rename from testdata/xml/invalid-before-plist.plist rename to testdata/xml/invalid-before-plist.failure.plist diff --git a/testdata/xml/invalid-data.plist b/testdata/xml/invalid-data.failure.plist similarity index 100% rename from testdata/xml/invalid-data.plist rename to testdata/xml/invalid-data.failure.plist diff --git a/testdata/xml/invalid-middle.plist b/testdata/xml/invalid-middle.failure.plist similarity index 100% rename from testdata/xml/invalid-middle.plist rename to testdata/xml/invalid-middle.failure.plist diff --git a/testdata/xml/invalid-start.plist b/testdata/xml/invalid-start.failure.plist similarity index 100% rename from testdata/xml/invalid-start.plist rename to testdata/xml/invalid-start.failure.plist diff --git a/testdata/xml/no-dict-end.plist b/testdata/xml/no-dict-end.failure.plist similarity index 100% rename from testdata/xml/no-dict-end.plist rename to testdata/xml/no-dict-end.failure.plist diff --git a/testdata/xml/no-plist-end.plist b/testdata/xml/no-plist-end.failure.plist similarity index 100% rename from testdata/xml/no-plist-end.plist rename to testdata/xml/no-plist-end.failure.plist diff --git a/testdata/xml/unescaped-plist.plist b/testdata/xml/unescaped-plist.failure.plist similarity index 100% rename from testdata/xml/unescaped-plist.plist rename to testdata/xml/unescaped-plist.failure.plist diff --git a/testdata/xml/unescaped-xml.plist b/testdata/xml/unescaped-xml.failure.plist similarity index 100% rename from testdata/xml/unescaped-xml.plist rename to testdata/xml/unescaped-xml.failure.plist