Repository: minio/minio (language: go)

Issue:
MinIO discards or corrupts GET parameters of success_action_redirect
## Expected Behavior
<!--- If you're describing a bug, tell us what should happen -->
<!--- If you're suggesting a change/improvement, tell us how it should work -->

Should send 303 redirect to untouched URL in `success_action_redirect` POST parameter, like does Amazon S3, or at least don't break it. 

## Current Behavior
<!--- If describing a bug, tell us what happens instead of the expected behavior -->
<!--- If suggesting a change/improvement, explain the difference from current behavior -->

If `success_action_redirect` has GET parameters, such as created by [Canvas-LMS](https://github.com/instructure/canvas-lms), such as 

[https://institutiondomian/api/v1/files/61/create_success?include%5B%5D=preview_url&on_duplicate=rename&uuid=someuuid](https://institutiondomian/api/v1/files/61/create_success?include%5B%5D=preview_url&on_duplicate=rename&uuid=someuuid)

When sending **303 See Other** redirect, MinIO either corrupts parameters by adding the second question mark: 
[https://institutiondomian/api/v1/files/61/create_success?include%5B%5D=preview_url&on_duplicate=rename&uuid=someuuid?bucket=bucketname-canvas_new_version&key=account_1%2Fattachments%2F61%2F3683.pdf&etag=%22eed43007721a768c96%22](https://institutiondomian/api/v1/files/61/create_success?include%5B%5D=preview_url&on_duplicate=rename&uuid=someuuid?bucket=bucketname-canvas_new_version&key=account_1%2Fattachments%2F61%2F3683.pdf&etag=%22eed43007721a768c96%22)

Or completely discards the original parameters:
[https://institutiondomian/api/v1/files/61/create_success?bucket=bucketname-canvas_new_version&key=account_1%2Fattachments%2F61%2F3683.pdf&etag=%22eed43007721a768c96%22](https://institutiondomian/api/v1/files/61/create_success?bucket=bucketname-canvas_new_version&key=account_1%2Fattachments%2F61%2F3683.pdf&etag=%22eed43007721a768c96%22)

See discussion in the [Canvas LMS Users](https://groups.google.com/g/canvas-lms-users/c/PuWVE3qtES8) Google Group and probably [this issue](https://github.com/instructure/canvas-lms/issues/1898) in Canvas issues (I am not sure that the issue starter uses MinIO, but I have the same behavior).

## Possible Solution
<!--- Not obligatory, but suggest a fix/reason for the bug, -->
<!--- or ideas how to implement the addition or change -->

1. Make the environment variable (e.g. `MINIO_SUCCESS_REDIRECT_POLICY`) allowing to keep `success_action_redirect` URL untouched.
2. Correctly process the GET parameters (keep original and add `&bucket=aaa&etag=bbb&key=cccc`).

## Steps to Reproduce (for bugs)
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
<!--- and make sure you have followed https://github.com/minio/minio/tree/release/docs/debugging to capture relevant logs -->

1. Create POST query having `success_action_redirect` with URL-encoded GET parameters
2. Open Google Chrome Debugger, trace Network requests, search for "303 See other" response of MinIO, and find that URL is corrupted (of course, you can see the same result by using `curl -v` or Postman).

## Context
<!--- How has this issue affected you? What are you trying to accomplish? -->
<!--- Providing context helps us come up with a solution that is most useful in the real world -->

I want to use Canvas LMS with MinIO as a replacement of Amazon S3 for file upload storage.

Steps to configure:

1. Setup Canvas using [Production Start](https://github.com/instructure/canvas-lms/wiki/Production-Start) wiki page.
2. Create `config/amazon_s3.yml` with the following parameters:
```
production:
  bucket_name: my-bucket
  access_key_id: 'somekey'
  secret_access_key: 'some_secret_key'
  region: us-east-1
  force_path_style: true
  endpoint: 'https://my-minio-domain.net'
```
3. Change `config/file_store.yml` to S3
```
production:
  storage: s3
```
4. Try to upload file using RCE editor, like described in the [discussion](https://groups.google.com/g/canvas-lms-users/c/PuWVE3qtES8) or in [this issue](https://github.com/instructure/canvas-lms/issues/1898) and get an error, because the main app cannot recieve the callback which is in the `success_action_redirect` having the format described above.
5. Uploads via Course Files (they are using `success_action_status`, not `success_action_redirect`) and downloads of manually uploaded files (I've transferred local files by using [this methodology](https://github.com/instructure/canvas-lms/issues/1313) and `mc` client) are working, so that S3 connection setup is correct.
6. Amazon S3 works correctly, so this is MinIO issue, not a Canvas issue.

## Regression
<!-- Is this issue a regression? (Yes / No) -->
<!-- If Yes, optionally please include minio version or commit id or PR# that caused this regression, if you have these details. -->
As you can see, current version discards the original GET parameters:
[https://github.com/minio/minio/blob/e60b67d246ceb1a5c85a363685df2c4acbece3e7/cmd/bucket-handlers.go#L941-L947](https://github.com/minio/minio/blob/e60b67d246ceb1a5c85a363685df2c4acbece3e7/cmd/bucket-handlers.go#L941-L947)

[https://github.com/minio/minio/blob/e60b67d246ceb1a5c85a363685df2c4acbece3e7/cmd/bucket-handlers.go#L1118-L1123](https://github.com/minio/minio/blob/e60b67d246ceb1a5c85a363685df2c4acbece3e7/cmd/bucket-handlers.go#L1118-L1123)

[https://github.com/minio/minio/blob/e60b67d246ceb1a5c85a363685df2c4acbece3e7/cmd/handler-utils.go#L200-L208](https://github.com/minio/minio/blob/e60b67d246ceb1a5c85a363685df2c4acbece3e7/cmd/handler-utils.go#L200-L208)

Older version (which can still be used in NAS boxes) breakes them by adding a second question mark:

[https://github.com/minio/minio/pull/3479/commits/e76e9951825550be65b427d488d719be7b9ed54e#diff-be8383605d637cd937d113f6baf42a205379935d376ecf1afdd1b631325d9e4aR458](https://github.com/minio/minio/pull/3479/commits/e76e9951825550be65b427d488d719be7b9ed54e#diff-be8383605d637cd937d113f6baf42a205379935d376ecf1afdd1b631325d9e4aR458)

## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
* Version used (`minio --version`):
Docker image `quay.io/minio/minio:RELEASE.2022-07-04`
* Server setup and configuration:
[Standard Docker Compose Installation](https://docs.min.io/docs/deploy-minio-on-docker-compose.html)
[Nginx reverse proxy](https://docs.min.io/docs/setup-nginx-proxy-with-minio.html)

* Operating System and version (`uname -a`):
```
$ uname -a
Linux s3-test 5.4.0-121-generic #137-Ubuntu SMP Wed Jun 15 13:33:07 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=20.04
DISTRIB_CODENAME=focal
DISTRIB_DESCRIPTION="Ubuntu 20.04.4 LTS"
```



Discussion / hints:

Fix patch:
diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go
index 4183a74a56f82..ae612cdbdba0c 100644
--- a/cmd/bucket-handlers.go
+++ b/cmd/bucket-handlers.go
@@ -1116,9 +1116,10 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
 		Host:         handlers.GetSourceIP(r),
 	})
 
-	if successRedirect != "" {
-		// Replace raw query params..
-		redirectURL.RawQuery = getRedirectPostRawQuery(objInfo)
+	if redirectURL != nil { // success_action_redirect is valid and set.
+		redirectURL.Query().Add("bucket", objInfo.Bucket)
+		redirectURL.Query().Add("key", objInfo.Name)
+		redirectURL.Query().Add("etag", "\""+objInfo.ETag+"\"")
 		writeRedirectSeeOther(w, redirectURL.String())
 		return
 	}
diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go
index b6d625637dc18..4b3ba66ebb5ac 100644
--- a/cmd/handler-utils.go
+++ b/cmd/handler-utils.go
@@ -27,7 +27,6 @@ import (
 	"mime/multipart"
 	"net/http"
 	"net/textproto"
-	"net/url"
 	"regexp"
 	"strings"
 
@@ -197,16 +196,6 @@ func extractMetadataFromMime(ctx context.Context, v textproto.MIMEHeader, m map[
 	return nil
 }
 
-// The Query string for the redirect URL the client is
-// redirected on successful upload.
-func getRedirectPostRawQuery(objInfo ObjectInfo) string {
-	redirectValues := make(url.Values)
-	redirectValues.Set("bucket", objInfo.Bucket)
-	redirectValues.Set("key", objInfo.Name)
-	redirectValues.Set("etag", "\""+objInfo.ETag+"\"")
-	return redirectValues.Encode()
-}
-
 // Returns access credentials in the request Authorization header.
 func getReqAccessCred(r *http.Request, region string) (cred auth.Credentials) {
 	cred, _, _ = getReqAccessKeyV4(r, region, serviceS3)


Test patch:
diff --git a/cmd/post-policy_test.go b/cmd/post-policy_test.go
index 3261118ff6a8e..4e9e250fa900e 100644
--- a/cmd/post-policy_test.go
+++ b/cmd/post-policy_test.go
@@ -447,7 +447,7 @@ func testPostPolicyBucketHandlerRedirect(obj ObjectLayer, instanceType string, t
 	targetObj := keyName + "/upload.txt"
 
 	// The url of success_action_redirect field
-	redirectURL, err := url.Parse("http://www.google.com")
+	redirectURL, err := url.Parse("http://www.google.com?query=value")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -499,7 +499,9 @@ func testPostPolicyBucketHandlerRedirect(obj ObjectLayer, instanceType string, t
 		t.Error("Unexpected error: ", err)
 	}
 
-	redirectURL.RawQuery = getRedirectPostRawQuery(info)
+	redirectURL.Query().Add("bucket", info.Bucket)
+	redirectURL.Query().Add("key", info.Name)
+	redirectURL.Query().Add("etag", "\""+info.ETag+"\"")
 	expectedLocation := redirectURL.String()
 
 	// Check the new location url