Merge pull request #7972 from IppX/bug/6081-bom-in-resource-file

Remove utf-8 BOM when loading chart files and helmignore

internal/ignore/rules.go

@@ -18,6 +18,7 @@ package ignore
 
 import (
 	"bufio"
+	"bytes"
 	"io"
 	"log"
 	"os"
@@ -65,8 +66,18 @@ func Parse(file io.Reader) (*Rules, error) {
 	r := &Rules{patterns: []*pattern{}}
 
 	s := bufio.NewScanner(file)
+	currentLine := 0
+	utf8bom := []byte{0xEF, 0xBB, 0xBF}
 	for s.Scan() {
-		if err := r.parseRule(s.Text()); err != nil {
+		scannedBytes := s.Bytes()
+		// We trim UTF8 BOM
+		if currentLine == 0 {
+			scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom)
+		}
+		line := string(scannedBytes)
+		currentLine++
+
+		if err := r.parseRule(line); err != nil {
 			return r, err
 		}
 	}

pkg/chart/loader/archive.go

@@ -173,7 +173,9 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) {
 			return nil, err
 		}
 
-		files = append(files, &BufferedFile{Name: n, Data: b.Bytes()})
+		data := bytes.TrimPrefix(b.Bytes(), utf8bom)
+
+		files = append(files, &BufferedFile{Name: n, Data: data})
 		b.Reset()
 	}
 

pkg/chart/loader/directory.go

@@ -17,6 +17,7 @@ limitations under the License.
 package loader
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -30,6 +31,8 @@ import (
 	"helm.sh/helm/v3/pkg/chart"
 )
 
+var utf8bom = []byte{0xEF, 0xBB, 0xBF}
+
 // DirLoader loads a chart from a directory
 type DirLoader string
 
@@ -104,6 +107,8 @@ func LoadDir(dir string) (*chart.Chart, error) {
 			return errors.Wrapf(err, "error reading %s", n)
 		}
 
+		data = bytes.TrimPrefix(data, utf8bom)
+
 		files = append(files, &BufferedFile{Name: n, Data: data})
 		return nil
 	}

pkg/chart/loader/load_test.go

@@ -20,6 +20,7 @@ import (
 	"archive/tar"
 	"bytes"
 	"compress/gzip"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -85,6 +86,86 @@ func TestLoadDirWithSymlink(t *testing.T) {
 	verifyDependenciesLock(t, c)
 }
 
+func TestBomTestData(t *testing.T) {
+	testFiles := []string{"frobnitz_with_bom/.helmignore", "frobnitz_with_bom/templates/template.tpl", "frobnitz_with_bom/Chart.yaml"}
+	for _, file := range testFiles {
+		data, err := ioutil.ReadFile("testdata/" + file)
+		if err != nil || !bytes.HasPrefix(data, utf8bom) {
+			t.Errorf("Test file has no BOM or is invalid: testdata/%s", file)
+		}
+	}
+
+	archive, err := ioutil.ReadFile("testdata/frobnitz_with_bom.tgz")
+	if err != nil {
+		t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+	}
+	unzipped, err := gzip.NewReader(bytes.NewReader(archive))
+	if err != nil {
+		t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+	}
+	defer unzipped.Close()
+	for _, testFile := range testFiles {
+		data := make([]byte, 3)
+		err := unzipped.Reset(bytes.NewReader(archive))
+		if err != nil {
+			t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+		}
+		tr := tar.NewReader(unzipped)
+		for {
+			file, err := tr.Next()
+			if err == io.EOF {
+				break
+			}
+			if err != nil {
+				t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+			}
+			if file != nil && strings.EqualFold(file.Name, testFile) {
+				_, err := tr.Read(data)
+				if err != nil {
+					t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err)
+				} else {
+					break
+				}
+			}
+		}
+		if !bytes.Equal(data, utf8bom) {
+			t.Fatalf("Test file has no BOM or is invalid: frobnitz_with_bom.tgz/%s", testFile)
+		}
+	}
+}
+
+func TestLoadDirWithUTFBOM(t *testing.T) {
+	l, err := Loader("testdata/frobnitz_with_bom")
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	c, err := l.Load()
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	verifyFrobnitz(t, c)
+	verifyChart(t, c)
+	verifyDependencies(t, c)
+	verifyDependenciesLock(t, c)
+	verifyBomStripped(t, c.Files)
+}
+
+func TestLoadArchiveWithUTFBOM(t *testing.T) {
+	l, err := Loader("testdata/frobnitz_with_bom.tgz")
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	c, err := l.Load()
+	if err != nil {
+		t.Fatalf("Failed to load testdata: %s", err)
+	}
+	verifyFrobnitz(t, c)
+	verifyChart(t, c)
+	verifyDependencies(t, c)
+	verifyDependenciesLock(t, c)
+	verifyBomStripped(t, c.Files)
+}
+
 func TestLoadV1(t *testing.T) {
 	l, err := Loader("testdata/frobnitz.v1")
 	if err != nil {
@@ -465,3 +546,11 @@ func verifyChartFileAndTemplate(t *testing.T, c *chart.Chart, name string) {
 		}
 	}
 }
