From db050217865173fb5cce961c9ed745a7940c4496 Mon Sep 17 00:00:00 2001 From: Pavel Pogodaev Date: Wed, 24 Apr 2024 15:20:11 +0300 Subject: [PATCH] [#379] Add Iana CharsetReader for Oracle integration Signed-off-by: Pavel Pogodaev --- CHANGELOG.md | 1 + api/handler/acl.go | 4 ++-- api/handler/delete.go | 3 ++- api/handler/multipart_upload.go | 2 +- api/handler/put.go | 5 +++-- api/middleware/policy.go | 2 +- cmd/s3-gw/app.go | 8 ++++++++ cmd/s3-gw/decoder_test.go | 16 ++++++++++++++++ go.mod | 2 +- 9 files changed, 35 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88eb3b8cc..b690177b4 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/api/handler/acl.go b/api/handler/acl.go index 96d676ebf..bf7dd8e8d 100644 --- a/api/handler/acl.go +++ b/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 } diff --git a/api/handler/delete.go b/api/handler/delete.go index dd02c24a5..1de3833a0 100644 --- a/api/handler/delete.go +++ b/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 } diff --git a/api/handler/multipart_upload.go b/api/handler/multipart_upload.go index 72f76892d..76bf7871d 100644 --- a/api/handler/multipart_upload.go +++ b/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 { diff --git a/api/handler/put.go b/api/handler/put.go index 28d66356d..830dfe3eb 100644 --- a/api/handler/put.go +++ b/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 } diff --git a/api/middleware/policy.go b/api/middleware/policy.go index 44fc5b21e..d3ffc093f 100644 --- a/api/middleware/policy.go +++ b/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 diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index 32cddd471..f2bbf8768 100644 --- a/cmd/s3-gw/app.go +++ b/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 { diff --git a/cmd/s3-gw/decoder_test.go b/cmd/s3-gw/decoder_test.go index 9325840de..93827e6c5 100644 --- a/cmd/s3-gw/decoder_test.go +++ b/cmd/s3-gw/decoder_test.go @@ -34,6 +34,15 @@ func TestDefaultNamespace(t *testing.T) { ` + xmlASCII := ` + + + 1 + + 8b73814bee405ec32b5d1fc88cd5d97a + + +` 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) diff --git a/go.mod b/go.mod index b17e58c6e..77c80ab49 100644 --- a/go.mod +++ b/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