From 0af06c3bd9ff26ed569cc2e2e96f092aaf2cf8f3 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Thu, 2 Mar 2023 17:54:33 +0300 Subject: [PATCH] [TrueCloudLab#40] Add param to configure xml decoder This parameter enables parsing xml body without xmlns="http://s3.amazonaws.com/doc/2006-03-01/" attribute for CompleteMultipartUpload requests Signed-off-by: Denis Kirillov --- CHANGELOG.md | 1 + api/handler/api.go | 7 +++ api/handler/handlers_test.go | 9 +++- api/handler/multipart_upload.go | 2 +- cmd/s3-gw/app.go | 14 ++++-- cmd/s3-gw/app_settings.go | 6 +++ config/config.env | 3 ++ config/config.yaml | 4 ++ docs/configuration.md | 14 ++++++ go.mod | 1 - internal/xml/decoder.go | 38 +++++++++++++++ internal/xml/decoder_test.go | 83 +++++++++++++++++++++++++++++++++ 12 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 internal/xml/decoder.go create mode 100644 internal/xml/decoder_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b796126a..ab427bd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This document outlines major changes between releases. - Multiple configs support (TrueCloudLab#21) - Bucket name resolving policy (TrueCloudLab#25) - Support string `Action` and `Resource` fields in `bucketPolicy.Statement` (TrueCloudLab#32) +- Add new `kludge.use_default_xmlns_for_complete_multipart` config param (TrueCloudLab#40) ### Changed - Update neo-go to v0.101.0 (#14) diff --git a/api/handler/api.go b/api/handler/api.go index 3981aa78..45b4b394 100644 --- a/api/handler/api.go +++ b/api/handler/api.go @@ -1,7 +1,9 @@ package handler import ( + "encoding/xml" "errors" + "io" "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" @@ -26,6 +28,7 @@ type ( // Config contains data which handler needs to keep. Config struct { Policy PlacementPolicy + XMLDecoder XMLDecoderProvider DefaultMaxAge int NotificatorEnabled bool CopiesNumber uint32 @@ -37,6 +40,10 @@ type ( Default() netmap.PlacementPolicy Get(string) (netmap.PlacementPolicy, bool) } + + XMLDecoderProvider interface { + NewCompleteMultipartDecoder(io.Reader) *xml.Decoder + } ) const ( diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index ff64b243..1f7d0a0d 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -63,6 +63,12 @@ func (p *placementPolicyMock) Get(string) (netmap.PlacementPolicy, bool) { return netmap.PlacementPolicy{}, false } +type xmlDecoderProviderMock struct{} + +func (p *xmlDecoderProviderMock) NewCompleteMultipartDecoder(r io.Reader) *xml.Decoder { + return xml.NewDecoder(r) +} + func prepareHandlerContext(t *testing.T) *handlerContext { key, err := keys.NewPrivateKey() require.NoError(t, err) @@ -93,7 +99,8 @@ func prepareHandlerContext(t *testing.T) *handlerContext { log: l, obj: layer.NewLayer(l, tp, layerCfg), cfg: &Config{ - Policy: &placementPolicyMock{defaultPolicy: pp}, + Policy: &placementPolicyMock{defaultPolicy: pp}, + XMLDecoder: &xmlDecoderProviderMock{}, }, } diff --git a/api/handler/multipart_upload.go b/api/handler/multipart_upload.go index 6b2fd551..e291b446 100644 --- a/api/handler/multipart_upload.go +++ b/api/handler/multipart_upload.go @@ -384,7 +384,7 @@ func (h *handler) CompleteMultipartUploadHandler(w http.ResponseWriter, r *http. ) reqBody := new(CompleteMultipartUpload) - if err = xml.NewDecoder(r.Body).Decode(reqBody); err != nil { + if err = h.cfg.XMLDecoder.NewCompleteMultipartDecoder(r.Body).Decode(reqBody); err != nil { h.logAndSendError(w, "could not read complete multipart upload xml", reqInfo, errors.GetAPIError(errors.ErrMalformedXML), additional...) return diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index 85a27960..bcbb65d4 100644 --- a/cmd/s3-gw/app.go +++ b/cmd/s3-gw/app.go @@ -23,6 +23,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/version" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/wallet" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/xml" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/metrics" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool" @@ -57,8 +58,9 @@ type ( } appSettings struct { - logLevel zap.AtomicLevel - policies *placementPolicy + logLevel zap.AtomicLevel + policies *placementPolicy + xmlDecoder *xml.DecoderProvider } Logger struct { @@ -152,8 +154,9 @@ func newAppSettings(log *Logger, v *viper.Viper) *appSettings { } return &appSettings{ - logLevel: log.lvl, - policies: policies, + logLevel: log.lvl, + policies: policies, + xmlDecoder: xml.NewDecoderProvider(v.GetBool(cfgKludgeUseDefaultXMLNSForCompleteMultipartUpload)), } } @@ -457,6 +460,8 @@ func (a *App) updateSettings() { if err := a.settings.policies.update(getDefaultPolicyValue(a.cfg), a.cfg.GetString(cfgPolicyRegionMapFile)); err != nil { a.log.Warn("policies won't be updated", zap.Error(err)) } + + a.settings.xmlDecoder.UseDefaultNamespaceForCompleteMultipart(a.cfg.GetBool(cfgKludgeUseDefaultXMLNSForCompleteMultipartUpload)) } func (a *App) startServices() { @@ -625,6 +630,7 @@ func (a *App) initHandler() { DefaultMaxAge: handler.DefaultMaxAge, NotificatorEnabled: a.cfg.GetBool(cfgEnableNATS), CopiesNumber: handler.DefaultCopiesNumber, + XMLDecoder: a.settings.xmlDecoder, } if a.cfg.IsSet(cfgDefaultMaxAge) { diff --git a/cmd/s3-gw/app_settings.go b/cmd/s3-gw/app_settings.go index a8fd9125..470abfa0 100644 --- a/cmd/s3-gw/app_settings.go +++ b/cmd/s3-gw/app_settings.go @@ -113,6 +113,9 @@ const ( // Settings. // Application. cfgApplicationBuildTime = "app.build_time" + // Kludge. + cfgKludgeUseDefaultXMLNSForCompleteMultipartUpload = "kludge.use_default_xmlns_for_complete_multipart" + // Command line args. cmdHelp = "help" cmdVersion = "version" @@ -253,6 +256,9 @@ func newSettings() *viper.Viper { v.SetDefault(cfgPProfAddress, "localhost:8085") v.SetDefault(cfgPrometheusAddress, "localhost:8086") + // kludge + v.SetDefault(cfgKludgeUseDefaultXMLNSForCompleteMultipartUpload, false) + // Bind flags if err := bindFlags(v, flags); err != nil { panic(fmt.Errorf("bind flags: %w", err)) diff --git a/config/config.env b/config/config.env index 5f65f840..b62452c2 100644 --- a/config/config.env +++ b/config/config.env @@ -127,3 +127,6 @@ S3_GW_ALLOWED_ACCESS_KEY_ID_PREFIXES=Ck9BHsgKcnwfCTUSFm6pxhoNS4cBqgN2NQ8zVgPjqZD # List of container NNS zones which are allowed or restricted to resolve with HEAD request S3_GW_RESOLVE_BUCKET_ALLOW=container # S3_GW_RESOLVE_BUCKET_DENY= + +# Enable using default xml namespace `http://s3.amazonaws.com/doc/2006-03-01/` when parse`CompleteMultipartUpload` xml body. +S3_GW_KLUDGE_USE_DEFAULT_XMLNS_FOR_COMPLETE_MULTIPART=false diff --git a/config/config.yaml b/config/config.yaml index 1a535d43..ccc631e2 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -149,3 +149,7 @@ resolve_bucket: allow: - container deny: + +kludge: + # Enable using default xml namespace `http://s3.amazonaws.com/doc/2006-03-01/` when parse`CompleteMultipartUpload` xml body. + use_default_xmlns_for_complete_multipart: false diff --git a/docs/configuration.md b/docs/configuration.md index 32e5a190..4bc4c463 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -184,6 +184,7 @@ There are some custom types used for brevity: | `prometheus` | [Prometheus configuration](#prometheus-section) | | `frostfs` | [Parameters of requests to FrostFS](#frostfs-section) | | `resolve_bucket` | [Bucket name resolving configuration](#resolve_bucket-section) | +| `kludge` | [Different kludge configuration](#kludge-section) | ### General section @@ -495,3 +496,16 @@ resolve_bucket: |-----------|------------|---------------|--------------------------------------------------------------------------------------------------------------------------| | `allow` | `[]string` | | List of container zones which are available to resolve. Mutual exclusive with `deny` list. Prioritized over `deny` list. | | `deny` | `[]string` | | List of container zones which are restricted to resolve. Mutual exclusive with `allow` list. | + +# `kludge` section + +Workarounds for non-standard use cases. + +```yaml +kludge: + use_default_xmlns_for_complete_multipart: false +``` + +| Parameter | Type | SIGHUP reload | Default value | Description | +|--------------------------------------------|--------|---------------|---------------|-----------------------------------------------------------------------------------------------------------------------------| +| `use_default_xmlns_for_complete_multipart` | `bool` | yes | false | Enable using default xml namespace `http://s3.amazonaws.com/doc/2006-03-01/` when parse `CompleteMultipartUpload` xml body. | diff --git a/go.mod b/go.mod index 97fc75f0..68d52cf9 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,6 @@ require ( require ( github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20221202181307-76fa05c21b12 // indirect - //github.com/aws/aws-sdk-go-v2 v1.16.7 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.1.2 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect diff --git a/internal/xml/decoder.go b/internal/xml/decoder.go new file mode 100644 index 00000000..0066c080 --- /dev/null +++ b/internal/xml/decoder.go @@ -0,0 +1,38 @@ +package xml + +import ( + "encoding/xml" + "io" + "sync" +) + +const awsDefaultNamespace = "http://s3.amazonaws.com/doc/2006-03-01/" + +type DecoderProvider struct { + mu sync.RWMutex + defaultXMLNSForCompleteMultipart bool +} + +func NewDecoderProvider(defaultNamespace bool) *DecoderProvider { + return &DecoderProvider{ + defaultXMLNSForCompleteMultipart: defaultNamespace, + } +} + +func (d *DecoderProvider) NewCompleteMultipartDecoder(r io.Reader) *xml.Decoder { + dec := xml.NewDecoder(r) + + d.mu.RLock() + if d.defaultXMLNSForCompleteMultipart { + dec.DefaultSpace = awsDefaultNamespace + } + d.mu.RUnlock() + + return dec +} + +func (d *DecoderProvider) UseDefaultNamespaceForCompleteMultipart(useDefaultNamespace bool) { + d.mu.Lock() + d.defaultXMLNSForCompleteMultipart = useDefaultNamespace + d.mu.Unlock() +} diff --git a/internal/xml/decoder_test.go b/internal/xml/decoder_test.go new file mode 100644 index 00000000..98326928 --- /dev/null +++ b/internal/xml/decoder_test.go @@ -0,0 +1,83 @@ +package xml + +import ( + "bytes" + "testing" + + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/handler" + "github.com/stretchr/testify/require" +) + +func TestDefaultNamespace(t *testing.T) { + xmlBodyWithNamespace := ` + + + string + 1 + + +` + xmlBodyWithInvalidNamespace := ` + + + string + 1 + + +` + xmlBody := ` + + + string + 1 + + +` + + for _, tc := range []struct { + provider *DecoderProvider + input string + err bool + }{ + { + provider: NewDecoderProvider(false), + input: xmlBodyWithNamespace, + err: false, + }, + { + provider: NewDecoderProvider(false), + input: xmlBody, + err: true, + }, + { + provider: NewDecoderProvider(false), + input: xmlBodyWithInvalidNamespace, + err: true, + }, + { + provider: NewDecoderProvider(true), + input: xmlBodyWithNamespace, + err: false, + }, + { + provider: NewDecoderProvider(true), + input: xmlBody, + err: false, + }, + { + provider: NewDecoderProvider(true), + input: xmlBodyWithInvalidNamespace, + err: true, + }, + } { + t.Run("", func(t *testing.T) { + model := new(handler.CompleteMultipartUpload) + err := tc.provider.NewCompleteMultipartDecoder(bytes.NewBufferString(tc.input)).Decode(model) + if tc.err { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +}