From 8902561f3cdc8f606b8cec208b02d5ff1df3c524 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 23 Aug 2022 17:04:11 -0700 Subject: [PATCH] use new xxml for XML responses to support rare control characters (#15511) use new xxml/XML responses to support rare control characters fixes #15023 --- buildscripts/unaligned-healing.sh | 4 +- cmd/api-headers.go | 13 ++- cmd/api-response.go | 126 +++++++++++++++++++++--------- cmd/object-handlers.go | 28 ++++++- cmd/server_test.go | 76 ++++++++++++++++++ go.mod | 1 + go.sum | 2 + 7 files changed, 203 insertions(+), 47 deletions(-) diff --git a/buildscripts/unaligned-healing.sh b/buildscripts/unaligned-healing.sh index 01a8d2482..fc2e5ea4d 100755 --- a/buildscripts/unaligned-healing.sh +++ b/buildscripts/unaligned-healing.sh @@ -120,7 +120,7 @@ function start_minio_16drive() { cat "${WORK_DIR}/server1.log" echo "FAILED" mkdir -p inspects - (cd inspects; "${WORK_DIR}/mc" admin inspect minio/healing-shard-bucket/unaligned/**) + (cd inspects; "${WORK_DIR}/mc" support inspect minio/healing-shard-bucket/unaligned/**) "${WORK_DIR}/mc" mb play/inspects "${WORK_DIR}/mc" mirror inspects play/inspects @@ -140,7 +140,7 @@ function start_minio_16drive() { cat "${WORK_DIR}/server1.log" echo "FAILED" mkdir -p inspects - (cd inspects; "${WORK_DIR}/mc" admin inspect minio/healing-shard-bucket/unaligned/**) + (cd inspects; "${WORK_DIR}/mc" support inspect minio/healing-shard-bucket/unaligned/**) "${WORK_DIR}/mc" mb play/inspects "${WORK_DIR}/mc" mirror inspects play/inspects diff --git a/cmd/api-headers.go b/cmd/api-headers.go index 60cd14f14..599b7a0fc 100644 --- a/cmd/api-headers.go +++ b/cmd/api-headers.go @@ -20,7 +20,6 @@ package cmd import ( "bytes" "encoding/json" - "encoding/xml" "fmt" "net/http" "net/url" @@ -30,6 +29,8 @@ import ( "github.com/minio/minio/internal/crypto" xhttp "github.com/minio/minio/internal/http" + "github.com/minio/minio/internal/logger" + xxml "github.com/minio/xxml" ) // Returns a hexadecimal representation of time at the @@ -64,9 +65,13 @@ func setCommonHeaders(w http.ResponseWriter) { // Encodes the response headers into XML format. func encodeResponse(response interface{}) []byte { var bytesBuffer bytes.Buffer - bytesBuffer.WriteString(xml.Header) - e := xml.NewEncoder(&bytesBuffer) - e.Encode(response) + bytesBuffer.WriteString(xxml.Header) + buf, err := xxml.Marshal(response) + if err != nil { + logger.LogIf(GlobalContext, err) + return nil + } + bytesBuffer.Write(buf) return bytesBuffer.Bytes() } diff --git a/cmd/api-response.go b/cmd/api-response.go index f39a0ed3d..2026342f7 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -89,7 +89,8 @@ type ListVersionsResponse struct { IsTruncated bool CommonPrefixes []CommonPrefix - Versions []ObjectVersion + DeleteMarkers []DeleteMarkerVersion `xml:"DeleteMarker,omitempty"` + Versions []ObjectVersion `xml:"Version,omitempty"` // Encoding type used to encode object keys in the response. EncodingType string `xml:"EncodingType,omitempty"` @@ -247,50 +248,80 @@ type ObjectVersion struct { Object IsLatest bool VersionID string `xml:"VersionId"` - - isDeleteMarker bool } -// MarshalXML - marshal ObjectVersion -func (o ObjectVersion) MarshalXML(e *xml.Encoder, start xml.StartElement) error { - if o.isDeleteMarker { - start.Name.Local = "DeleteMarker" - } else { - start.Name.Local = "Version" +// DeleteMarkerVersion container for delete marker metadata +type DeleteMarkerVersion struct { + Key string + LastModified string // time string of format "2006-01-02T15:04:05.000Z" + + // Owner of the object. + Owner Owner + + IsLatest bool + VersionID string `xml:"VersionId"` +} + +// Metadata metadata items implemented to ensure XML marshaling works. +type Metadata struct { + Items []struct { + Key string + Value string } - - type objectVersionWrapper ObjectVersion - return e.EncodeElement(objectVersionWrapper(o), start) } -// StringMap is a map[string]string -type StringMap map[string]string +// Set add items, duplicate items get replaced. +func (s *Metadata) Set(k, v string) { + for i, item := range s.Items { + if item.Key == k { + s.Items[i] = struct { + Key string + Value string + }{ + Key: k, + Value: v, + } + return + } + } + s.Items = append(s.Items, struct { + Key string + Value string + }{ + Key: k, + Value: v, + }) +} + +type xmlKeyEntry struct { + XMLName xml.Name + Value string `xml:",chardata"` +} // MarshalXML - StringMap marshals into XML. -func (s StringMap) MarshalXML(e *xml.Encoder, start xml.StartElement) error { - tokens := []xml.Token{start} - - for key, value := range s { - t := xml.StartElement{} - t.Name = xml.Name{ - Space: "", - Local: key, - } - tokens = append(tokens, t, xml.CharData(value), xml.EndElement{Name: t.Name}) +func (s *Metadata) MarshalXML(e *xml.Encoder, start xml.StartElement) error { + if s == nil { + return nil } - tokens = append(tokens, xml.EndElement{ - Name: start.Name, - }) + if len(s.Items) == 0 { + return nil + } - for _, t := range tokens { - if err := e.EncodeToken(t); err != nil { + if err := e.EncodeToken(start); err != nil { + return err + } + + for _, item := range s.Items { + if err := e.Encode(xmlKeyEntry{ + XMLName: xml.Name{Local: item.Key}, + Value: item.Value, + }); err != nil { return err } } - // flush to ensure tokens are written - return e.Flush() + return e.EncodeToken(start.End()) } // Object container for object metadata @@ -307,7 +338,7 @@ type Object struct { StorageClass string // UserMetadata user-defined metadata - UserMetadata StringMap `xml:"UserMetadata,omitempty"` + UserMetadata *Metadata `xml:"UserMetadata,omitempty"` } // CopyObjectResponse container returns ETag and LastModified of the successfully copied object @@ -438,6 +469,8 @@ func generateListBucketsResponse(buckets []BucketInfo) ListBucketsResponse { // generates an ListBucketVersions response for the said bucket with other enumerated options. func generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delimiter, encodingType string, maxKeys int, resp ListObjectVersionsInfo) ListVersionsResponse { versions := make([]ObjectVersion, 0, len(resp.Objects)) + deleteMarkers := make([]DeleteMarkerVersion, 0, len(resp.Objects)) + owner := Owner{ ID: globalMinioDefaultOwnerID, DisplayName: "minio", @@ -445,10 +478,25 @@ func generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delim data := ListVersionsResponse{} for _, object := range resp.Objects { - content := ObjectVersion{} if object.Name == "" { continue } + + if object.DeleteMarker { + deleteMarker := DeleteMarkerVersion{ + Key: s3EncodeName(object.Name, encodingType), + LastModified: object.ModTime.UTC().Format(iso8601TimeFormat), + Owner: owner, + VersionID: object.VersionID, + } + if deleteMarker.VersionID == "" { + deleteMarker.VersionID = nullVersionID + } + deleteMarker.IsLatest = object.IsLatest + deleteMarkers = append(deleteMarkers, deleteMarker) + continue + } + content := ObjectVersion{} content.Key = s3EncodeName(object.Name, encodingType) content.LastModified = object.ModTime.UTC().Format(iso8601TimeFormat) if object.ETag != "" { @@ -466,12 +514,12 @@ func generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delim content.VersionID = nullVersionID } content.IsLatest = object.IsLatest - content.isDeleteMarker = object.DeleteMarker versions = append(versions, content) } data.Name = bucket data.Versions = versions + data.DeleteMarkers = deleteMarkers data.EncodingType = encodingType data.Prefix = s3EncodeName(prefix, encodingType) data.KeyMarker = s3EncodeName(marker, encodingType) @@ -569,14 +617,14 @@ func generateListObjectsV2Response(bucket, prefix, token, nextToken, startAfter, } content.Owner = owner if metadata { - content.UserMetadata = make(StringMap) + content.UserMetadata = &Metadata{} switch kind, _ := crypto.IsEncrypted(object.UserDefined); kind { case crypto.S3: - content.UserMetadata[xhttp.AmzServerSideEncryption] = xhttp.AmzEncryptionAES + content.UserMetadata.Set(xhttp.AmzServerSideEncryption, xhttp.AmzEncryptionAES) case crypto.S3KMS: - content.UserMetadata[xhttp.AmzServerSideEncryption] = xhttp.AmzEncryptionKMS + content.UserMetadata.Set(xhttp.AmzServerSideEncryption, xhttp.AmzEncryptionKMS) case crypto.SSEC: - content.UserMetadata[xhttp.AmzServerSideEncryptionCustomerAlgorithm] = xhttp.AmzEncryptionAES + content.UserMetadata.Set(xhttp.AmzServerSideEncryptionCustomerAlgorithm, xhttp.AmzEncryptionAES) } for k, v := range CleanMinioInternalMetadataKeys(object.UserDefined) { if strings.HasPrefix(strings.ToLower(k), ReservedMetadataPrefixLower) { @@ -588,7 +636,7 @@ func generateListObjectsV2Response(bucket, prefix, token, nextToken, startAfter, if equals(k, xhttp.AmzMetaUnencryptedContentLength, xhttp.AmzMetaUnencryptedContentMD5) { continue } - content.UserMetadata[k] = v + content.UserMetadata.Set(k, v) } } contents = append(contents, content) diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 610baec3a..c86ea292a 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -20,6 +20,7 @@ package cmd import ( "context" "encoding/hex" + "encoding/xml" "errors" "fmt" "io" @@ -3066,6 +3067,16 @@ func (api objectAPIHandlers) GetObjectRetentionHandler(w http.ResponseWriter, r }) } +// ObjectTagSet key value tags +type ObjectTagSet struct { + Tags []tags.Tag `xml:"Tag"` +} + +type objectTagging struct { + XMLName xml.Name `xml:"Tagging"` + TagSet *ObjectTagSet `xml:"TagSet"` +} + // GetObjectTaggingHandler - GET object tagging func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "GetObjectTagging") @@ -3103,7 +3114,7 @@ func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *h } // Get object tags - tags, err := objAPI.GetObjectTags(ctx, bucket, object, opts) + ot, err := objAPI.GetObjectTags(ctx, bucket, object, opts) if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return @@ -3113,7 +3124,20 @@ func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *h w.Header()[xhttp.AmzVersionID] = []string{opts.VersionID} } - writeSuccessResponseXML(w, encodeResponse(tags)) + otags := &objectTagging{ + TagSet: &ObjectTagSet{}, + } + + var list []tags.Tag + for k, v := range ot.ToMap() { + list = append(list, tags.Tag{ + Key: k, + Value: v, + }) + } + otags.TagSet.Tags = list + + writeSuccessResponseXML(w, encodeResponse(otags)) } // PutObjectTaggingHandler - PUT object tagging diff --git a/cmd/server_test.go b/cmd/server_test.go index dd2ebef84..d79c383a4 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -28,6 +28,7 @@ import ( "net/http" "net/url" "reflect" + "runtime" "strings" "sync" "testing" @@ -1636,6 +1637,81 @@ func (s *TestSuiteCommon) TestListObjectsHandler(c *check) { } } +// TestListObjectsSpecialCharactersHandler - Setting valid parameters to List Objects +// and then asserting the response with the expected one. +func (s *TestSuiteCommon) TestListObjectsSpecialCharactersHandler(c *check) { + if runtime.GOOS == globalWindowsOSName { + c.Skip("skip special character test for windows") + } + + // generate a random bucket name. + bucketName := getRandomBucketName() + // HTTP request to create the bucket. + request, err := newTestSignedRequest(http.MethodPut, getMakeBucketURL(s.endPoint, bucketName), + 0, nil, s.accessKey, s.secretKey, s.signer) + c.Assert(err, nil) + + // execute the HTTP request to create bucket. + response, err := s.client.Do(request) + c.Assert(err, nil) + c.Assert(response.StatusCode, http.StatusOK) + + for _, objectName := range []string{"foo bar 1", "foo bar 2", "foo \x01 bar"} { + buffer := bytes.NewReader([]byte("Hello World")) + request, err = newTestSignedRequest(http.MethodPut, getPutObjectURL(s.endPoint, bucketName, objectName), + int64(buffer.Len()), buffer, s.accessKey, s.secretKey, s.signer) + c.Assert(err, nil) + + response, err = s.client.Do(request) + c.Assert(err, nil) + c.Assert(response.StatusCode, http.StatusOK) + } + + testCases := []struct { + getURL string + expectedStrings []string + }{ + {getListObjectsV1URL(s.endPoint, bucketName, "", "1000", ""), []string{"foo bar 1", "foo bar 2", "foo  bar"}}, + {getListObjectsV1URL(s.endPoint, bucketName, "", "1000", "url"), []string{"foo+bar+1", "foo+bar+2", "foo+%01+bar"}}, + { + getListObjectsV2URL(s.endPoint, bucketName, "", "1000", "", ""), + []string{ + "foo bar 1", + "foo bar 2", + "foo  bar", + fmt.Sprintf("%sminio", globalMinioDefaultOwnerID), + }, + }, + { + getListObjectsV2URL(s.endPoint, bucketName, "", "1000", "true", ""), + []string{ + "foo bar 1", + "foo bar 2", + "foo  bar", + fmt.Sprintf("%sminio", globalMinioDefaultOwnerID), + }, + }, + {getListObjectsV2URL(s.endPoint, bucketName, "", "1000", "", "url"), []string{"foo+bar+1", "foo+bar+2", "foo+%01+bar"}}, + } + + for _, testCase := range testCases { + // create listObjectsV1 request with valid parameters + request, err = newTestSignedRequest(http.MethodGet, testCase.getURL, 0, nil, s.accessKey, s.secretKey, s.signer) + c.Assert(err, nil) + // execute the HTTP request. + response, err = s.client.Do(request) + c.Assert(err, nil) + c.Assert(response.StatusCode, http.StatusOK) + + getContent, err := ioutil.ReadAll(response.Body) + c.Assert(err, nil) + + for _, expectedStr := range testCase.expectedStrings { + c.Assert(strings.Contains(string(getContent), expectedStr), true) + } + } +} + // TestListObjectsHandlerErrors - Setting invalid parameters to List Objects // and then asserting the error response with the expected one. func (s *TestSuiteCommon) TestListObjectsHandlerErrors(c *check) { diff --git a/go.mod b/go.mod index 8b501520e..f5daa2ad0 100644 --- a/go.mod +++ b/go.mod @@ -55,6 +55,7 @@ require ( github.com/minio/sha256-simd v1.0.0 github.com/minio/simdjson-go v0.4.2 github.com/minio/sio v0.3.0 + github.com/minio/xxml v0.0.3 github.com/minio/zipindex v0.2.1 github.com/mitchellh/go-homedir v1.1.0 github.com/nats-io/nats-server/v2 v2.7.4 diff --git a/go.sum b/go.sum index ad3cf087a..8eb992ab8 100644 --- a/go.sum +++ b/go.sum @@ -645,6 +645,8 @@ github.com/minio/simdjson-go v0.4.2 h1:+5rVznvslY/oLIGOO0M7y2g0D8DnFbFZyF2Nr+KT5 github.com/minio/simdjson-go v0.4.2/go.mod h1:w6ptSSCcUPtqAOOdDVywJkX+8CyYzCdjrTCir0l6BVg= github.com/minio/sio v0.3.0 h1:syEFBewzOMOYVzSTFpp1MqpSZk8rUNbz8VIIc+PNzus= github.com/minio/sio v0.3.0/go.mod h1:8b0yPp2avGThviy/+OCJBI6OMpvxoUuiLvE6F1lebhw= +github.com/minio/xxml v0.0.3 h1:ZIpPQpfyG5uZQnqqC0LZuWtPk/WT8G/qkxvO6jb7zMU= +github.com/minio/xxml v0.0.3/go.mod h1:wcXErosl6IezQIMEWSK/LYC2VS7LJ1dAkgvuyIN3aH4= github.com/minio/zipindex v0.2.1 h1:A37vDQJ7Uyp4RHpQEEpintgiIxg0t3npH2CWjLT//u4= github.com/minio/zipindex v0.2.1/go.mod h1:s+b/Qyw9JtSEnYfaM4ASOWNO2xGnXCfzQ+SWAzVkVZc= github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc=