From 4cadb33da267c83c7064c727157d5eb88b5df7b7 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 24 Sep 2017 16:43:21 -0700 Subject: [PATCH] api/PostPolicy: Allow location header fully qualified URL (#4926) req.Host is used to construct the final object location. Fixes #4910 --- cmd/api-response.go | 22 ++++++++++-- cmd/api-response_test.go | 74 ++++++++++++++++++++++++++++++++++++++++ cmd/bucket-handlers.go | 6 ++-- cmd/event-notifier.go | 7 +--- cmd/net.go | 7 +--- 5 files changed, 99 insertions(+), 17 deletions(-) create mode 100644 cmd/api-response_test.go diff --git a/cmd/api-response.go b/cmd/api-response.go index 9e139b9d7..088df94fa 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -275,9 +275,25 @@ func getLocation(r *http.Request) string { return path.Clean(r.URL.Path) // Clean any trailing slashes. } -// getObjectLocation gets the relative URL for an object -func getObjectLocation(bucketName string, key string) string { - return "/" + bucketName + "/" + key +// returns "https" if the tls boolean is true, "http" otherwise. +func getURLScheme(tls bool) string { + if tls { + return httpsScheme + } + return httpScheme +} + +// getObjectLocation gets the fully qualified URL of an object. +func getObjectLocation(host, proto, bucket, object string) string { + if proto == "" { + proto = getURLScheme(globalIsSSL) + } + u := url.URL{ + Host: host, + Path: path.Join(slashSeparator, bucket, object), + Scheme: proto, + } + return u.String() } // generates ListBucketsResponse from array of BucketInfo which can be diff --git a/cmd/api-response_test.go b/cmd/api-response_test.go new file mode 100644 index 000000000..6ad4dda10 --- /dev/null +++ b/cmd/api-response_test.go @@ -0,0 +1,74 @@ +/* + * Minio Cloud Storage, (C) 2017 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "testing" +) + +// Tests object location. +func TestObjectLocation(t *testing.T) { + testCases := []struct { + host, proto, bucket, object string + expectedLocation string + }{ + // Server binding to localhost IP with https. + { + host: "127.0.0.1:9000", + proto: httpsScheme, + bucket: "testbucket1", + object: "test/1.txt", + expectedLocation: "https://127.0.0.1:9000/testbucket1/test/1.txt", + }, + // Server binding to fqdn. + { + host: "s3.mybucket.org", + proto: httpScheme, + bucket: "mybucket", + object: "test/1.txt", + expectedLocation: "http://s3.mybucket.org/mybucket/test/1.txt", + }, + // Server binding to fqdn. + { + host: "mys3.mybucket.org", + proto: "", + bucket: "mybucket", + object: "test/1.txt", + expectedLocation: "http://mys3.mybucket.org/mybucket/test/1.txt", + }, + } + for i, testCase := range testCases { + gotLocation := getObjectLocation(testCase.host, testCase.proto, testCase.bucket, testCase.object) + if testCase.expectedLocation != gotLocation { + t.Errorf("Test %d: expected %s, got %s", i+1, testCase.expectedLocation, gotLocation) + } + } +} + +// Tests getURLScheme function behavior. +func TestGetURLScheme(t *testing.T) { + tls := false + gotScheme := getURLScheme(tls) + if gotScheme != httpScheme { + t.Errorf("Expected %s, got %s", httpScheme, gotScheme) + } + tls = true + gotScheme = getURLScheme(tls) + if gotScheme != httpsScheme { + t.Errorf("Expected %s, got %s", httpsScheme, gotScheme) + } +} diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 971369682..8bb4692e5 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -569,8 +569,10 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h return } + port := r.Header.Get("X-Forward-Proto") + location := getObjectLocation(r.Host, port, bucket, object) w.Header().Set("ETag", `"`+objInfo.ETag+`"`) - w.Header().Set("Location", getObjectLocation(bucket, object)) + w.Header().Set("Location", location) // Get host and port from Request.RemoteAddr. host, port, err := net.SplitHostPort(r.RemoteAddr) @@ -603,7 +605,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h Bucket: objInfo.Bucket, Key: objInfo.Name, ETag: `"` + objInfo.ETag + `"`, - Location: getObjectLocation(objInfo.Bucket, objInfo.Name), + Location: location, }) writeResponse(w, http.StatusCreated, resp, "application/xml") case "200": diff --git a/cmd/event-notifier.go b/cmd/event-notifier.go index 8fa2780b8..72d306ef5 100644 --- a/cmd/event-notifier.go +++ b/cmd/event-notifier.go @@ -101,12 +101,7 @@ func newNotificationEvent(event eventData) NotificationEvent { host = localIP4.ToSlice()[0] } - scheme := httpScheme - if globalIsSSL { - scheme = httpsScheme - } - - return fmt.Sprintf("%s://%s:%s", scheme, host, globalMinioPort) + return fmt.Sprintf("%s://%s:%s", getURLScheme(globalIsSSL), host, globalMinioPort) } // Fetch the region. diff --git a/cmd/net.go b/cmd/net.go index 8cccc2373..7ae2b7157 100644 --- a/cmd/net.go +++ b/cmd/net.go @@ -175,13 +175,8 @@ func getAPIEndpoints(serverAddr string) (apiEndpoints []string) { ipList = []string{host} } - scheme := httpScheme - if globalIsSSL { - scheme = httpsScheme - } - for _, ip := range ipList { - apiEndpoints = append(apiEndpoints, fmt.Sprintf("%s://%s:%s", scheme, ip, port)) + apiEndpoints = append(apiEndpoints, fmt.Sprintf("%s://%s:%s", getURLScheme(globalIsSSL), ip, port)) } return apiEndpoints