Fix CRLF detection on single-file app + fix YAML documents order detection for single-file apps

- Split and Merge remembers the CRLF detection while outputting the result

Signed-off-by: Silvin Lubecki <[email protected]>

Author: silvin-lubecki

internal/packager/split.go

@@ -45,9 +45,9 @@ func Merge(app *types.App, target io.Writer) error {
 	}
 	for _, data := range [][]byte{
 		app.MetadataRaw(),
-		[]byte(types.SingleFileSeparator),
+		types.YamlSingleFileSeparator(app.HasCRLF()),
 		app.Composes()[0],
-		[]byte(types.SingleFileSeparator),
+		types.YamlSingleFileSeparator(app.HasCRLF()),
 		app.ParametersRaw()[0],
 	} {
 		if _, err := target.Write(data); err != nil {

loader/loader.go

@@ -1,11 +1,16 @@
 package loader
 
 import (
+	"bytes"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
+
+	"github.com/docker/cli/cli/compose/loader"
+
+	"github.com/docker/app/specification"
+	"github.com/docker/cli/cli/compose/schema"
 
 	"github.com/docker/app/internal"
 	"github.com/docker/app/types"
@@ -19,20 +24,38 @@ func LoadFromSingleFile(path string, r io.Reader, ops ...func(*types.App) error)
 	if err != nil {
 		return nil, errors.Wrap(err, "error reading single-file")
 	}
-	parts := strings.Split(string(data), types.SingleFileSeparator)
+	hasCRLF := bytes.Contains(data, []byte{'\r', '\n'})
+
+	parts := bytes.Split(data, types.YamlSingleFileSeparator(hasCRLF))
 	if len(parts) != 3 {
 		return nil, errors.Errorf("malformed single-file application: expected 3 documents, got %d", len(parts))
 	}
-	// 0. is metadata
-	metadata := strings.NewReader(parts[0])
-	// 1. is compose
-	compose := strings.NewReader(parts[1])
-	// 2. is parameters
-	parameters := strings.NewReader(parts[2])
+
+	var (
+		metadata io.Reader
+		compose  io.Reader
+		params   io.Reader
+	)
+	for i := 0; i < 3; i++ {
+		parsed, err := loader.ParseYAML(parts[i])
+		if err != nil {
+			return nil, err
+		}
+		if err := specification.Validate(parsed, internal.MetadataVersion); metadata == nil && err == nil {
+			metadata = bytes.NewBuffer(parts[i])
+		} else if err2 := schema.Validate(parsed, schema.Version(parsed)); compose == nil && err2 == nil {
+			compose = bytes.NewBuffer(parts[i])
+		} else if params == nil {
+			params = bytes.NewBuffer(parts[i])
+		} else {
+			return nil, errors.New("malformed single-file application")
+		}
+	}
 	appOps := append([]func(*types.App) error{
 		types.WithComposes(compose),
-		types.WithParameters(parameters),
+		types.WithParameters(params),
 		types.Metadata(metadata),
+		types.WithCRLF(hasCRLF),
 	}, ops...)
 	return types.NewApp(path, appOps...)
 }

loader/loader_test.go

@@ -18,26 +18,44 @@ import (
 
 const (
 	metadata = `name: my-app
-version: 1.0.0`
-	yaml = `version: "3.1"
-
-services:
+version: 1.0.0
+`
+	compose = `services:
   web:
-    image: nginx`
-	parameters = `foo: bar`
+    image: nginx
+version: "3.1"
+`
+	params = `foo: bar
+`
 )
 
 func TestLoadFromSingleFile(t *testing.T) {
-	singlefile := fmt.Sprintf(`%s
----
-%s
----
-%s`, metadata, yaml, parameters)
-	app, err := LoadFromSingleFile("my-app", strings.NewReader(singlefile))
-	assert.NilError(t, err)
-	assert.Assert(t, app != nil)
-	assert.Assert(t, is.Equal(app.Path, "my-app"))
-	assertAppContent(t, app)
+	testCases := []struct {
+		name string
+		file string
+	}{
+		{
+			name: "line-feed",
+			file: fmt.Sprintf("%s\n---\n%s\n---\n%s", metadata, compose, params),
+		},
+		{
+			name: "carriage-return-line-feed",
+			file: fmt.Sprintf("%s\r\n---\r\n%s\r\n---\r\n%s", metadata, compose, params),
+		},
+		{
+			name: "unordered-documents",
+			file: fmt.Sprintf("%s\n---\n%s\n---\n%s", params, metadata, compose),
+		},
+	}
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			app, err := LoadFromSingleFile("my-app", strings.NewReader(test.file))
+			assert.NilError(t, err)
+			assert.Assert(t, app != nil)
+			assert.Assert(t, is.Equal(app.Path, "my-app"))
+			assertAppContent(t, app)
+		})
+	}
 }
 
 func TestLoadFromSingleFileInvalidReader(t *testing.T) {
@@ -46,17 +64,17 @@ func TestLoadFromSingleFileInvalidReader(t *testing.T) {
 }
 
 func TestLoadFromSingleFileMalformed(t *testing.T) {
-	_, err := LoadFromSingleFile("my-app", strings.NewReader(`foo
+	_, err := LoadFromSingleFile("my-app", strings.NewReader(`foo: foo
 ---
-bar`))
+bar: bar`))
 	assert.ErrorContains(t, err, "malformed single-file application")
 }
 
 func TestLoadFromDirectory(t *testing.T) {
 	dir := fs.NewDir(t, "my-app",
 		fs.WithFile(internal.MetadataFileName, metadata),
-		fs.WithFile(internal.ParametersFileName, parameters),
-		fs.WithFile(internal.ComposeFileName, yaml),
+		fs.WithFile(internal.ParametersFileName, params),
+		fs.WithFile(internal.ComposeFileName, compose),
 	)
 	defer dir.Remove()
 	app, err := LoadFromDirectory(dir.Path())
@@ -69,8 +87,8 @@ func TestLoadFromDirectory(t *testing.T) {
 func TestLoadFromDirectoryDeprecatedSettings(t *testing.T) {
 	dir := fs.NewDir(t, "my-app",
 		fs.WithFile(internal.MetadataFileName, metadata),
-		fs.WithFile(internal.DeprecatedSettingsFileName, parameters),
-		fs.WithFile(internal.ComposeFileName, yaml),
+		fs.WithFile(internal.DeprecatedSettingsFileName, params),
+		fs.WithFile(internal.ComposeFileName, compose),
 	)
 	defer dir.Remove()
 	app, err := LoadFromDirectory(dir.Path())
@@ -97,8 +115,8 @@ func createAppTar(t *testing.T) *fs.File {
 	t.Helper()
 	dir := fs.NewDir(t, "my-app",
 		fs.WithFile(internal.MetadataFileName, metadata),
-		fs.WithFile(internal.ParametersFileName, parameters),
-		fs.WithFile(internal.ComposeFileName, yaml),
+		fs.WithFile(internal.ParametersFileName, params),
+		fs.WithFile(internal.ComposeFileName, compose),
 	)
 	defer dir.Remove()
 	r, err := archive.TarWithOptions(dir.Path(), &archive.TarOptions{
@@ -117,9 +135,9 @@ func assertContentIs(t *testing.T, actual []byte, expected string) {
 
 func assertAppContent(t *testing.T, app *types.App) {
 	assert.Assert(t, is.Len(app.ParametersRaw(), 1))
-	assertContentIs(t, app.ParametersRaw()[0], parameters)
+	assertContentIs(t, app.ParametersRaw()[0], params)
 	assert.Assert(t, is.Len(app.Composes(), 1))
-	assertContentIs(t, app.Composes()[0], yaml)
+	assertContentIs(t, app.Composes()[0], compose)
 	assertContentIs(t, app.MetadataRaw(), metadata)
 }
 

specification/schema.go

@@ -36,5 +36,4 @@ func Validate(config map[string]interface{}, version string) error {
 	}
 
 	return nil
-
 }

types/types.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"bytes"
 	"errors"
 	"io"
 	"io/ioutil"
@@ -13,9 +14,6 @@ import (
 	"github.com/docker/app/types/parameters"
 )
 
-// SingleFileSeparator is the separator used in single-file app
-const SingleFileSeparator = "\n---\n"
-
 // AppSourceKind represents what format the app was in when read
 type AppSourceKind int
 
@@ -30,6 +28,14 @@ const (
 	AppSourceArchive
 )
 
+// YamlSingleFileSeparator returns the separator used in single-file app, depending detected CRLF
+func YamlSingleFileSeparator(hasCRLF bool) []byte {
+	if hasCRLF {
+		return []byte("\r\n---\r\n")
+	}
+	return []byte("\n---\n")
+}
+
 // ShouldRunInsideDirectory returns whether the package is run from a directory on disk
 func (a AppSourceKind) ShouldRunInsideDirectory() bool {
 	return a == AppSourceSplit || a == AppSourceImage || a == AppSourceArchive
@@ -48,6 +54,7 @@ type App struct {
 	metadataContent   []byte
 	metadata          metadata.AppMetadata
 	attachments       []Attachment
+	hasCRLF           bool
 }
 
 // Attachment is a summary of an attachment (attached file) stored in the app definition
@@ -96,6 +103,10 @@ func (a *App) Attachments() []Attachment {
 	return a.attachments
 }
 
+func (a *App) HasCRLF() bool {
+	return a.hasCRLF
+}
+
 // Extract writes the app in the specified folder
 func (a *App) Extract(path string) error {
 	if err := ioutil.WriteFile(filepath.Join(path, internal.MetadataFileName), a.MetadataRaw(), 0644); err != nil {
@@ -177,6 +188,13 @@ func WithSource(source AppSourceKind) func(*App) error {
 	}
 }
 
+func WithCRLF(hasCRLF bool) func(*App) error {
+	return func(app *App) error {
+		app.hasCRLF = hasCRLF
+		return nil
+	}
+}
+
 // WithParametersFiles adds the specified parameters files to the app
 func WithParametersFiles(files ...string) func(*App) error {
 	return parametersLoader(func() ([][]byte, error) { return readFiles(files...) })
@@ -258,6 +276,7 @@ func metadataLoader(f func() ([]byte, error)) func(app *App) error {
 		}
 		app.metadata = loaded
 		app.metadataContent = d
+		app.hasCRLF = bytes.Contains(d, []byte{'\r', '\n'})
 		return nil
 	}
 }