Repository: helm/helm (go)

Issue:
Stack overflow in pkg/chartutil/errors.go
`pkg/chartutil/errors.go` has a recursion bug which results in a stack overflow when trying to serialize the value of the error:

https://github.com/helm/helm/blob/dev-v3/pkg/chartutil/errors.go#L26

If you attempt to render one of these Errors, you'll get a stack overflow. I have proposed a unit test in #6672 which demonstrates the stack overflow.

Excerpt of logs:

```
goroutine 179 [running]:
runtime.heapBitsSetType(0xc006dd8750, 0xd0, 0xc8, 0x110f540)
	/usr/local/go/src/runtime/mbitmap.go:938 +0xa4e fp=0xc02055a3a0 sp=0xc02055a398 pc=0x41823e
runtime.mallocgc(0xd0, 0x110f540, 0x1, 0x0)
	/usr/local/go/src/runtime/malloc.go:1052 +0x53e fp=0xc02055a440 sp=0xc02055a3a0 pc=0x40dfde
runtime.newobject(0x110f540, 0xc02055a518)
	/usr/local/go/src/runtime/malloc.go:1151 +0x38 fp=0xc02055a470 sp=0xc02055a440 pc=0x40e628
fmt.glob..func1(0x1b25100, 0x0)
	/usr/local/go/src/fmt/print.go:132 +0x2d fp=0xc02055a490 sp=0xc02055a470 pc=0x4e372d
sync.(*Pool).Get(0x1b25100, 0xc000177c00, 0x7fdec8505008)
	/usr/local/go/src/sync/pool.go:148 +0xa6 fp=0xc02055a4d8 sp=0xc02055a490 pc=0x46e6a6
fmt.newPrinter(0x0)
	/usr/local/go/src/fmt/print.go:137 +0x31 fp=0xc02055a500 sp=0xc02055a4d8 pc=0x4d5471
fmt.Sprintf(0x119495e, 0x11, 0xc02055a590, 0x1, 0x1, 0x10717c0, 0x42fb0a)
	/usr/local/go/src/fmt/print.go:218 +0x26 fp=0xc02055a558 sp=0xc02055a500 pc=0x4d5a36
helm.sh/helm/v3/pkg/chartutil.ErrNoTable.Error(...)
	/home/circleci/helm.sh/helm/pkg/chartutil/errors.go:26
helm.sh/helm/v3/pkg/chartutil.(*ErrNoTable).Error(0xc006dc3af0, 0x104ea40, 0xc006dc3af0)
	<autogenerated>:1 +0xb4 fp=0xc02055a5b0 sp=0xc02055a558 pc=0xf95274
fmt.(*pp).handleMethods(0xc006dd8680, 0x71, 0xc02055a801)
	/usr/local/go/src/fmt/print.go:624 +0x1db fp=0xc02055a7c0 sp=0xc02055a5b0 pc=0x4d8aeb
fmt.(*pp).printArg(0xc006dd8680, 0x104ea40, 0xc006dc3af0, 0xc000000071)
	/usr/local/go/src/fmt/print.go:713 +0x206 fp=0xc02055a858 sp=0xc02055a7c0 pc=0x4d91d6
fmt.(*pp).doPrintf(0xc006dd8680, 0x119495e, 0x11, 0xc02055a9d0, 0x1, 0x1)
	/usr/local/go/src/fmt/print.go:1030 +0x15b fp=0xc02055a940 sp=0xc02055a858 pc=0x4dcb9b
fmt.Sprintf(0x119495e, 0x11, 0xc02055a9d0, 0x1, 0x1, 0x10717c0, 0x42fb0a)
	/usr/local/go/src/fmt/print.go:219 +0x66 fp=0xc02055a998 sp=0xc02055a940 pc=0x4d5a76
helm.sh/helm/v3/pkg/chartutil.ErrNoTable.Error(...)
	/home/circleci/helm.sh/helm/pkg/chartutil/errors.go:26
helm.sh/helm/v3/pkg/chartutil.(*ErrNoTable).Error(0xc006dc3ae0, 0x104ea40, 0xc006dc3ae0)
	<autogenerated>:1 +0xb4 fp=0xc02055a9f0 sp=0xc02055a998 pc=0xf95274
fmt.(*pp).handleMethods(0xc006dd85b0, 0x71, 0xc02055ac01)
	/usr/local/go/src/fmt/print.go:624 +0x1db fp=0xc02055ac00 sp=0xc02055a9f0 pc=0x4d8aeb
fmt.(*pp).printArg(0xc006dd85b0, 0x104ea40, 0xc006dc3ae0, 0xc000000071)
	/usr/local/go/src/fmt/print.go:713 +0x206 fp=0xc02055ac98 sp=0xc02055ac00 pc=0x4d91d6
fmt.(*pp).doPrintf(0xc006dd85b0, 0x119495e, 0x11, 0xc02055ae10, 0x1, 0x1)
	/usr/local/go/src/fmt/print.go:1030 +0x15b fp=0xc02055ad80 sp=0xc02055ac98 pc=0x4dcb9b
fmt.Sprintf(0x119495e, 0x11, 0xc02055ae10, 0x1, 0x1, 0x10717c0, 0x42fb0a)
	/usr/local/go/src/fmt/print.go:219 +0x66 fp=0xc02055add8 sp=0xc02055ad80 pc=0x4d5a76
helm.sh/helm/v3/pkg/chartutil.ErrNoTable.Error(...)
	/home/circleci/helm.sh/helm/pkg/chartutil/errors.go:26
```