+
+func verifyBomStripped(t *testing.T, files []*chart.File) {
+	for _, file := range files {
+		if bytes.HasPrefix(file.Data, utf8bom) {
+			t.Errorf("Byte Order Mark still present in processed file %s", file.Name)
+		}
+	}
+}

pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore

@@ -0,0 +1 @@
+ignore/

pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock

@@ -0,0 +1,8 @@
+dependencies:
+  - name: alpine
+    version: "0.1.0"
+    repository: https://example.com/charts
+  - name: mariner
+    version: "4.3.2"
+    repository: https://example.com/charts
+digest: invalid

pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml

@@ -0,0 +1,27 @@
+apiVersion: v1
+name: frobnitz
+description: This is a frobnitz.
+version: "1.2.3"
+keywords:
+  - frobnitz
+  - sprocket
+  - dodad
+maintainers:
+  - name: The Helm Team
+    email: helm@example.com
+  - name: Someone Else
+    email: nobody@example.com
+sources:
+  - https://example.com/foo/bar
+home: http://example.com
+icon: https://example.com/64x64.png
+annotations:
+  extrakey: extravalue
+  anotherkey: anothervalue
+dependencies:
+  - name: alpine
+    version: "0.1.0"
+    repository: https://example.com/charts
+  - name: mariner
+    version: "4.3.2"
+    repository: https://example.com/charts

pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt

@@ -0,0 +1 @@
+This is an install document. The client may display this.

pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE

@@ -0,0 +1 @@
+LICENSE placeholder.

pkg/chart/loader/testdata/frobnitz_with_bom/README.md

@@ -0,0 +1,11 @@
+# Frobnitz
+
+This is an example chart.
+
+## Usage
+
+This is an example. It has no usage.
+
+## Development
+
+For developer info, see the top-level repository.

pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me

@@ -0,0 +1 @@
+This should be ignored by the loader, but may be included in a chart.

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+name: alpine
+description: Deploy a basic Alpine Linux pod
+version: 0.1.0
+home: https://helm.sh/helm

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md

@@ -0,0 +1,9 @@
+This example was generated using the command `helm create alpine`.
+
+The `templates/` directory contains a very simple pod resource with a
+couple of parameters.
+
+The `values.toml` file contains the default values for the
+`alpine-pod.yaml` template.
+
+You can install this example using `helm install ./alpine`.

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+name: mast1
+description: A Helm chart for Kubernetes
+version: 0.1.0
+home: ""

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml

@@ -0,0 +1,4 @@
+# Default values for mast1.
+# This is a YAML-formatted file.
+# Declare name/value pairs to be passed into your templates.
+# name = "value"

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml

@@ -0,0 +1,14 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: {{.Release.Name}}-{{.Chart.Name}}
+  labels:
+    app.kubernetes.io/managed-by: {{.Release.Service}}
+    app.kubernetes.io/name: {{.Chart.Name}}
+    helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}"
+spec:
+  restartPolicy: {{default "Never" .restart_policy}}
+  containers:
+  - name: waiter
+    image: "alpine:3.9"
+    command: ["/bin/sleep","9000"]

pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml

@@ -0,0 +1,2 @@
+# The pod name
+name: "my-alpine"

pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md

@@ -0,0 +1 @@
+This is a placeholder for documentation.

pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg

@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<svg xmlns:svg="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg"
+    xmlns:xlink="http://www.w3.org/1999/xlink"
+    version="1.0" width="256" height="256" id="test">
+  <desc>Example icon</desc>
+  <rect id="first" x="2" y="2" width="40" height="60" fill="navy"/>
+  <rect id="second" x="15" y="4" width="40" height="60" fill="red"/>
+</svg>

pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl

@@ -0,0 +1 @@
+Hello {{.Name | default "world"}}

pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml

@@ -0,0 +1,6 @@
+# A values file contains configuration.
+
+name: "Some Name"
+
+section:
+  name: "Name in a section"