From 75321cf733ceb11d52d0be64510d9fdaeebe7bdf Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Mon, 13 Jan 2025 14:59:41 -0600 Subject: [PATCH 1/5] Add backwards compatible changes to ParsePath for extra behaviors --- parseutil/parsepath.go | 72 +++++++++++++++++++++++++++++++------ parseutil/parsepath_test.go | 39 +++++++++++++++++--- 2 files changed, 96 insertions(+), 15 deletions(-) diff --git a/parseutil/parsepath.go b/parseutil/parsepath.go index ec81236..98f606c 100644 --- a/parseutil/parsepath.go +++ b/parseutil/parsepath.go @@ -17,6 +17,15 @@ var ( ErrNotParsed = errors.New("not a parsed value") ) +type Options struct { + errorOnMissingEnv bool + noTrimSpaces bool +} + +type Option func() OptionFunc + +type OptionFunc func(*Options) + // ParsePath parses a URL with schemes file://, env://, or any other. Depending // on the scheme it will return specific types of data: // @@ -34,31 +43,56 @@ var ( // step that errored or something else (such as a file not found). This is // useful to attempt to read a non-URL string from some resource, but where the // original input may simply be a valid string of that type. -func ParsePath(path string) (string, error) { - return parsePath(path, false) +func ParsePath(path string, options ...Option) (string, error) { + return parsePath(path, false, options) } // MustParsePath behaves like ParsePath but will return ErrNotAUrl if the value // is not a URL with a scheme that can be parsed by this function. -func MustParsePath(path string) (string, error) { - return parsePath(path, true) +func MustParsePath(path string, options ...Option) (string, error) { + return parsePath(path, true, options) } -func parsePath(path string, mustParse bool) (string, error) { - path = strings.TrimSpace(path) - parsed, err := url.Parse(path) +func parsePath(path string, mustParse bool, options []Option) (string, error) { + var opts Options + for _, o := range options { + of := o() + of(&opts) + } + + trimmedPath := strings.TrimSpace(path) + parsed, err := url.Parse(trimmedPath) if err != nil { - return path, fmt.Errorf("error parsing url (%q): %w", err.Error(), ErrNotAUrl) + return trimmedPath, fmt.Errorf("error parsing url (%q): %w", err.Error(), ErrNotAUrl) } switch parsed.Scheme { case "file": - contents, err := ioutil.ReadFile(strings.TrimPrefix(path, "file://")) + contents, err := ioutil.ReadFile(strings.TrimPrefix(trimmedPath, "file://")) if err != nil { - return path, fmt.Errorf("error reading file at %s: %w", path, err) + return trimmedPath, fmt.Errorf("error reading file at %s: %w", trimmedPath, err) + } + if opts.noTrimSpaces { + return string(contents), nil } return strings.TrimSpace(string(contents)), nil case "env": - return strings.TrimSpace(os.Getenv(strings.TrimPrefix(path, "env://"))), nil + envKey := strings.TrimPrefix(trimmedPath, "env://") + envVal, ok := os.LookupEnv(envKey) + if opts.errorOnMissingEnv && !ok { + return "", fmt.Errorf("environment variable %s unset", envKey) + } + if opts.noTrimSpaces { + return envVal, nil + } + return strings.TrimSpace(envVal), nil + case "string": + // Meant if there is a need to provide a string literal that is prefixed by one of these URL schemes but want to "escape" it, + // e.g. "string://env://foo", in order to get the value "env://foo" + val := strings.TrimPrefix(trimmedPath, "string://") + if opts.noTrimSpaces { + return val, nil + } + return strings.TrimSpace(val), nil default: if mustParse { return "", ErrNotParsed @@ -66,3 +100,19 @@ func parsePath(path string, mustParse bool) (string, error) { return path, nil } } + +func WithNoTrimSpaces(noTrim bool) Option { + return func() OptionFunc { + return OptionFunc(func(o *Options) { + o.noTrimSpaces = noTrim + }) + } +} + +func WithErrorOnMissingEnv(errorOnMissingEnv bool) Option { + return func() OptionFunc { + return OptionFunc(func(o *Options) { + o.errorOnMissingEnv = errorOnMissingEnv + }) + } +} diff --git a/parseutil/parsepath_test.go b/parseutil/parsepath_test.go index baa9935..b20feb0 100644 --- a/parseutil/parsepath_test.go +++ b/parseutil/parsepath_test.go @@ -18,12 +18,12 @@ func TestParsePath(t *testing.T) { file, err := os.CreateTemp("", "") require.NoError(t, err) - _, err = file.WriteString("foo") + _, err = file.WriteString(" foo ") require.NoError(t, err) require.NoError(t, file.Close()) defer os.Remove(file.Name()) - require.NoError(t, os.Setenv("PATHTEST", "bar")) + require.NoError(t, os.Setenv("PATHTEST", " bar ")) cases := []struct { name string @@ -33,12 +33,19 @@ func TestParsePath(t *testing.T) { must bool notParsed bool expErrorContains string + options []Option }{ { name: "file", inPath: fmt.Sprintf("file://%s", file.Name()), outStr: "foo", }, + { + name: "file-untrimmed", + inPath: fmt.Sprintf("file://%s", file.Name()), + outStr: " foo ", + options: []Option{WithNoTrimSpaces(true)}, + }, { name: "file-mustparse", inPath: fmt.Sprintf("file://%s", file.Name()), @@ -50,17 +57,36 @@ func TestParsePath(t *testing.T) { inPath: "env://PATHTEST", outStr: "bar", }, + { + name: "env-untrimmed", + inPath: "env://PATHTEST", + outStr: " bar ", + options: []Option{WithNoTrimSpaces(true)}, + }, { name: "env-mustparse", inPath: "env://PATHTEST", outStr: "bar", must: true, }, + { + name: "env-error-missing", + inPath: "env://PATHTEST2", + outStr: "bar", + expErrorContains: "environment variable PATHTEST2 unset", + options: []Option{WithErrorOnMissingEnv(true)}, + }, { name: "plain", inPath: "zipzap", outStr: "zipzap", }, + { + name: "plan-untrimmed", + inPath: " zipzap ", + outStr: " zipzap ", + options: []Option{WithNoTrimSpaces(true)}, + }, { name: "plain-mustparse", inPath: "zipzap", @@ -68,6 +94,11 @@ func TestParsePath(t *testing.T) { must: true, notParsed: true, }, + { + name: "escaped", + inPath: "string://env://foo", + outStr: "env://foo", + }, { name: "no file", inPath: "file:///dev/nullface", @@ -88,9 +119,9 @@ func TestParsePath(t *testing.T) { var err error switch tt.must { case false: - out, err = ParsePath(tt.inPath) + out, err = ParsePath(tt.inPath, tt.options...) default: - out, err = MustParsePath(tt.inPath) + out, err = MustParsePath(tt.inPath, tt.options...) } if tt.expErrorContains != "" { require.Error(err) From 727f8533ed2ff88c5c3de52df386b4d7eb8c3031 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Mon, 13 Jan 2025 15:08:08 -0600 Subject: [PATCH 2/5] No need to export these --- listenerutil/parse.go | 4 ++-- nonceutil/encrypted_nonce.go | 2 ++ parseutil/parsepath.go | 30 ++++++++++++++++-------------- parseutil/parsepath_test.go | 10 +++++----- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/listenerutil/parse.go b/listenerutil/parse.go index a0bb4d5..9ac932e 100644 --- a/listenerutil/parse.go +++ b/listenerutil/parse.go @@ -440,14 +440,14 @@ const defaultUiContentSecurityPolicyHeader = "default-src 'none'; script-src 'se // Header names consts const contentSecurityPolicy = "Content-Security-Policy" const strictTransportSecurity = "Strict-Transport-Security" -const xContentTypeOptions = "X-Content-Type-Options" +const xContentTypeOptions = "X-Content-Type-options" const cacheControl = "Cache-Control" // parseCustomResponseHeaders takes raw config values for the "custom_ui_response_headers" // and "custom_api_response_headers". It makes sure the config entry is passed in as a map // of status code to a map of header name and header values. It verifies the validity of the // status codes, and header values. It also adds the default headers values for "Cache-Control", -// "Strict-Transport-Security", "X-Content-Type-Options", and "Content-Security-Policy". +// "Strict-Transport-Security", "X-Content-Type-options", and "Content-Security-Policy". // Supported options: // - WithDefaultUiContentSecurityPolicyHeader func parseCustomResponseHeaders(responseHeaders interface{}, uiHeaders bool, opt ...Option) (map[int]http.Header, error) { diff --git a/nonceutil/encrypted_nonce.go b/nonceutil/encrypted_nonce.go index afa1489..cde5946 100644 --- a/nonceutil/encrypted_nonce.go +++ b/nonceutil/encrypted_nonce.go @@ -28,6 +28,7 @@ import ( "encoding/base64" "encoding/binary" "fmt" + "github.com/hashicorp/go-secure-stdlib/parseutil" "io" "sort" "sync" @@ -93,6 +94,7 @@ type encryptedNonceService struct { } func newEncryptedNonceService(validity time.Duration) *encryptedNonceService { + parseutil.ParsePath("foo") return &encryptedNonceService{ validity: validity, diff --git a/parseutil/parsepath.go b/parseutil/parsepath.go index 98f606c..b631dbe 100644 --- a/parseutil/parsepath.go +++ b/parseutil/parsepath.go @@ -17,14 +17,14 @@ var ( ErrNotParsed = errors.New("not a parsed value") ) -type Options struct { +type options struct { errorOnMissingEnv bool noTrimSpaces bool } -type Option func() OptionFunc +type option func() optionFunc -type OptionFunc func(*Options) +type optionFunc func(*options) // ParsePath parses a URL with schemes file://, env://, or any other. Depending // on the scheme it will return specific types of data: @@ -43,19 +43,19 @@ type OptionFunc func(*Options) // step that errored or something else (such as a file not found). This is // useful to attempt to read a non-URL string from some resource, but where the // original input may simply be a valid string of that type. -func ParsePath(path string, options ...Option) (string, error) { +func ParsePath(path string, options ...option) (string, error) { return parsePath(path, false, options) } // MustParsePath behaves like ParsePath but will return ErrNotAUrl if the value // is not a URL with a scheme that can be parsed by this function. -func MustParsePath(path string, options ...Option) (string, error) { +func MustParsePath(path string, options ...option) (string, error) { return parsePath(path, true, options) } -func parsePath(path string, mustParse bool, options []Option) (string, error) { - var opts Options - for _, o := range options { +func parsePath(path string, mustParse bool, passedOptions []option) (string, error) { + var opts options + for _, o := range passedOptions { of := o() of(&opts) } @@ -101,17 +101,19 @@ func parsePath(path string, mustParse bool, options []Option) (string, error) { } } -func WithNoTrimSpaces(noTrim bool) Option { - return func() OptionFunc { - return OptionFunc(func(o *Options) { +// When true, values returned from ParsePath won't have leading/trailing spaces trimmed. +func WithNoTrimSpaces(noTrim bool) option { + return func() optionFunc { + return optionFunc(func(o *options) { o.noTrimSpaces = noTrim }) } } -func WithErrorOnMissingEnv(errorOnMissingEnv bool) Option { - return func() OptionFunc { - return OptionFunc(func(o *Options) { +// When true, if an environment variable is unset, an error will be returned rather than the empty string. +func WithErrorOnMissingEnv(errorOnMissingEnv bool) option { + return func() optionFunc { + return optionFunc(func(o *options) { o.errorOnMissingEnv = errorOnMissingEnv }) } diff --git a/parseutil/parsepath_test.go b/parseutil/parsepath_test.go index b20feb0..df4e0c0 100644 --- a/parseutil/parsepath_test.go +++ b/parseutil/parsepath_test.go @@ -33,7 +33,7 @@ func TestParsePath(t *testing.T) { must bool notParsed bool expErrorContains string - options []Option + options []option }{ { name: "file", @@ -44,7 +44,7 @@ func TestParsePath(t *testing.T) { name: "file-untrimmed", inPath: fmt.Sprintf("file://%s", file.Name()), outStr: " foo ", - options: []Option{WithNoTrimSpaces(true)}, + options: []option{WithNoTrimSpaces(true)}, }, { name: "file-mustparse", @@ -61,7 +61,7 @@ func TestParsePath(t *testing.T) { name: "env-untrimmed", inPath: "env://PATHTEST", outStr: " bar ", - options: []Option{WithNoTrimSpaces(true)}, + options: []option{WithNoTrimSpaces(true)}, }, { name: "env-mustparse", @@ -74,7 +74,7 @@ func TestParsePath(t *testing.T) { inPath: "env://PATHTEST2", outStr: "bar", expErrorContains: "environment variable PATHTEST2 unset", - options: []Option{WithErrorOnMissingEnv(true)}, + options: []option{WithErrorOnMissingEnv(true)}, }, { name: "plain", @@ -85,7 +85,7 @@ func TestParsePath(t *testing.T) { name: "plan-untrimmed", inPath: " zipzap ", outStr: " zipzap ", - options: []Option{WithNoTrimSpaces(true)}, + options: []option{WithNoTrimSpaces(true)}, }, { name: "plain-mustparse", From 0ea9c1816f9926b88820cc78d6ec6622296675b3 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Mon, 13 Jan 2025 15:10:09 -0600 Subject: [PATCH 3/5] overzealous search/replace --- listenerutil/parse.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/listenerutil/parse.go b/listenerutil/parse.go index 9ac932e..a0bb4d5 100644 --- a/listenerutil/parse.go +++ b/listenerutil/parse.go @@ -440,14 +440,14 @@ const defaultUiContentSecurityPolicyHeader = "default-src 'none'; script-src 'se // Header names consts const contentSecurityPolicy = "Content-Security-Policy" const strictTransportSecurity = "Strict-Transport-Security" -const xContentTypeOptions = "X-Content-Type-options" +const xContentTypeOptions = "X-Content-Type-Options" const cacheControl = "Cache-Control" // parseCustomResponseHeaders takes raw config values for the "custom_ui_response_headers" // and "custom_api_response_headers". It makes sure the config entry is passed in as a map // of status code to a map of header name and header values. It verifies the validity of the // status codes, and header values. It also adds the default headers values for "Cache-Control", -// "Strict-Transport-Security", "X-Content-Type-options", and "Content-Security-Policy". +// "Strict-Transport-Security", "X-Content-Type-Options", and "Content-Security-Policy". // Supported options: // - WithDefaultUiContentSecurityPolicyHeader func parseCustomResponseHeaders(responseHeaders interface{}, uiHeaders bool, opt ...Option) (map[int]http.Header, error) { From e0f8a3489629fd44384390b607311e31a21b13b3 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Mon, 13 Jan 2025 15:10:30 -0600 Subject: [PATCH 4/5] remove test code --- nonceutil/encrypted_nonce.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/nonceutil/encrypted_nonce.go b/nonceutil/encrypted_nonce.go index cde5946..afa1489 100644 --- a/nonceutil/encrypted_nonce.go +++ b/nonceutil/encrypted_nonce.go @@ -28,7 +28,6 @@ import ( "encoding/base64" "encoding/binary" "fmt" - "github.com/hashicorp/go-secure-stdlib/parseutil" "io" "sort" "sync" @@ -94,7 +93,6 @@ type encryptedNonceService struct { } func newEncryptedNonceService(validity time.Duration) *encryptedNonceService { - parseutil.ParsePath("foo") return &encryptedNonceService{ validity: validity, From 9be4eaaf3d174cd5f36cd22d44fa03f0ade3b6f0 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Tue, 14 Jan 2025 13:00:36 -0600 Subject: [PATCH 5/5] handle trim in the ErrNotAURL case --- parseutil/parsepath.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/parseutil/parsepath.go b/parseutil/parsepath.go index b631dbe..46ccbe7 100644 --- a/parseutil/parsepath.go +++ b/parseutil/parsepath.go @@ -63,7 +63,11 @@ func parsePath(path string, mustParse bool, passedOptions []option) (string, err trimmedPath := strings.TrimSpace(path) parsed, err := url.Parse(trimmedPath) if err != nil { - return trimmedPath, fmt.Errorf("error parsing url (%q): %w", err.Error(), ErrNotAUrl) + err = fmt.Errorf("error parsing url (%q): %w", err.Error(), ErrNotAUrl) + if opts.noTrimSpaces { + return path, err + } + return trimmedPath, err } switch parsed.Scheme { case "file":