Skip to content
This repository was archived by the owner on Feb 13, 2026. It is now read-only.

Write xml.Header first instead of spaces to handle XML parsers#7253

Merged
nitisht merged 1 commit intominio:masterfrom
harshavardhana:fix-whitespaces
Feb 21, 2019
Merged

Write xml.Header first instead of spaces to handle XML parsers#7253
nitisht merged 1 commit intominio:masterfrom
harshavardhana:fix-whitespaces

Conversation

@harshavardhana
Copy link
Copy Markdown
Member

@harshavardhana harshavardhana commented Feb 18, 2019 •
edited
Loading

Description

Write xml.Header first instead of spaces to handle XML parsers

Motivation and Context

Clients like AWS SDK Java and AWS cli XML parsers are
unable to handle on \r\n characters to avoid these
errors send XML header first and write white space characters
instead.

Also handle cases to avoid double WriteHeader calls

Regression

In a way yes

How Has This Been Tested?

Using AWS SDK Java upload manager and AWS CLI upload

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added unit tests to cover my changes.
  • I have added/updated functional tests in mint. (If yes, add mint PR # here: )
  • All new and existing tests passed.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 18, 2019 •
edited
Loading

Codecov Report

Merging #7253 into master will decrease coverage by 0.03%.
The diff coverage is 41.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7253      +/-   ##
==========================================
- Coverage   48.49%   48.45%   -0.04%     
==========================================
  Files         295      294       -1     
  Lines       45888    45900      +12     
==========================================
- Hits        22252    22242      -10     
- Misses      21605    21622      +17     
- Partials     2031     2036       +5
Impacted Files Coverage Δ
cmd/api-response.go 82.75% <100%> (+0.06%) ⬆️
cmd/object-handlers.go 53.53% <38.63%> (-0.34%) ⬇️
cmd/bitrot-streaming.go 78.64% <0%> (-3.89%) ⬇️
cmd/os-reliable.go 65.65% <0%> (-2.03%) ⬇️
cmd/fs-v1-helpers.go 68.19% <0%> (-0.62%) ⬇️
cmd/posix-list-dir_windows.go 64.7% <0%> (+5.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3aabe45...f7ced1a. Read the comment docs.

Praveenrajmani
Praveenrajmani previously approved these changes Feb 19, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@Praveenrajmani Praveenrajmani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@minio minio deleted a comment from minio-ops Feb 19, 2019
@minio minio deleted a comment from minio-ops Feb 19, 2019
@harshavardhana harshavardhana force-pushed the fix-whitespaces branch 2 times, most recently from 33e3023 to 5fe2894 Compare February 19, 2019 18:39
@krishnasrinivas
Copy link
Copy Markdown
Contributor

krishnasrinivas commented Feb 19, 2019 •
edited
Loading

with this diff

diff --git a/cmd/fs-v1-multipart.go b/cmd/fs-v1-multipart.go
index b64541dc..63aeecd1 100644
--- a/cmd/fs-v1-multipart.go
+++ b/cmd/fs-v1-multipart.go
@@ -481,6 +481,7 @@ func (fs *FSObjects) CompleteMultipartUpload(ctx context.Context, bucket string,
 
        var actualSize int64
 
+       time.Sleep(20 * time.Second)
        if err := checkCompleteMultipartArgs(ctx, bucket, object, fs); err != nil {
                return oi, toObjectErr(err)
        }
diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go
index 8f942537..671f7da3 100644
--- a/cmd/object-handlers.go
+++ b/cmd/object-handlers.go
@@ -2167,6 +2167,7 @@ func sendWhiteSpace(ctx context.Context, w http.ResponseWriter) <-chan struct{}
                        case <-ticker.C:
                                // Write a white space char to the client to prevent client timeouts
                                // when the server takes a long time to completeMultiPartUpload()
+                               w.Write([]byte(xml.Header))
                                if _, err := w.Write([]byte("\n\r")); err != nil {
                                        logger.LogIf(ctx, err)
                                        return

i got:

krishna@escape:~$ aws --endpoint http://localhost:9000 s3 cp kvemul.tgz s3://test
upload failed: ./kvemul.tgz to s3://test/kvemul.tgz Unable to parse response (XML or text declaration not at start of entity: line 4, column 0), invalid XML received:
<?xml version="1.0" encoding="UTF-8"?>

<?xml version="1.0" encoding="UTF-8"?>

<?xml version="1.0" encoding="UTF-8"?>
<CompleteMultipartUploadResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Location>http://localhost:9000/test/kvemul.tgz</Location><Bucket>test</Bucket><Key>kvemul.tgz</Key><ETag>634fce2c6c1619b0e037a227ed3e3f1e-2</ETag></CompleteMultipartUploadResult>
krishna@escape:~$ 

[REQUEST objectAPIHandlers.CompleteMultipartUploadHandler-fm] [155060374.585548] [2019-02-19 11:15:45 -0800]
POST /test/kvemul.tgz?uploadId=89a7a118-9ea9-4049-9369-323222120c1b
Host: localhost:9000
User-Agent: aws-cli/1.15.11 Python/2.7.14 Linux/4.13.0-46-generic botocore/1.10.22
Accept-Encoding: identity
X-Amz-Content-Sha256: ca65f49d4650abef81afe222a3836ed6346079c513a34637a7c7b09cc72b0993
Content-Length: 271
X-Amz-Date: 20190219T191545Z
Authorization: AWS4-HMAC-SHA256 Credential=minio/20190219/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=5094141f54a4550a718f3bf543b2a88b2b3e77cddcdab2297d3d23caab560513

<CompleteMultipartUpload xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Part><PartNumber>1</PartNumber><ETag>"38fab05ecc7b2b35da51fc08cc316999"</ETag></Part><Part><PartNumber>2</PartNumber><ETag>"8c08205614409dd2918031f3889a2a90"</ETag></Part></CompleteMultipartUpload>

[RESPONSE] [155060374.585548] [2019-02-19 11:16:05 -0800]
200 OK
Content-Security-Policy: block-all-mixed-content
X-Amz-Request-Id: 1584D9E4331949B1
X-Minio-Deployment-Id: 2fd5e87e-3193-4a82-b7db-cc24efcec049
Vary: Origin
X-Xss-Protection: 1; mode=block

<?xml version="1.0" encoding="UTF-8"?>

<?xml version="1.0" encoding="UTF-8"?>

<?xml version="1.0" encoding="UTF-8"?>
<CompleteMultipartUploadResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Location>http://localhost:9000/test/kvemul.tgz</Location><Bucket>test</Bucket><Key>kvemul.tgz</Key><ETag>634fce2c6c1619b0e037a227ed3e3f1e-2</ETag></CompleteMultipartUploadResult>

@harshavardhana
Copy link
Copy Markdown
Member Author

harshavardhana commented Feb 19, 2019

ah, crap there are double XML headers @krishnasrinivas - it should be added outside not here...

@harshavardhana harshavardhana force-pushed the fix-whitespaces branch 2 times, most recently from 33e0155 to 6c2a052 Compare February 19, 2019 20:02
@harshavardhana harshavardhana changed the title Set XML header before sending whitespace characters Send empty characters instead of \r\n or empty characters Feb 19, 2019
@harshavardhana
Copy link
Copy Markdown
Member Author

harshavardhana commented Feb 19, 2019

Fixe @krishnasrinivas @Praveenrajmani PTAL - the functionality of the fix is different now.

krishnasrinivas
krishnasrinivas reviewed Feb 20, 2019
View reviewed changes
@harshavardhana harshavardhana force-pushed the fix-whitespaces branch 3 times, most recently from 23178c1 to 3f584c0 Compare February 20, 2019 22:18
@harshavardhana harshavardhana changed the title Send empty characters instead of \r\n or empty characters Write xml.Header first instead of spaces to handle XML parsers Feb 20, 2019
@harshavardhana
Copy link
Copy Markdown
Member Author

harshavardhana commented Feb 20, 2019

Fixed @krishnasrinivas @Praveenrajmani - PTAL

@harshavardhana harshavardhana self-assigned this Feb 20, 2019
@harshavardhana
Write xml.Header first instead of spaces to handle XML parsers
f7ced1a 
Clients like AWS SDK Java and AWS cli XML parsers are
unable to handle on `\r\n` characters to avoid these
errors send XML header first and write white space characters
instead.

Also handle cases to avoid double WriteHeader calls
@harshavardhana harshavardhana removed their assignment Feb 20, 2019
@minio-ops
Copy link
Copy Markdown

minio-ops commented Feb 21, 2019

Mint Automation

Test Result
mint-compression-xl.sh ✔️
mint-large-bucket.sh ✔️
mint-compression-dist-xl.sh ✔️
mint-compression-fs.sh ✔️
mint-worm.sh ✔️
mint-fs.sh ✔️
mint-dist-xl.sh ✔️
mint-gateway-nas.sh ✔️
mint-xl.sh ❌ more...

7253-f7ced1a/mint-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.53:32056
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp fe787f9b1e6c:/mint/log /tmp/mint-logs'

(1/13) Running aws-sdk-go tests ... done in 0 seconds
(2/13) Running aws-sdk-java tests ... done in 1 seconds
(3/13) Running aws-sdk-php tests ... done in 42 seconds
(4/13) Running aws-sdk-ruby tests ... done in 3 seconds
(5/13) Running awscli tests ... done in 1 minutes and 48 seconds
(6/13) Running mc tests ... done in 25 seconds
(7/13) Running minio-dotnet tests ... done in 54 seconds
(8/13) Running minio-go tests ... done in 46 seconds
(9/13) Running minio-java tests ... done in 1 minutes and 34 seconds
(10/13) Running minio-js tests ... FAILED in 4 seconds
{
  "name": "minio-js",
  "function": "fPutObject(bucketName, objectName, filePath, metaData)",
  "args": "bucketName:minio-js-test-1acd8717-78a4-4955-86f0-48a475dcb793, objectName:datafile-6-MB, filePath:/tmp/datafile-6-MB",
  "duration": 881,
  "status": "FAIL",
  "error": "Error: read ECONNRESET at exports._errnoException (util.js:1020:11) at TCP.onread (net.js:580:26)"
}

Executed 9 out of 13 tests successfully.

@nitisht nitisht merged commit bedcb74 into minio:master Feb 21, 2019
@harshavardhana harshavardhana mentioned this pull request Feb 21, 2019
Merged
9 tasks
@harshavardhana harshavardhana deleted the fix-whitespaces branch February 21, 2019 07:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@krishnasrinivas krishnasrinivas krishnasrinivas approved these changes

@Praveenrajmani Praveenrajmani Praveenrajmani approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@harshavardhana @krishnasrinivas @minio-ops @Praveenrajmani @nitisht