Skip to content

Commit f770da7

Browse files
marktheunissenmarktheunissen
committed
additional test for negative case, test coverage for policy, remove comment
1 parent 49125e9 commit f770da7

File tree

5 files changed

+659
-41
lines changed

5 files changed

+659
-41
lines changed

‎functional_tests.go‎

Lines changed: 170 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5149,38 +5149,13 @@ func testPresignedPostPolicy() {
51495149
}
51505150

51515151
policy := minio.NewPostPolicy()
5152-
5153-
if err := policy.SetBucket(""); err == nil {
5154-
logError(testName, function, args, startTime, "", "SetBucket did not fail for invalid conditions", err)
5155-
return
5156-
}
5157-
if err := policy.SetKey(""); err == nil {
5158-
logError(testName, function, args, startTime, "", "SetKey did not fail for invalid conditions", err)
5159-
return
5160-
}
5161-
if err := policy.SetExpires(time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC)); err == nil {
5162-
logError(testName, function, args, startTime, "", "SetExpires did not fail for invalid conditions", err)
5163-
return
5164-
}
5165-
if err := policy.SetContentType(""); err == nil {
5166-
logError(testName, function, args, startTime, "", "SetContentType did not fail for invalid conditions", err)
5167-
return
5168-
}
5169-
if err := policy.SetContentLengthRange(1024*1024, 1024); err == nil {
5170-
logError(testName, function, args, startTime, "", "SetContentLengthRange did not fail for invalid conditions", err)
5171-
return
5172-
}
5173-
if err := policy.SetUserMetadata("", ""); err == nil {
5174-
logError(testName, function, args, startTime, "", "SetUserMetadata did not fail for invalid conditions", err)
5175-
return
5176-
}
5177-
51785152
policy.SetBucket(bucketName)
51795153
policy.SetKey(objectName)
51805154
policy.SetExpires(time.Now().UTC().AddDate(0, 0, 10)) // expires in 10 days
51815155
policy.SetContentType("binary/octet-stream")
51825156
policy.SetContentLengthRange(10, 1024*1024)
51835157
policy.SetUserMetadata(metadataKey, metadataValue)
5158+
policy.SetContentEncoding("gzip")
51845159

51855160
// Add CRC32C
51865161
checksum := minio.ChecksumCRC32C.ChecksumBytes(buf)
@@ -5322,6 +5297,174 @@ func testPresignedPostPolicy() {
53225297
logSuccess(testName, function, args, startTime)
53235298
}
53245299

