CHANGELOG.md

@@ -12,6 +12,7 @@ This document outlines major changes between releases.
 - Fix flaky `TestErrorTimeoutChecking` (`make test` sometimes failed) (#290)
 - Fix user owner ID in billing metrics (#321)
 - Fix HTTP/2 requests (#341)
+- Fix Decoder.CharsetReader is nil (#379)
 
 ### Added
 - Add new `frostfs.buffer_max_size_for_put` config param and sync TZ hash for PUT operations (#197)

api/handler/acl.go

@@ -399,7 +399,7 @@ func (h *handler) PutBucketACLHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	} else if err = h.cfg.NewXMLDecoder(r.Body).Decode(list); err != nil {
-		h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
+		h.logAndSendError(w, "could not parse bucket acl", reqInfo, fmt.Errorf("%w: %s", errors.GetAPIError(errors.ErrMalformedXML), err.Error()))
 		return
 	}
 
@@ -615,7 +615,7 @@ func (h *handler) PutObjectACLHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	} else if err = h.cfg.NewXMLDecoder(r.Body).Decode(list); err != nil {
-		h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
+		h.logAndSendError(w, "could not parse bucket acl", reqInfo, fmt.Errorf("%w: %s", errors.GetAPIError(errors.ErrMalformedXML), err.Error()))
 		return
 	}
 

api/handler/delete.go

@@ -2,6 +2,7 @@ package handler
 
 import (
 	"encoding/xml"
+	"fmt"
 	"net/http"
 	"strconv"
 	"strings"
@@ -179,7 +180,7 @@ func (h *handler) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *http.Re
 	// Unmarshal list of keys to be deleted.
 	requested := &DeleteObjectsRequest{}
 	if err := h.cfg.NewXMLDecoder(r.Body).Decode(requested); err != nil {
-		h.logAndSendError(w, "couldn't decode body", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
+		h.logAndSendError(w, "couldn't decode body", reqInfo, fmt.Errorf("%w: %s", errors.GetAPIError(errors.ErrMalformedXML), err.Error()))
 		return
 	}
 

api/handler/multipart_upload.go

@@ -441,7 +441,7 @@ func (h *handler) CompleteMultipartUploadHandler(w http.ResponseWriter, r *http.
 	reqBody := new(CompleteMultipartUpload)
 	if err = h.cfg.NewXMLDecoder(r.Body).Decode(reqBody); err != nil {
 		h.logAndSendError(w, "could not read complete multipart upload xml", reqInfo,
-			errors.GetAPIError(errors.ErrMalformedXML), additional...)
+			fmt.Errorf("%w: %s", errors.GetAPIError(errors.ErrMalformedXML), err.Error()), additional...)
 		return
 	}
 	if len(reqBody.Parts) == 0 {

api/handler/put.go

@@ -485,7 +485,8 @@ func (h *handler) PostObject(w http.ResponseWriter, r *http.Request) {
 		buffer := bytes.NewBufferString(tagging)
 		tags := new(data.Tagging)
 		if err = h.cfg.NewXMLDecoder(buffer).Decode(tags); err != nil {
-			h.logAndSendError(w, "could not decode tag set", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
+			h.logAndSendError(w, "could not decode tag set", reqInfo,
+				fmt.Errorf("%w: %s", errors.GetAPIError(errors.ErrMalformedXML), err.Error()))
 			return
 		}
 		tagSet, err = h.readTagSet(tags)
@@ -1211,7 +1212,7 @@ func (h *handler) parseLocationConstraint(r *http.Request) (*createBucketParams,
 
 	params := new(createBucketParams)
 	if err := h.cfg.NewXMLDecoder(r.Body).Decode(params); err != nil {
-		return nil, errors.GetAPIError(errors.ErrMalformedXML)
+		return nil, fmt.Errorf("%w: %s", errors.GetAPIError(errors.ErrMalformedXML), err.Error())
 	}
 	return params, nil
 }

api/middleware/policy.go

@@ -484,7 +484,7 @@ func determineRequestTags(r *http.Request, decoder XMLDecoder, op string) (map[s
 	if strings.HasSuffix(op, PutObjectTaggingOperation) || strings.HasSuffix(op, PutBucketTaggingOperation) {
 		tagging := new(data.Tagging)
 		if err := decoder.NewXMLDecoder(r.Body).Decode(tagging); err != nil {
-			return nil, apiErr.GetAPIError(apiErr.ErrMalformedXML)
+			return nil, fmt.Errorf("%w: %s", apiErr.GetAPIError(apiErr.ErrMalformedXML), err.Error())
 		}
 		GetReqInfo(r.Context()).Tagging = tagging
 

cmd/s3-gw/app.go

@@ -50,6 +50,7 @@ import (
 	"github.com/spf13/viper"
 	"go.uber.org/zap"
 	"golang.org/x/exp/slices"
+	"golang.org/x/text/encoding/ianaindex"
 	"google.golang.org/grpc"
 )
 
@@ -317,6 +318,13 @@ func (s *appSettings) DefaultCopiesNumbers(namespace string) []uint32 {
 
 func (s *appSettings) NewXMLDecoder(r io.Reader) *xml.Decoder {
 	dec := xml.NewDecoder(r)
+	dec.CharsetReader = func(charset string, reader io.Reader) (io.Reader, error) {
+		enc, err := ianaindex.IANA.Encoding(charset)
+		if err != nil {
+			return nil, fmt.Errorf("charset %s: %w", charset, err)
+		}
+		return enc.NewDecoder().Reader(reader), nil
+	}
 
 	s.mu.RLock()
 	if s.defaultXMLNS {

cmd/s3-gw/decoder_test.go

@@ -34,6 +34,15 @@ func TestDefaultNamespace(t *testing.T) {
    </Part>
 </CompleteMultipartUpload>
 `
+	xmlASCII := `<?xml version="1.0" encoding="US-ASCII"?>
+<CompleteMultipartUpload>
+    <Part>
+        <PartNumber>1</PartNumber>
+        <ETag>
+            8b73814bee405ec32b5d1fc88cd5d97a
+        </ETag>
+    </Part>
+</CompleteMultipartUpload>`
 
 	for _, tc := range []struct {
 		settings *appSettings
@@ -82,6 +91,13 @@ func TestDefaultNamespace(t *testing.T) {
 			input: xmlBodyWithInvalidNamespace,
 			err:   true,
 		},
+		{
+			settings: &appSettings{
+				defaultXMLNS: true,
+			},
+			input: xmlASCII,
+			err:   false,
+		},
 	} {
 		t.Run("", func(t *testing.T) {
 			model := new(handler.CompleteMultipartUpload)

go.mod

@@ -31,6 +31,7 @@ require (
 	golang.org/x/crypto v0.21.0
 	golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63
 	golang.org/x/net v0.23.0
+	golang.org/x/text v0.14.0
 	google.golang.org/grpc v1.59.0
 	google.golang.org/protobuf v1.33.0
 )
@@ -95,7 +96,6 @@ require (
 	golang.org/x/sync v0.3.0 // indirect
 	golang.org/x/sys v0.18.0 // indirect
 	golang.org/x/term v0.18.0 // indirect
-	golang.org/x/text v0.14.0 // indirect
 	google.golang.org/genproto v0.0.0-20231120223509-83a465c0220f // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20231106174013-bbf56f31fb17 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20231127180814-3a041ad873d4 // indirect