actually follow the spec for java "properties" format

* filter out various "interactive shell" variables
* refactor to get the effective environment

9 files changed, 140 insertions, 32 deletions

diff --git a/client/go/jvm/application_container.go b/client/go/jvm/application_container.go
index e1ce45f890c..1cbc8877aa3 100644
--- a/client/go/jvm/application_container.go
+++ b/client/go/jvm/application_container.go
@@ -53,11 +53,7 @@ func (a *ApplicationContainer) addJdiscProperties() {
 	propsFile := fmt.Sprintf("%s/%s.properties", containerHomeDir, "jdisc")
 	opts.fixSpec.FixDir(containerHomeDir)
 	opts.fixSpec.FixDir(bCacheDir)
-	trace.Trace("write props file:", propsFile)
-	err := os.WriteFile(propsFile, selectedEnv(), 0600)
-	if err != nil {
-		util.JustExitWith(err)
-	}
+	a.propsFile = propsFile
 	opts.AddOption("-Djdisc.config.file=" + propsFile)
 	opts.AddOption("-Djdisc.cache.path=" + bCacheDir)
 	opts.AddOption("-Djdisc.logger.tag=" + cfgId)
diff --git a/client/go/jvm/container.go b/client/go/jvm/container.go
index 3a66a2d37c4..53ff9f9a809 100644
--- a/client/go/jvm/container.go
+++ b/client/go/jvm/container.go
@@ -26,6 +26,7 @@ type containerBase struct {
 	configId    string
 	serviceName string
 	jvmArgs     *Options
+	propsFile   string
 }
 
 func (cb *containerBase) ServiceName() string {
@@ -67,6 +68,7 @@ func (cb *containerBase) Exec() {
 	p := prog.NewSpec(argv)
 	p.ConfigureNumaCtl()
 	cb.JvmOptions().exportEnvSettings(p)
+	writeEnvAsProperties(p.EffectiveEnv(), cb.propsFile)
 	trace.Info("starting container; env:", readableEnv(p.Env))
 	trace.Info("starting container; exec:", argv)
 	err := p.Run()
diff --git a/client/go/jvm/jdisc_options.go b/client/go/jvm/jdisc_options.go
index 32cfb840336..84348bbda63 100644
--- a/client/go/jvm/jdisc_options.go
+++ b/client/go/jvm/jdisc_options.go
@@ -4,10 +4,6 @@
 package jvm
 
 import (
-	"bytes"
-	"os"
-	"strings"
-
 	"github.com/vespa-engine/vespa/client/go/defaults"
 )
 
@@ -16,19 +12,6 @@ const (
 	DEFAULT_CP_FILE    = "lib/jars/jdisc_core-jar-with-dependencies.jar"
 )
 
-func selectedEnv() []byte {
-	var buf bytes.Buffer
-	for _, vv := range os.Environ() {
-		varName := strings.Split(vv, "=")[0]
-		if strings.Contains(vv, "\\u") || strings.Contains(vv, "\n") || strings.Contains(varName, "%%") {
-			continue
-		}
-		buf.WriteString(vv)
-		buf.WriteString("\n")
-	}
-	return buf.Bytes()
-}
-
 func (opts *Options) AddCommonJdiscProperties() {
 	logCtlDir := defaults.UnderVespaHome("var/db/vespa/logcontrol")
 	opts.fixSpec.FixDir(logCtlDir)
diff --git a/client/go/jvm/mem_avail_test.go b/client/go/jvm/mem_avail_test.go
index 34366759523..e85f2876d3a 100644
--- a/client/go/jvm/mem_avail_test.go
+++ b/client/go/jvm/mem_avail_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func TestCg2Get(t *testing.T) {
-	trace.AdjustVerbosity(2)
+	trace.AdjustVerbosity(0)
 	const MM = "memory.max"
 	res, err := vespa_cg2get(MM)
 
diff --git a/client/go/jvm/properties.go b/client/go/jvm/properties.go
new file mode 100644
index 00000000000..6d944c82c2a
--- /dev/null
+++ b/client/go/jvm/properties.go
@@ -0,0 +1,106 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Author: arnej
+
+package jvm
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"sort"
+	"strings"
+
+	"github.com/vespa-engine/vespa/client/go/trace"
+	"github.com/vespa-engine/vespa/client/go/util"
+)
+
+// quote as specified in JDK source file java.base/share/classes/java/util/Properties.java
+func propQuote(s string, buf *bytes.Buffer) {
+	inKey := true
+	inVal := false
+	for _, ch := range s {
+		needQ := false
+		switch {
+		case ch == ' ':
+			needQ = !inVal
+		case ch == ':':
+			needQ = inKey
+		case ch < ' ':
+			needQ = true
+		case ch > '~':
+			needQ = true
+		case ch == '\\':
+			buf.WriteString("\\")
+		case ch == '=':
+			inKey = false
+		default:
+			inVal = !inKey
+		}
+		if needQ {
+			fmt.Fprintf(buf, "\\u%04X", ch)
+		} else {
+			buf.WriteRune(ch)
+		}
+	}
+}
+
+func envAsProperties(envv []string) []byte {
+	suppress := map[string]bool{
+		"_":                      true,
+		"HISTCONTROL":            true,
+		"HISTSIZE":               true,
+		"IFS":                    true,
+		"LESSOPEN":               true,
+		"LOADEDMODULES":          true,
+		"LS_COLORS":              true,
+		"MAIL":                   true,
+		"MODULEPATH":             true,
+		"MODULEPATH_modshare":    true,
+		"MODULESHOME":            true,
+		"MODULES_CMD":            true,
+		"MODULES_RUN_QUARANTINE": true,
+		"OLDPWD":                 true,
+		"PCP_DIR":                true,
+		"PS1":                    true,
+		"PWD":                    true,
+		"SHLVL":                  true,
+		"SSH_AUTH_SOCK":          true,
+		"SSH_CLIENT":             true,
+		"SSH_CONNECTION":         true,
+		"SSH_TTY":                true,
+		"S_COLORS":               true,
+		"which_declare":          true,
+		"":                       true,
+	}
+	var buf bytes.Buffer
+	buf.WriteString("# properties converted from environment variables\n")
+	sort.Strings(envv)
+	for _, env := range envv {
+		parts := strings.Split(env, "=")
+		if len(parts) >= 2 {
+			varName := parts[0]
+			if suppress[varName] || strings.Contains(varName, "%%") {
+				continue
+			}
+			if strings.Contains(env, "\n") {
+				continue
+			}
+			propQuote(env, &buf)
+			buf.WriteRune('\n')
+		} else {
+			trace.Warning("environment value without '=':", env)
+		}
+	}
+	return buf.Bytes()
+}
+
+func writeEnvAsProperties(envv []string, propsFile string) {
+	if propsFile == "" {
+		panic("missing propsFile")
+	}
+	trace.Trace("write props file:", propsFile)
+	err := os.WriteFile(propsFile, envAsProperties(envv), 0600)
+	if err != nil {
+		util.JustExitWith(err)
+	}
+}
diff --git a/client/go/jvm/properties_test.go b/client/go/jvm/properties_test.go
new file mode 100644
index 00000000000..c417319a51e
--- /dev/null
+++ b/client/go/jvm/properties_test.go
@@ -0,0 +1,27 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package jvm
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSomeQuoting(t *testing.T) {
+	var buf bytes.Buffer
+	propQuote("a=b", &buf)
+	assert.Equal(t, "a=b", buf.String())
+	buf.Reset()
+	propQuote("foobar", &buf)
+	assert.Equal(t, "foobar", buf.String())
+	buf.Reset()
+	propQuote("x y = foo bar ", &buf)
+	assert.Equal(t, `x\u0020y\u0020=\u0020foo bar `, buf.String())
+	buf.Reset()
+	propQuote("x:y=foo:bar", &buf)
+	assert.Equal(t, "x\\u003Ay=foo:bar", buf.String())
+	buf.Reset()
+	propQuote(`PS1=\[\e[0;32m\]AWS-US\[\e[0m\] [\u@\[\e[1m\]\h\[\e[0m\]:\w]$ `, &buf)
+	assert.Equal(t, `PS1=\\[\\e[0;32m\\]AWS-US\\[\\e[0m\\] [\\u@\\[\\e[1m\\]\\h\\[\\e[0m\\]:\\w]$ `, buf.String())
+}
diff --git a/client/go/jvm/standalone_container.go b/client/go/jvm/standalone_container.go
index edaea346889..33007862f58 100644
--- a/client/go/jvm/standalone_container.go
+++ b/client/go/jvm/standalone_container.go
@@ -5,12 +5,10 @@ package jvm
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/vespa-engine/vespa/client/go/defaults"
 	"github.com/vespa-engine/vespa/client/go/envvars"
 	"github.com/vespa-engine/vespa/client/go/prog"
-	"github.com/vespa-engine/vespa/client/go/trace"
 	"github.com/vespa-engine/vespa/client/go/util"
 )
 
@@ -70,11 +68,7 @@ func (a *StandaloneContainer) addJdiscProperties() {
 	opts.fixSpec.FixDir(containerParentDir)
 	opts.fixSpec.FixDir(bCacheParentDir)
 	opts.fixSpec.FixDir(bCacheDir)
-	trace.Trace("write props file:", propsFile)
-	err := os.WriteFile(propsFile, selectedEnv(), 0600)
-	if err != nil {
-		util.JustExitWith(err)
-	}
+	a.propsFile = propsFile
 	opts.AddOption("-Djdisc.export.packages=")
 	opts.AddOption("-Djdisc.config.file=" + propsFile)
 	opts.AddOption("-Djdisc.cache.path=" + bCacheDir)
diff --git a/client/go/prog/run.go b/client/go/prog/run.go
index 29f35837bf5..0d11cef4d30 100644
--- a/client/go/prog/run.go
+++ b/client/go/prog/run.go
@@ -21,6 +21,6 @@ func (spec *Spec) Run() error {
 	if spec.shouldUseVespaMalloc {
 		spec.Setenv(envvars.LD_PRELOAD, spec.vespaMallocPreload)
 	}
-	envv := spec.effectiveEnv()
+	envv := spec.EffectiveEnv()
 	return util.Execvpe(prog, args, envv)
 }
diff --git a/client/go/prog/spec_env.go b/client/go/prog/spec_env.go
index ffdafe7676b..b40b3f172fd 100644
--- a/client/go/prog/spec_env.go
+++ b/client/go/prog/spec_env.go
@@ -21,7 +21,7 @@ func (p *Spec) Getenv(k string) string {
 	return os.Getenv(k)
 }
 
-func (spec *Spec) effectiveEnv() []string {
+func (spec *Spec) EffectiveEnv() []string {
 	envMap := make(map[string]string)
 	addToMap := func(kv string) {
 		for idx, elem := range kv {