Output of `helm version`:
dev-v3 (Beta 4)

Output of `kubectl version`:
N/A

Cloud Provider/Platform (AKS, GKE, Minikube etc.): 
N/A



Pull request:
[v3] Introducing tests for chartutils/errors.go
- Should panic on string recursion error

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/master/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:
A stack overflow exists when the `ErrNoTable` or `ErrNoValue` types in `pkg/chartutil` are serialized to strings. This PR introduces failing tests which will be address as a second commit to this PR with a small refactor to not have the string recursion error.

Fixes #6673 

Signed-off-by: Ken Perkins <ken@perkins.io>

Patch:
diff --git a/pkg/chartutil/errors.go b/pkg/chartutil/errors.go
index 061610d41..fcdcc27ea 100644
--- a/pkg/chartutil/errors.go
+++ b/pkg/chartutil/errors.go
@@ -21,11 +21,15 @@ import (
 )
 
 // ErrNoTable indicates that a chart does not have a matching table.
-type ErrNoTable string
+type ErrNoTable struct {
+	Key string
+}
 
-func (e ErrNoTable) Error() string { return fmt.Sprintf("%q is not a table", e) }
+func (e ErrNoTable) Error() string { return fmt.Sprintf("%q is not a table", e.Key) }
 
 // ErrNoValue indicates that Values does not contain a key with a value
-type ErrNoValue string
+type ErrNoValue struct {
+	Key string
+}
 
-func (e ErrNoValue) Error() string { return fmt.Sprintf("%q is not a value", e) }
+func (e ErrNoValue) Error() string { return fmt.Sprintf("%q is not a value", e.Key) }
diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go
index fe9152857..a6b55701c 100644
--- a/pkg/chartutil/values.go
+++ b/pkg/chartutil/values.go
@@ -88,7 +88,7 @@ func (v Values) Encode(w io.Writer) error {
 func tableLookup(v Values, simple string) (Values, error) {
 	v2, ok := v[simple]
 	if !ok {
-		return v, ErrNoTable(simple)
+		return v, ErrNoTable{simple}
 	}
 	if vv, ok := v2.(map[string]interface{}); ok {
 		return vv, nil
@@ -101,7 +101,7 @@ func tableLookup(v Values, simple string) (Values, error) {
 		return vv, nil
 	}
 
-	return Values{}, ErrNoTable(simple)
+	return Values{}, ErrNoTable{simple}
 }
 
 // ReadValues will parse YAML byte data into a Values.
@@ -193,20 +193,20 @@ func (v Values) pathValue(path []string) (interface{}, error) {
 		if _, ok := v[path[0]]; ok && !istable(v[path[0]]) {
 			return v[path[0]], nil
 		}
-		return nil, ErrNoValue(path[0])
+		return nil, ErrNoValue{path[0]}
 	}
 
 	key, path := path[len(path)-1], path[:len(path)-1]
 	// get our table for table path
 	t, err := v.Table(joinPath(path...))
 	if err != nil {
-		return nil, ErrNoValue(key)
+		return nil, ErrNoValue{key}
 	}
 	// check table for key and ensure value is not a table
 	if k, ok := t[key]; ok && !istable(k) {
 		return k, nil
 	}
-	return nil, ErrNoValue(key)
+	return nil, ErrNoValue{key}
 }
 
 func parsePath(key string) []string { return strings.Split(key, ".") }


Test patch:
diff --git a/pkg/chartutil/errors_test.go b/pkg/chartutil/errors_test.go
new file mode 100644
index 000000000..3f9d04a71
--- /dev/null
+++ b/pkg/chartutil/errors_test.go
@@ -0,0 +1,37 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package chartutil
+
+import (
+	"testing"
+)
+
+func TestErrorNoTableDoesntPanic(t *testing.T) {
+	x := "empty"
+
+	y := ErrNoTable{x}
+
+	t.Logf("error is: %s", y)
+}
+
+func TestErrorNoValueDoesntPanic(t *testing.T) {
+	x := "empty"
+
+	y := ErrNoValue{x}
+
+	t.Logf("error is: %s", y)
+}