Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2 throws custom error Content-Length shorter handle it #7334

Merged
merged 1 commit into from Mar 8, 2019

Conversation

harshavardhana
Copy link
Member

Description

http2 throws custom error Content-Length shorter handle it

Motivation and Context

We should internally handle when http2 input stream has smaller
content than its content-length header

Upstream issue reported golang/go#30648

This a change which we need to handle internally until Go fixes it
correctly, till now our code doesn't expect a custom error to be returned.

Regression

In a way

How Has This Been Tested?

Using http2 server and minio-java tests with this PR https://github.com/minio/minio-java/pull/758/files#diff-b434019f6f2c56a278f611c67601eb94R1140 should pass.

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

codecov bot commented Mar 7, 2019

Codecov Report

Merging #7334 into master will increase coverage by <.01%.
The diff coverage is 52.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7334      +/-   ##
==========================================
+ Coverage   48.42%   48.42%   +<.01%     
==========================================
  Files         296      296              
  Lines       46394    46408      +14     
==========================================
+ Hits        22465    22473       +8     
- Misses      21862    21869       +7     
+ Partials     2067     2066       -1
Impacted Files Coverage Δ
cmd/api-errors.go 50.62% <52.63%> (-0.93%) ⬇️
cmd/posix.go 63.86% <0%> (-0.22%) ⬇️
cmd/fs-v1-helpers.go 68.8% <0%> (+0.61%) ⬆️
cmd/bitrot-streaming.go 81.55% <0%> (+2.91%) ⬆️

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 f4879ed...e86a4f4. Read the comment docs.

krishnasrinivas
krishnasrinivas previously approved these changes Mar 7, 2019
We should internally handle when http2 input stream has smaller
content than its content-length header

Upstream issue reported golang/go#30648

This a change which we need to handle internally until Go fixes it
correctly, till now our code doesn't expect a custom error to be returned.
@minio-ops
Copy link

Mint Automation

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

7334-e86a4f4/mint-dist-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.56:31290
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 c2f65ca1d0a9:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... FAILED in 0 seconds
{
  "alert": "",
  "args": {
    "bucketName": "aws-sdk-go-test-cpmpohe1s2mr9v",
    "expiry": 60000000000,
    "objectName": "presignedTest"
  },
  "duration": 322,
  "error": "RequestError: send request failed\ncaused by: Put http://72.28.97.56:31290/aws-sdk-go-test-cpmpohe1s2mr9v: dial tcp 72.28.97.56:31290: connect: connection refused",
  "function": "PresignedPut",
  "message": "AWS SDK Go CreateBucket Failed",
  "name": "aws-sdk-go",
  "status": "FAIL"
}

Executed 0 out of 14 tests successfully.

7334-e86a4f4/mint-large-bucket.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.60:30917
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 bc678a0599dd:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... FAILED in 1 seconds
{
  "alert": "",
  "args": {
    "bucketName": "aws-sdk-go-test-4r4w54poohcfsx",
    "expiry": 60000000000,
    "objectName": "presignedTest"
  },
  "duration": 276,
  "error": "RequestError: send request failed\ncaused by: Put http://72.28.97.60:30917/aws-sdk-go-test-4r4w54poohcfsx: dial tcp 72.28.97.60:30917: connect: connection refused",
  "function": "PresignedPut",
  "message": "AWS SDK Go CreateBucket Failed",
  "name": "aws-sdk-go",
  "status": "FAIL"
}

Executed 0 out of 14 tests successfully.

@harshavardhana harshavardhana merged commit 0b96ad4 into minio:master Mar 8, 2019
@harshavardhana harshavardhana deleted the http2 branch March 8, 2019 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants