add mint test for metadata size restrictions #816
Conversation
aead
force-pushed
the
restirct_metadata_mint_tests
branch
from
4ad5494
to
f4aa544
Compare
|
Thanks @poornas |
functional_tests.go
Outdated
| @@ -262,6 +262,59 @@ func testMakeBucketError() { | |||
| successLogger(function, args, startTime).Info() | |||
| } | |||
|
|
|||
| func testMetadataSizeLimit() { | |||
| startTime := time.Now() | |||
| function := "PutObjectWithMetadata(bucketName, objectName, reader, metadata)" | |||
There was a problem hiding this comment.
the signature of the function needs to change.
functional_tests.go
Outdated
| args["metadata"] = fmt.Sprint(metadata) | ||
| _, err = c.PutObject(bucketName, objectName, bytes.NewReader(nil), 0, &minio.PutObjectOptions{UserMetadata: metadata}) | ||
| if err == nil { | ||
| failureLog(function, args, startTime, "", "Created object with too much user-defined metadata successfully", nil).Fatal() |
There was a problem hiding this comment.
Perhaps the message could be worded as
Created object with user-defined metadata exceeding metadata size limits.
functional_tests.go
Outdated
| args["metadata"] = fmt.Sprint(metadata) | ||
| _, err = c.PutObject(bucketName, objectName, bytes.NewReader(nil), 0, &minio.PutObjectOptions{UserMetadata: metadata}) | ||
| if err == nil { | ||
| failureLog(function, args, startTime, "", "Created object with too much metadata successfully", nil).Fatal() |
There was a problem hiding this comment.
similar suggestion for "Created object with too much metadata successfully" as previous comment.
functional_tests.go
Outdated
| @@ -262,6 +262,59 @@ func testMakeBucketError() { | |||
| successLogger(function, args, startTime).Info() | |||
| } | |||
|
|
|||
| func testMetadataSizeLimit() { | |||
| startTime := time.Now() | |||
| function := "PutObjectWithMetadata(bucketName, objectName, reader, metadata)" | |||
There was a problem hiding this comment.
Excerpt from the documentation:
API methods PutObjectWithSize, PutObjectWithMetadata, PutObjectStreaming, and
PutObjectWithProgress available in minio-go SDK release v3.0.2 are replaced by the
new PutObject call variant that accepts a pointer to PutObjectOptions struct.
So:
function := "PutObjectWithMetadata(bucketName, objectName, reader, metadata)"
=>
function := "PutObject(bucketName, objectName, reader, objectSize, opts)"
functional_tests.go
Outdated
|
|
||
| metadata := make(map[string]string) | ||
| metadata["X-Amz-Meta-Mint-Test"] = string(bytes.Repeat([]byte("m"), 1+UserMetadataLimit-len("X-Amz-Meta-Mint-Test"))) | ||
| args["metadata"] = fmt.Sprint(metadata) |
There was a problem hiding this comment.
There is no "metadata" argument for PutObject api. So, it needs to be replaced with args["opts"]. However, since "opts" argument is a pointer, we have to show somehow the real metadata value we are passing into the test for meaningful logging purposes, instead of having a pointer value/memory address logged as part of the PutObject api arguments.
You may need to discuss how to handle this case with Nitish, Bala and the team.
I guess the same comment is valid for any arguments which is a pointer or anything like "reader".
functional_tests.go
Outdated
| const UserMetadataLimit = 2 * 1024 | ||
|
|
||
| metadata := make(map[string]string) | ||
| metadata["X-Amz-Meta-Mint-Test"] = string(bytes.Repeat([]byte("m"), 1+UserMetadataLimit-len("X-Amz-Meta-Mint-Test"))) |
There was a problem hiding this comment.
A comment here would be good to explain that this is a negative test and hence a metadata will be created with 1 + <allowed_metadata_limit_bytes> - len("X-Amz-Meta-Mint-Test").
functional_tests.go
Outdated
| args["metadata"] = fmt.Sprint(metadata) | ||
| _, err = c.PutObject(bucketName, objectName, bytes.NewReader(nil), 0, &minio.PutObjectOptions{UserMetadata: metadata}) | ||
| if err == nil { | ||
| failureLog(function, args, startTime, "", "Created object with too much user-defined metadata successfully", nil).Fatal() |
There was a problem hiding this comment.
too much = > too many if the message wording is going to have some kind of a quantification.
| _, err = c.PutObject(bucketName, objectName, bytes.NewReader(nil), 0, &minio.PutObjectOptions{UserMetadata: metadata}) | ||
| if err == nil { | ||
| failureLog(function, args, startTime, "", "Created object with headers exceeding header size limits", nil).Fatal() | ||
| } |
There was a problem hiding this comment.
Could you pls call successLogger() as well when the test passes? Otherwise looks good.
aead
force-pushed
the
restirct_metadata_mint_tests
branch
2 times, most recently
from
5538615
to
4ae64e9
Compare
This change adds a test for the metadata size restriction of AWS. AWS allows user-defined metadata of 2KB and a total HTTP header size of 8KB. Fixes minio/mint#119
aead
force-pushed
the
restirct_metadata_mint_tests
branch
from
cc00d4a
to
e44c57f
Compare
|
Sorry, Github's 'update branch' does a merge commit not a rebase -.- |
This change adds a test for the metadata size restriction of AWS.
AWS allows user-defined metadata of 2KB and a total HTTP header
size of 8KB.
Fixes minio/mint#119