fix(jar): allow a missing final newline in case of only one section

Also tweaks output formatting to sort keys and wrap long values in a way
more consistent with other tool output.

lib/signjar/manifest.go

@@ -23,6 +23,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"sort"
 	"strings"
 
 	"github.com/sassoftware/relic/v7/config"
@@ -94,13 +95,19 @@ func splitManifest(manifest []byte) ([][]byte, error) {
 		i1 := bytes.Index(manifest, []byte("\r\n\r\n"))
 		i2 := bytes.Index(manifest, []byte("\n\n"))
 		var idx int
-		if i1 < 0 {
-			if i2 < 0 {
-				return nil, errors.New("trailing bytes after last newline")
-			}
-			idx = i2 + 2
-		} else {
+		switch {
+		case i1 >= 0:
 			idx = i1 + 4
+		case i2 >= 0:
+			idx = i2 + 2
+		case len(sections) == 0:
+			// as a special case, accept a single final newline if it's the only section
+			if manifest[len(manifest)-1] == '\n' {
+				return [][]byte{manifest}, nil
+			}
+			fallthrough
+		default:
+			return nil, errors.New("trailing bytes after last newline")
 		}
 		section := manifest[:idx]
 		manifest = manifest[idx:]
@@ -178,16 +185,19 @@ const maxLineLength = 70
 // Write a key-value pair, wrapping long lines as necessary
 func writeAttribute(out *bytes.Buffer, key, value string) {
 	line := []byte(fmt.Sprintf("%s: %s", key, value))
-	for i := 0; i < len(line); i += maxLineLength {
-		j := i + maxLineLength
-		if j > len(line) {
-			j = len(line)
-		}
+	for i := 0; i < len(line); {
+		goal := maxLineLength
 		if i != 0 {
 			out.Write([]byte{' '})
+			goal--
+		}
+		j := i + goal
+		if j > len(line) {
+			j = len(line)
 		}
 		out.Write(line[i:j])
 		out.Write([]byte("\r\n"))
+		i = j
 	}
 }
 
@@ -196,11 +206,16 @@ func writeSection(out *bytes.Buffer, hdr http.Header, first string) {
 	if value != "" {
 		writeAttribute(out, first, value)
 	}
-	for key, values := range hdr {
+	keys := make([]string, 0, len(hdr))
+	for key := range hdr {
 		if key == first {
 			continue
 		}
-		for _, value := range values {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	for _, key := range keys {
+		for _, value := range hdr[key] {
 			writeAttribute(out, key, value)
 		}
 	}

lib/signjar/manifest_test.go

@@ -0,0 +1,113 @@
+package signjar
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParse(t *testing.T) {
+	t.Parallel()
+	t.Run("Full", func(t *testing.T) {
+		const manifest = `Manifest-Version: 1.0
+Built-By: nobody
+Long-Header-Line: 0123456789abcdef0123456789abcdef0123456789abcdef
+ 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+ 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+
+Name: foo
+Ham: spam
+Eggs: bacon
+
+`
+		main := http.Header{
+			"Manifest-Version": []string{"1.0"},
+			"Built-By":         []string{"nobody"},
+			"Long-Header-Line": []string{
+				"0123456789abcdef0123456789abcdef" +
+					"0123456789abcdef0123456789abcdef0123456789abcdef" +
+					"0123456789abcdef0123456789abcdef0123456789abcdef" +
+					"0123456789abcdef0123456789abcdef0123456789abcdef"},
+		}
+		file := http.Header{
+			"Name": []string{"foo"},
+			"Ham":  []string{"spam"},
+			"Eggs": []string{"bacon"},
+		}
+		expected := &FilesMap{
+			Main:  main,
+			Files: map[string]http.Header{"foo": file},
+			Order: []string{"foo"},
+		}
+		parsed, err := ParseManifest([]byte(manifest))
+		require.NoError(t, err)
+		assert.Equal(t, expected, parsed)
+		crlfManifest := []byte(strings.ReplaceAll(manifest, "\n", "\r\n"))
+		parsed, err = ParseManifest(crlfManifest)
+		require.NoError(t, err)
+		assert.Equal(t, expected, parsed)
+	})
+	t.Run("Truncated", func(t *testing.T) {
+		const manifest = "Manifest-Version: 1.0\n"
+		expected := &FilesMap{
+			Main: http.Header{
+				"Manifest-Version": []string{"1.0"},
+			},
+			Order: []string{},
+			Files: map[string]http.Header{},
+		}
+		parsed, err := ParseManifest([]byte(manifest))
+		require.NoError(t, err)
+		assert.Equal(t, expected, parsed)
+	})
+	t.Run("InvalidTruncated", func(t *testing.T) {
+		const manifest = "Manifest-Version: 1.0\n\nName: foo\n"
+		_, err := ParseManifest([]byte(manifest))
+		require.Error(t, err)
+	})
+	t.Run("InvalidNoName", func(t *testing.T) {
+		const manifest = "Manifest-Version: 1.0\n\nFoo: bar\n\n"
+		_, err := ParseManifest([]byte(manifest))
+		require.Error(t, err)
+	})
+}
+
+func TestDump(t *testing.T) {
+	manifest := &FilesMap{
+		Main: http.Header{
+			"Manifest-Version": []string{"1.0"},
+			"D":                []string{"D"},
+			"C":                []string{"C"},
+			"B":                []string{"B"},
+			"A":                []string{"A"},
+			"Long-Header":      []string{strings.Repeat("0123456789abcdef", 10)},
+		},
+		Files: map[string]http.Header{
+			"foo": {"Name": []string{"foo"}, "Foo": []string{"bar"}},
+			"bar": {"Name": []string{"bar"}, "Foo": []string{"bar"}},
+		},
+		Order: []string{"bar", "foo"},
+	}
+	result := string(manifest.Dump())
+	expected := `Manifest-Version: 1.0
+A: A
+B: B
+C: C
+D: D
+Long-Header: 0123456789abcdef0123456789abcdef0123456789abcdef012345678
+ 9abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcd
+ ef0123456789abcdef0123456789abcdef
+
+Name: bar
+Foo: bar
+
+Name: foo
+Foo: bar
+
+`
+	expected = strings.ReplaceAll(expected, "\n", "\r\n")
+	assert.Equal(t, expected, result)
+}