From 6b8c28bc192ec4f3b90aa7443338750a16c47b01 Mon Sep 17 00:00:00 2001 From: Kaviraj Date: Tue, 7 Feb 2023 22:22:44 +0100 Subject: [PATCH 1/3] flagext: Add new type `URLEscaped` Sometimes we need to store just escaped URL in the config like S3 URL that contains credentials which can include characters like `:` and `/`. Currently we cannot use normal `flagext.URLValue` as it just uses unescaped URL. Please check issue https://github.com/grafana/loki/issues/1607 --- flagext/urlescaped.go | 63 ++++++++++++++++++++++++++++++++++++++ flagext/urlescaped_test.go | 38 +++++++++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 flagext/urlescaped.go create mode 100644 flagext/urlescaped_test.go diff --git a/flagext/urlescaped.go b/flagext/urlescaped.go new file mode 100644 index 000000000..0bac0eb82 --- /dev/null +++ b/flagext/urlescaped.go @@ -0,0 +1,63 @@ +package flagext + +import "net/url" + +// URLEscaped is a url.URL that can be used as a flag. +// URL value it contains will always be URL escaped and safe. +type URLEscaped struct { + *url.URL +} + +// String implements flag.Value +func (v URLEscaped) String() string { + if v.URL == nil { + return "" + } + return v.URL.String() +} + +// Set implements flag.Value +// Set make sure given URL string is escaped. +func (v *URLEscaped) Set(s string) error { + s = url.QueryEscape(s) + + u, err := url.Parse(s) + if err != nil { + return err + } + v.URL = u + return nil +} + +// UnmarshalYAML implements yaml.Unmarshaler. +func (v *URLEscaped) UnmarshalYAML(unmarshal func(interface{}) error) error { + var s string + if err := unmarshal(&s); err != nil { + return err + } + + // An empty string means no URL has been configured. + if s == "" { + v.URL = nil + return nil + } + + return v.Set(s) +} + +// Marshalyaml Implements yaml.Marshaler. +func (v URLEscaped) MarshalYAML() (interface{}, error) { + if v.URL == nil { + return "", nil + } + + // Mask out passwords when marshalling URLs back to YAML. + u := *v.URL + if u.User != nil { + if _, set := u.User.Password(); set { + u.User = url.UserPassword(u.User.Username(), "********") + } + } + + return u.String(), nil +} diff --git a/flagext/urlescaped_test.go b/flagext/urlescaped_test.go new file mode 100644 index 000000000..12c0ebee5 --- /dev/null +++ b/flagext/urlescaped_test.go @@ -0,0 +1,38 @@ +package flagext + +import ( + "flag" + "fmt" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" +) + +const ( + testS3URL = "s3://ASDFGHJIQWETTYUI:Jkasduahdkjh213kj1h31+lkjaflkjzvKASDOasofhjafaKFAF/GoQd@region/bucket_name" +) + +func TestURLEscaped(t *testing.T) { + expected, err := url.Parse(url.QueryEscape(testS3URL)) + require.NoError(t, err) + + // flag + var v URLEscaped + flags := flag.NewFlagSet("test", flag.ExitOnError) + flags.Var(&v, "v", "some secret credentials") + err = flags.Parse([]string{"-v", testS3URL}) + + assert.Equal(t, v.String(), expected.String()) + + // yaml + yv := struct { + S3 URLEscaped `yaml:"s3"` + }{} + err = yaml.Unmarshal([]byte(fmt.Sprintf("s3: %s", testS3URL)), &yv) + assert.NoError(t, err) + assert.Equal(t, yv.S3.String(), expected.String()) + +} From 059a5412619c8a00c6706dd35959791cddbf30a0 Mon Sep 17 00:00:00 2001 From: Kaviraj Date: Tue, 7 Feb 2023 22:35:09 +0100 Subject: [PATCH 2/3] fix linter --- flagext/urlescaped_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flagext/urlescaped_test.go b/flagext/urlescaped_test.go index 12c0ebee5..697bf8c1c 100644 --- a/flagext/urlescaped_test.go +++ b/flagext/urlescaped_test.go @@ -24,7 +24,7 @@ func TestURLEscaped(t *testing.T) { flags := flag.NewFlagSet("test", flag.ExitOnError) flags.Var(&v, "v", "some secret credentials") err = flags.Parse([]string{"-v", testS3URL}) - + require.NoError(t, err) assert.Equal(t, v.String(), expected.String()) // yaml @@ -32,7 +32,7 @@ func TestURLEscaped(t *testing.T) { S3 URLEscaped `yaml:"s3"` }{} err = yaml.Unmarshal([]byte(fmt.Sprintf("s3: %s", testS3URL)), &yv) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, yv.S3.String(), expected.String()) } From 1a8714430dc3047fb38acba6f1ed39463e686614 Mon Sep 17 00:00:00 2001 From: Kaviraj Date: Tue, 21 Feb 2023 15:26:46 +0100 Subject: [PATCH 3/3] idk Signed-off-by: Kaviraj --- flagext/urlescaped.go | 11 ++++++++++- flagext/urlescaped_test.go | 19 +++++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/flagext/urlescaped.go b/flagext/urlescaped.go index 0bac0eb82..0865e9b80 100644 --- a/flagext/urlescaped.go +++ b/flagext/urlescaped.go @@ -17,14 +17,23 @@ func (v URLEscaped) String() string { } // Set implements flag.Value -// Set make sure given URL string is escaped. +// Set make sure given URL string is saved as escaped. func (v *URLEscaped) Set(s string) error { + // if given s is already escaped + // NOTE: unescaping already unescaped URL is no-op. + s, err := url.QueryUnescape(s) + if err != nil { + return err + } + + // now we know for sure `s` is unescaped. s = url.QueryEscape(s) u, err := url.Parse(s) if err != nil { return err } + v.URL = u return nil } diff --git a/flagext/urlescaped_test.go b/flagext/urlescaped_test.go index 697bf8c1c..7b78c96d7 100644 --- a/flagext/urlescaped_test.go +++ b/flagext/urlescaped_test.go @@ -3,7 +3,6 @@ package flagext import ( "flag" "fmt" - "net/url" "testing" "github.com/stretchr/testify/assert" @@ -12,20 +11,24 @@ import ( ) const ( - testS3URL = "s3://ASDFGHJIQWETTYUI:Jkasduahdkjh213kj1h31+lkjaflkjzvKASDOasofhjafaKFAF/GoQd@region/bucket_name" + testS3URL = "s3://ASDFGHJIQWETTYUI:Jkasduahdkjh213kj1h31+lkjaflkjzvKASDOasofhjafaKFAF/GoQd@region/bucket_name" + fromWebsite = "s3%3A%2F%2FASDFGHJIQWETTYUI%3AJkasduahdkjh213kj1h31%2BlkjaflkjzvKASDOasofhjafaKFAF%2FGoQd%40region%2Fbucket_name" + testS3URLEscaped = "s3%3A%2F%2FASDFGHJIQWETTYUI%3AJkasduahdkjh213kj1h31%2BlkjaflkjzvKASDOasofhjafaKFAF%2FGoQd%40region%2Fbucket_name" ) func TestURLEscaped(t *testing.T) { - expected, err := url.Parse(url.QueryEscape(testS3URL)) - require.NoError(t, err) - // flag var v URLEscaped flags := flag.NewFlagSet("test", flag.ExitOnError) flags.Var(&v, "v", "some secret credentials") - err = flags.Parse([]string{"-v", testS3URL}) + err := flags.Parse([]string{"-v", testS3URL}) + require.NoError(t, err) + assert.Equal(t, v.String(), testS3URLEscaped) + + // flag (but already escaped) + err = flags.Parse([]string{"-v", testS3URLEscaped}) require.NoError(t, err) - assert.Equal(t, v.String(), expected.String()) + assert.Equal(t, v.String(), testS3URLEscaped) // yaml yv := struct { @@ -33,6 +36,6 @@ func TestURLEscaped(t *testing.T) { }{} err = yaml.Unmarshal([]byte(fmt.Sprintf("s3: %s", testS3URL)), &yv) require.NoError(t, err) - assert.Equal(t, yv.S3.String(), expected.String()) + assert.Equal(t, yv.S3.String(), testS3URLEscaped) }