[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 <d.kirillov@yadro.com>

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)

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 (

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{},
 		},
 	}
 

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

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) {

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))

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

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

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. |

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

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()
+}

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 := `
+ <CompleteMultipartUpload xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
+   <Part>
+      <ETag>string</ETag>
+      <PartNumber>1</PartNumber>
+   </Part>
+</CompleteMultipartUpload>
+`
+	xmlBodyWithInvalidNamespace := `
+ <CompleteMultipartUpload xmlns="http://bla.bla.bla/">
+   <Part>
+      <ETag>string</ETag>
+      <PartNumber>1</PartNumber>
+   </Part>
+</CompleteMultipartUpload>
+`
+	xmlBody := `
+ <CompleteMultipartUpload>
+   <Part>
+      <ETag>string</ETag>
+      <PartNumber>1</PartNumber>
+   </Part>
+</CompleteMultipartUpload>
+`
+
+	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)
+			}
+		})
+	}
+}