5300+
// testPresignedPostPolicyWrongFile tests that when we have a policy with a checksum, we cannot POST the wrong file
5301+
func testPresignedPostPolicyWrongFile() {
5302+
// initialize logging params
5303+
startTime := time.Now()
5304+
testName := getFuncName()
5305+
function := "PresignedPostPolicy(policy)"
5306+
args := map[string]interface{}{
5307+
"policy": "",
5308+
}
5309+
5310+
c, err := NewClient(ClientConfig{})
5311+
if err != nil {
5312+
logError(testName, function, args, startTime, "", "MinIO client object creation failed", err)
5313+
return
5314+
}
5315+
5316+
// Generate a new random bucket name.
5317+
bucketName := randString(60, rand.NewSource(time.Now().UnixNano()), "minio-go-test-")
5318+
5319+
// Make a new bucket in 'us-east-1' (source bucket).
5320+
err = c.MakeBucket(context.Background(), bucketName, minio.MakeBucketOptions{Region: "us-east-1"})
5321+
if err != nil {
5322+
logError(testName, function, args, startTime, "", "MakeBucket failed", err)
5323+
return
5324+
}
5325+
5326+
defer cleanupBucket(bucketName, c)
5327+
5328+
// Generate 33K of data.
5329+
reader := getDataReader("datafile-33-kB")
5330+
defer reader.Close()
5331+
5332+
objectName := randString(60, rand.NewSource(time.Now().UnixNano()), "")
5333+
// Azure requires the key to not start with a number
5334+
metadataKey := randString(60, rand.NewSource(time.Now().UnixNano()), "user")
5335+
metadataValue := randString(60, rand.NewSource(time.Now().UnixNano()), "")
5336+
5337+
buf, err := io.ReadAll(reader)
5338+
if err != nil {
5339+
logError(testName, function, args, startTime, "", "ReadAll failed", err)
5340+
return
5341+
}
5342+
5343+
policy := minio.NewPostPolicy()
5344+
policy.SetBucket(bucketName)
5345+
policy.SetKey(objectName)
5346+
policy.SetExpires(time.Now().UTC().AddDate(0, 0, 10)) // expires in 10 days
5347+
policy.SetContentType("binary/octet-stream")
5348+
policy.SetContentLengthRange(10, 1024*1024)
5349+
policy.SetUserMetadata(metadataKey, metadataValue)
5350+
5351+
// Add CRC32C of the 33kB file that the policy will explicitly allow.
5352+
checksum := minio.ChecksumCRC32C.ChecksumBytes(buf)
5353+
err = policy.SetChecksum(checksum)
5354+
if err != nil {
5355+
logError(testName, function, args, startTime, "", "SetChecksum failed", err)
5356+
return
5357+
}
5358+
5359+
args["policy"] = policy.String()
5360+
5361+
presignedPostPolicyURL, formData, err := c.PresignedPostPolicy(context.Background(), policy)
5362+
if err != nil {
5363+
logError(testName, function, args, startTime, "", "PresignedPostPolicy failed", err)
5364+
return
5365+
}
5366+
5367+
// At this stage, we have a policy that allows us to upload datafile-33-kB.
5368+
// Test that uploading datafile-10-kB, with a different checksum, fails as expected
5369+
filePath := getMintDataDirFilePath("datafile-10-kB")
5370+
if filePath == "" {
5371+
// Make a temp file with 10 KB data.
5372+
file, err := os.CreateTemp(os.TempDir(), "PresignedPostPolicyTest")
5373+
if err != nil {
5374+
logError(testName, function, args, startTime, "", "TempFile creation failed", err)
5375+
return
5376+
}
5377+
if _, err = io.Copy(file, getDataReader("datafile-10-kB")); err != nil {
5378+
logError(testName, function, args, startTime, "", "Copy failed", err)
5379+
return
5380+
}
5381+
if err = file.Close(); err != nil {
5382+
logError(testName, function, args, startTime, "", "File Close failed", err)
5383+
return
5384+
}
5385+
filePath = file.Name()
5386+
}
5387+
fileReader := getDataReader("datafile-10-kB")
5388+
defer fileReader.Close()
5389+
buf10k, err := io.ReadAll(fileReader)
5390+
if err != nil {
5391+
logError(testName, function, args, startTime, "", "ReadAll failed", err)
5392+
return
5393+
}
5394+
otherChecksum := minio.ChecksumCRC32C.ChecksumBytes(buf10k)
5395+
5396+
var formBuf bytes.Buffer
5397+
writer := multipart.NewWriter(&formBuf)
5398+
for k, v := range formData {
5399+
if k == "x-amz-checksum-crc32c" {
5400+
v = otherChecksum.Encoded()
5401+
}
5402+
writer.WriteField(k, v)
5403+
}
5404+
5405+
// Add file to post request
5406+
f, err := os.Open(filePath)
5407+
defer f.Close()
5408+
if err != nil {
5409+
logError(testName, function, args, startTime, "", "File open failed", err)
5410+
return
5411+
}
5412+
w, err := writer.CreateFormFile("file", filePath)
5413+
if err != nil {
5414+
logError(testName, function, args, startTime, "", "CreateFormFile failed", err)
5415+
return
5416+
}
5417+
_, err = io.Copy(w, f)
5418+
if err != nil {
5419+
logError(testName, function, args, startTime, "", "Copy failed", err)
5420+
return
5421+
}
5422+
writer.Close()
5423+
5424+
httpClient := &http.Client{
5425+
Timeout: 30 * time.Second,
5426+
Transport: createHTTPTransport(),
5427+
}
5428+
args["url"] = presignedPostPolicyURL.String()
5429+
5430+
req, err := http.NewRequest(http.MethodPost, presignedPostPolicyURL.String(), bytes.NewReader(formBuf.Bytes()))
5431+
if err != nil {
5432+
logError(testName, function, args, startTime, "", "HTTP request failed", err)
5433+
return
5434+
}
5435+
5436+
req.Header.Set("Content-Type", writer.FormDataContentType())
5437+
5438+
// Make the POST request with the form data.
5439+
res, err := httpClient.Do(req)
5440+
if err != nil {
5441+
logError(testName, function, args, startTime, "", "HTTP request failed", err)
5442+
return
5443+
}
5444+
defer res.Body.Close()
5445+
if res.StatusCode != http.StatusForbidden {
5446+
logError(testName, function, args, startTime, "", "HTTP request unexpected status", errors.New(res.Status))
5447+
return
5448+
}
5449+
5450+
// Read the response body, ensure it has checksum failure message
5451+
resBody, err := io.ReadAll(res.Body)
5452+
if err != nil {
5453+
logError(testName, function, args, startTime, "", "ReadAll failed", err)
5454+
return
5455+
}
5456+
5457+
// Normalize the response body, because S3 uses quotes around the policy condition components
5458+
// in the error message, MinIO does not.
5459+
resBodyStr := strings.ReplaceAll(string(resBody), `"`, "")
5460+
if !strings.Contains(resBodyStr, "Policy Condition failed: [eq, $x-amz-checksum-crc32c, aHnJMw==]") {
5461+
logError(testName, function, args, startTime, "", "Unexpected response body", errors.New(resBodyStr))
5462+
return
5463+
}
5464+
5465+
logSuccess(testName, function, args, startTime)
5466+
}
5467+
53255468
// Tests copy object
53265469
func testCopyObject() {
53275470
// initialize logging params
@@ -13653,6 +13796,7 @@ func main() {
1365313796
testGetObjectReadAtFunctional()
1365413797
testGetObjectReadAtWhenEOFWasReached()
1365513798
testPresignedPostPolicy()
13799+
testPresignedPostPolicyWrongFile()
1365613800
testCopyObject()
1365713801
testComposeObjectErrorCases()
1365813802
testCompose10KSources()

‎post-policy.go‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (p *PostPolicy) SetExpires(t time.Time) error {
8585

8686
// SetKey - Sets an object name for the policy based upload.
8787
func (p *PostPolicy) SetKey(key string) error {
88-
if strings.TrimSpace(key) == "" || key == "" {
88+
if strings.TrimSpace(key) == "" {
8989
return errInvalidArgument("Object name is empty.")
9090
}
9191
policyCond := policyCondition{
@@ -118,7 +118,7 @@ func (p *PostPolicy) SetKeyStartsWith(keyStartsWith string) error {
118118

119119
// SetBucket - Sets bucket at which objects will be uploaded to.
120120
func (p *PostPolicy) SetBucket(bucketName string) error {
121-
if strings.TrimSpace(bucketName) == "" || bucketName == "" {
121+
if strings.TrimSpace(bucketName) == "" {
122122
return errInvalidArgument("Bucket name is empty.")
123123
}
124124
policyCond := policyCondition{
@@ -135,7 +135,7 @@ func (p *PostPolicy) SetBucket(bucketName string) error {
135135

136136
// SetCondition - Sets condition for credentials, date and algorithm
137137
func (p *PostPolicy) SetCondition(matchType, condition, value string) error {
138-
if strings.TrimSpace(value) == "" || value == "" {
138+
if strings.TrimSpace(value) == "" {
139139
return errInvalidArgument("No value specified for condition")
140140
}
141141

@@ -156,7 +156,7 @@ func (p *PostPolicy) SetCondition(matchType, condition, value string) error {
156156

157157
// SetTagging - Sets tagging for the object for this policy based upload.
158158
func (p *PostPolicy) SetTagging(tagging string) error {
159-
if strings.TrimSpace(tagging) == "" || tagging == "" {
159+
if strings.TrimSpace(tagging) == "" {
160160
return errInvalidArgument("No tagging specified.")
161161
}
162162
_, err := tags.ParseObjectXML(strings.NewReader(tagging))
@@ -178,7 +178,7 @@ func (p *PostPolicy) SetTagging(tagging string) error {
178178
// SetContentType - Sets content-type of the object for this policy
179179
// based upload.
180180
func (p *PostPolicy) SetContentType(contentType string) error {
181-
if strings.TrimSpace(contentType) == "" || contentType == "" {
181+
if strings.TrimSpace(contentType) == "" {
182182
return errInvalidArgument("No content type specified.")
183183
}
184184
policyCond := policyCondition{
@@ -211,7 +211,7 @@ func (p *PostPolicy) SetContentTypeStartsWith(contentTypeStartsWith string) erro
211211

212212
// SetContentDisposition - Sets content-disposition of the object for this policy
213213
func (p *PostPolicy) SetContentDisposition(contentDisposition string) error {
214-
if strings.TrimSpace(contentDisposition) == "" || contentDisposition == "" {
214+
if strings.TrimSpace(contentDisposition) == "" {
215215
return errInvalidArgument("No content disposition specified.")
216216
}
217217
policyCond := policyCondition{
@@ -226,6 +226,23 @@ func (p *PostPolicy) SetContentDisposition(contentDisposition string) error {
226226
return nil
227227
}
228228

229+
// SetContentEncoding - Sets content-encoding of the object for this policy
230+
func (p *PostPolicy) SetContentEncoding(contentEncoding string) error {
231+
if strings.TrimSpace(contentEncoding) == "" {
232+
return errInvalidArgument("No content encoding specified.")
233+
}
234+
policyCond := policyCondition{
235+
matchType: "eq",
236+
condition: "$Content-Encoding",
237+
value: contentEncoding,
238+
}
239+
if err := p.addNewPolicy(policyCond); err != nil {
240+
return err
241+
}
242+
p.formData["Content-Encoding"] = contentEncoding
243+
return nil
244+
}
245+
229246
// SetContentLengthRange - Set new min and max content length
230247
// condition for all incoming uploads.
231248
func (p *PostPolicy) SetContentLengthRange(minLen, maxLen int64) error {
@@ -246,7 +263,7 @@ func (p *PostPolicy) SetContentLengthRange(minLen, maxLen int64) error {
246263
// SetSuccessActionRedirect - Sets the redirect success url of the object for this policy
247264
// based upload.
248265
func (p *PostPolicy) SetSuccessActionRedirect(redirect string) error {
249-
if strings.TrimSpace(redirect) == "" || redirect == "" {
266+
if strings.TrimSpace(redirect) == "" {
250267
return errInvalidArgument("Redirect is empty")
251268
}
252269
policyCond := policyCondition{
@@ -264,7 +281,7 @@ func (p *PostPolicy) SetSuccessActionRedirect(redirect string) error {
264281
// SetSuccessStatusAction - Sets the status success code of the object for this policy
265282
// based upload.
266283
func (p *PostPolicy) SetSuccessStatusAction(status string) error {
267-
if strings.TrimSpace(status) == "" || status == "" {
284+
if strings.TrimSpace(status) == "" {
268285
return errInvalidArgument("Status is empty")
269286
}
270287
policyCond := policyCondition{
@@ -282,10 +299,10 @@ func (p *PostPolicy) SetSuccessStatusAction(status string) error {
282299
// SetUserMetadata - Set user metadata as a key/value couple.
283300
// Can be retrieved through a HEAD request or an event.
284301
func (p *PostPolicy) SetUserMetadata(key, value string) error {
285-
if strings.TrimSpace(key) == "" || key == "" {
302+
if strings.TrimSpace(key) == "" {
286303
return errInvalidArgument("Key is empty")
287304
}
288-
if strings.TrimSpace(value) == "" || value == "" {
305+
if strings.TrimSpace(value) == "" {
289306
return errInvalidArgument("Value is empty")
290307
}
291308
headerName := fmt.Sprintf("x-amz-meta-%s", key)
@@ -304,7 +321,7 @@ func (p *PostPolicy) SetUserMetadata(key, value string) error {
304321
// SetUserMetadataStartsWith - Set how an user metadata should starts with.
305322
// Can be retrieved through a HEAD request or an event.
306323
func (p *PostPolicy) SetUserMetadataStartsWith(key, value string) error {
307-
if strings.TrimSpace(key) == "" || key == "" {
324+
if strings.TrimSpace(key) == "" {
308325
return errInvalidArgument("Key is empty")
309326
}
310327
headerName := fmt.Sprintf("x-amz-meta-%s", key)
@@ -326,8 +343,6 @@ func (p *PostPolicy) SetChecksum(c Checksum) error {
326343
p.formData[amzChecksumAlgo] = c.Type.String()
327344
p.formData[c.Type.Key()] = c.Encoded()
328345

329-
// Needed for S3 compatibility. MinIO ignores the checksum keys in the policy.
330-
// https://github.com/minio/minio/blob/RELEASE.2024-08-29T01-40-52Z/cmd/postpolicyform.go#L60-L65
331346
policyCond := policyCondition{
332347
matchType: "eq",
333348
condition: fmt.Sprintf("$%s", amzChecksumAlgo),

0 commit comments

Comments
 (0)