Skip to content

Commit

Permalink
fix issue with non utf8 encodings and environment variable expansion (#…
Browse files Browse the repository at this point in the history
…5515)

* Fix issue with encoding not being utf8, since envar shoves utf8 into an utfX file. Now we convert all to utf8.

* Fix linting and test on linux.

* update changelog

* update go.mod

* update go.sum

* Allow strict usage of utf8.

* fix spelling

* reorganize to prevent cyclic dependencies
  • Loading branch information
mattdurham authored Oct 17, 2023
1 parent 549dc33 commit 49b569e
Show file tree
Hide file tree
Showing 14 changed files with 119 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ Main (unreleased)

- Fixed issue where adding a module after initial start, that failed to load then subsequently resolving the issue would cause the module to
permanently fail to load with `id already exists` error. (@mattdurham)

- Allow the usage of encodings other than UTF8 to be used with environment variable expansion. (@mattdurham)

### Enhancements

Expand Down
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ require (
github.com/denverdino/aliyungo v0.0.0-20190125010748-a747050bb1ba // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/digitalocean/godo v1.99.0 // indirect
github.com/dimchansky/utfbom v1.1.1 // indirect
github.com/dimchansky/utfbom v1.1.1
github.com/docker/cli v23.0.3+incompatible // indirect
github.com/docker/distribution v2.8.2+incompatible // indirect
github.com/docker/go-units v0.5.0 // indirect
Expand Down Expand Up @@ -703,6 +703,8 @@ replace (
// and eventually remove if windows_exporter shifts to it. https://github.com/leoluk/perflib_exporter/pull/43
github.com/leoluk/perflib_exporter => github.com/grafana/perflib_exporter v0.1.1-0.20230511173423-6166026bd090

github.com/prometheus-community/postgres_exporter => github.com/grafana/postgres_exporter v0.8.1-0.20210722175051-db35d7c2f520

// TODO(mattdurham): this is to allow defaults to propogate properly: https://github.com/prometheus-community/windows_exporter/pull/1227
github.com/prometheus-community/windows_exporter => github.com/grafana/windows_exporter v0.15.1-0.20230612134738-fdb3ba7accd8

Expand All @@ -711,8 +713,6 @@ replace (

// TODO(marctc, mattdurham): Replace node_export with custom fork for multi usage. https://github.com/prometheus/node_exporter/pull/2812
github.com/prometheus/node_exporter => github.com/grafana/node_exporter v0.18.1-grafana-r01.0.20231004161416-702318429731

github.com/prometheus-community/postgres_exporter => github.com/grafana/postgres_exporter v0.8.1-0.20210722175051-db35d7c2f520
)

// Excluding fixes a conflict in test packages and allows "go mod tidy" to run.
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -3067,8 +3067,6 @@ google.golang.org/grpc v1.39.0/go.mod h1:PImNr+rS9TWYb2O4/emRugxiyHZ5JyHW5F+RPnD
google.golang.org/grpc v1.39.1/go.mod h1:PImNr+rS9TWYb2O4/emRugxiyHZ5JyHW5F+RPnDzfrE=
google.golang.org/grpc v1.40.0/go.mod h1:ogyxbiOoUXAkP+4+xa6PZSE9DZgIHtSpzjDTB9KAK34=
google.golang.org/grpc v1.41.0/go.mod h1:U3l9uK9J0sini8mHphKoXyaqDA/8VyGnDee1zzIUK6k=
google.golang.org/grpc v1.58.0 h1:32JY8YpPMSR45K+c3o6b8VL73V+rR8k+DeMIr4vRH8o=
google.golang.org/grpc v1.58.0/go.mod h1:tgX3ZQDlNJGU96V6yHh1T/JeoBQ2TXdr43YbYSsCJk0=
google.golang.org/grpc v1.58.3 h1:BjnpXut1btbtgN/6sp+brB2Kbm2LjNXnidYujAVbSoQ=
google.golang.org/grpc v1.58.3/go.mod h1:tgX3ZQDlNJGU96V6yHh1T/JeoBQ2TXdr43YbYSsCJk0=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw=
Expand Down
12 changes: 7 additions & 5 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/grafana/agent/pkg/build"
"github.com/grafana/agent/pkg/config/encoder"
"github.com/grafana/agent/pkg/config/features"
"github.com/grafana/agent/pkg/config/instrumentation"
"github.com/grafana/agent/pkg/logs"
Expand Down Expand Up @@ -245,13 +246,10 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) {
// LoadFile reads a file and passes the contents to Load
func LoadFile(filename string, expandEnvVars bool, c *Config) error {
buf, err := os.ReadFile(filename)

if err != nil {
return fmt.Errorf("error reading config file %w", err)
}

instrumentation.InstrumentConfig(buf)

return LoadBytes(buf, expandEnvVars, c)
}

Expand Down Expand Up @@ -341,15 +339,19 @@ func LoadRemote(url string, expandEnvVars bool, c *Config) error {
}

func performEnvVarExpansion(buf []byte, expandEnvVars bool) ([]byte, error) {
utf8Buf, err := encoder.EnsureUTF8(buf, false)
if err != nil {
return nil, err
}
// (Optionally) expand with environment variables
if expandEnvVars {
s, err := envsubst.Eval(string(buf), getenv)
s, err := envsubst.Eval(string(utf8Buf), getenv)
if err != nil {
return nil, fmt.Errorf("unable to substitute config with environment variables: %w", err)
}
return []byte(s), nil
}
return buf, nil
return utf8Buf, nil
}

// LoadBytes unmarshals a config from a buffer. Defaults are not
Expand Down
50 changes: 46 additions & 4 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,24 @@ package config

import (
"flag"

"net/url"
"os"
"path"
"strings"
"testing"
"time"

commonCfg "github.com/prometheus/common/config"

"github.com/stretchr/testify/assert"

"github.com/grafana/agent/pkg/config/encoder"
"github.com/grafana/agent/pkg/metrics"
"github.com/grafana/agent/pkg/metrics/instance"
"github.com/grafana/agent/pkg/server"
"github.com/grafana/agent/pkg/util"
commonCfg "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
promCfg "github.com/prometheus/prometheus/config"
"github.com/prometheus/prometheus/model/labels"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"
)
Expand Down Expand Up @@ -545,3 +547,43 @@ integrations:
require.NoError(t, LoadBytes([]byte(input), false, &cfg))
require.NoError(t, cfg.Validate(nil))
}

func TestConfigEncoding(t *testing.T) {
type testCase struct {
filename string
success bool
}
cases := []testCase{
{filename: "test_encoding_unknown.txt", success: false},
{filename: "test_encoding_utf8.txt", success: true},
{filename: "test_encoding_utf8bom.txt", success: true},
{filename: "test_encoding_utf16le.txt", success: true},
{filename: "test_encoding_utf16be.txt", success: true},
{filename: "test_encoding_utf32be.txt", success: true},
{filename: "test_encoding_utf32le.txt", success: true},
}
for _, tt := range cases {
t.Run(tt.filename, func(t *testing.T) {
buf, err := os.ReadFile(path.Join("encoder", tt.filename))
t.Setenv("TEST", "debug")
require.NoError(t, err)
c := &Config{}
err = LoadBytes(buf, true, c)
if tt.success {
require.NoError(t, err)
require.True(t, c.Server.LogLevel.String() == "debug")
} else {
require.Error(t, err)
}
})
}
}

func TestConfigEncodingStrict(t *testing.T) {
buf, err := os.ReadFile(path.Join("encoder", "test_encoding_utf16le.txt"))
require.NoError(t, err)
_, err = encoder.EnsureUTF8(buf, false)
require.NoError(t, err)
_, err = encoder.EnsureUTF8(buf, true)
require.Error(t, err)
}
50 changes: 50 additions & 0 deletions pkg/config/encoder/encoder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package encoder

import (
"bytes"
"fmt"
"io"
"unicode/utf8"

"github.com/dimchansky/utfbom"
"golang.org/x/text/encoding"
uni "golang.org/x/text/encoding/unicode"
"golang.org/x/text/encoding/unicode/utf32"
)

// EnsureUTF8 will convert from the most common encodings to UTF8.
// If useStrictUTF8 is enabled then if the file is not already utf8 then an error will be returned.
func EnsureUTF8(config []byte, useStrictUTF8 bool) ([]byte, error) {
buffer := bytes.NewBuffer(config)
src, enc := utfbom.Skip(buffer)
var converted []byte
skippedBytes, err := io.ReadAll(src)
if err != nil {
return nil, err
}
var encoder encoding.Encoding
switch enc {
case utfbom.UTF16BigEndian:
encoder = uni.UTF16(uni.BigEndian, uni.IgnoreBOM)
case utfbom.UTF16LittleEndian:
encoder = uni.UTF16(uni.LittleEndian, uni.IgnoreBOM)
case utfbom.UTF32BigEndian:
encoder = utf32.UTF32(utf32.BigEndian, utf32.IgnoreBOM)
case utfbom.UTF32LittleEndian:
encoder = utf32.UTF32(utf32.LittleEndian, utf32.IgnoreBOM)
case utfbom.UTF8: // This only checks utf8 bom
return config, nil
default:
// If its utf8 valid then return.
if utf8.Valid(config) {
return config, nil
}
return nil, fmt.Errorf("unknown encoding for config")
}
if useStrictUTF8 {
return nil, fmt.Errorf("configuration is encoded with %s but must be utf8", enc.String())
}
decoder := encoder.NewDecoder()
converted, err = decoder.Bytes(skippedBytes)
return converted, err
}
2 changes: 2 additions & 0 deletions pkg/config/encoder/test_encoding_unknown.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ñ:
log_level: ${TESTz�߭�߭��
Binary file added pkg/config/encoder/test_encoding_utf16be.txt
Binary file not shown.
Binary file added pkg/config/encoder/test_encoding_utf16le.txt
Binary file not shown.
Binary file added pkg/config/encoder/test_encoding_utf32be.txt
Binary file not shown.
Binary file added pkg/config/encoder/test_encoding_utf32le.txt
Binary file not shown.
2 changes: 2 additions & 0 deletions pkg/config/encoder/test_encoding_utf8.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
server:
log_level: ${TEST}
2 changes: 2 additions & 0 deletions pkg/config/encoder/test_encoding_utf8bom.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
server:
log_level: ${TEST}
5 changes: 5 additions & 0 deletions pkg/flow/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sort"
"strings"

"github.com/grafana/agent/pkg/config/encoder"
"github.com/grafana/river/ast"
"github.com/grafana/river/diag"
"github.com/grafana/river/parser"
Expand All @@ -27,6 +28,10 @@ type Source struct {
//
// bb must not be modified after passing to ParseSource.
func ParseSource(name string, bb []byte) (*Source, error) {
bb, err := encoder.EnsureUTF8(bb, true)
if err != nil {
return nil, err
}
node, err := parser.ParseFile(name, bb)
if err != nil {
return nil, err
Expand Down

0 comments on commit 49b569e

Please sign in to comment.