Generate {application,text}/xml headers on gateway by client's choice (…

…#992)

* Generate {application,text}/xml headers on gateway by client's choice

If no header is specified, none is explicitly generated.  Then the Go http server generates
one by scanning content, it happens to be text/xml.  If either of these headers is
specified, generate that requested content type.  If (only) other headers are generated,
respond "Not Acceptable".

This does not break backwards compatibility with clients that do not generate such a
header (boto, mc).  This should improve compatibility with a client that explicitly asks for
application/xml (the Akka S3 client alpakka only accepts that Content-Type, so it had better
ask for it).  It *will* break a lying client that requests only anything-but-standard-XML.

* Change default return type to application/xml

Matches behavipours of S3 (verified by @ozkatz on encrypted with Postman) and
minio (verified on plaintext with Wireshark)

* Set application/xml by default, except for object HEAD and GET

All S3 gateway responses are application/xml, except when replying with contents from
adapter.  There keep the existing behaviour (Go http content type detection).  We can
add content-type forwarding to all our adapters, but that would be a separate change.
Files returned from get-object keep their original Content-Type - auto-detected right
now, but may change to forward from the block adapter if we decide to be more like S3
(in response to customer demand).

Honour an Accepts header *if present*, allowing a client to prefer text/xml.  Accepts
headers are not usefully relevant to get-object, and continue to be ignored.

* [CR] Improve documentation of default vs proxied content-type

gateway/handler.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"net/http"
 	"reflect"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -347,8 +348,46 @@ func notFound(w http.ResponseWriter, _ *http.Request) {
 	w.WriteHeader(http.StatusNotFound)
 }
 
+var commaSeparator = regexp.MustCompile(`,\s*`)
+
+var (
+	contentTypeApplicationXML = "application/xml"
+	contentTypeTextXML        = "text/xml"
+)
+
+func selectContentType(acceptable []string) *string {
+	for _, acceptableTypes := range acceptable {
+		acceptable := commaSeparator.Split(acceptableTypes, -1)
+		for _, a := range acceptable {
+			switch a {
+			case contentTypeTextXML:
+				return &contentTypeTextXML
+			case contentTypeApplicationXML:
+				return &contentTypeApplicationXML
+			}
+		}
+	}
+	return nil
+}
+
+func setDefaultContentType(w http.ResponseWriter, r *http.Request) {
+	acceptable, ok := r.Header["Accept"]
+	if ok {
+		defaultContentType := selectContentType(acceptable)
+		if defaultContentType != nil {
+			w.Header().Set("Content-Type", *defaultContentType)
+		}
+		// If no requested content type matched, still OK at least for proxied content
+		// (GET or HEAD), so set up to auto-detect.
+	} else {
+		w.Header().Set("Content-Type", contentTypeApplicationXML)
+		// For proxied content (GET or HEAD) the type will be reset according to
+		// whatever headers arrive, including setting up to auto-detect content-type if
+		// none is specified by the adapter.
+	}
+}
+
 func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	// pprof endpoints
 	handler := h.servePathBased(r)
 	if handler == nil {
 		handler = h.serveVirtualHost(r)
@@ -358,6 +397,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 	start := time.Now()
 	mrw := httputil.NewMetricResponseWriter(w)
+	setDefaultContentType(mrw, r)
 	handler.ServeHTTP(mrw, r)
 	requestHistograms.WithLabelValues(h.operationID, strconv.Itoa(mrw.StatusCode)).Observe(time.Since(start).Seconds())
 }

gateway/operations/base.go

@@ -116,6 +116,11 @@ func (o *Operation) SetHeader(key, value string) {
 	o.ResponseWriter.Header()[key] = []string{value}
 }
 
+// DeleteHeader deletes a header from the response
+func (o *Operation) DeleteHeader(key string) {
+	o.ResponseWriter.Header().Del(key)
+}
+
 // SetHeaders sets a map of headers on the response while preserving the header's case
 func (o *Operation) SetHeaders(headers map[string]string) {
 	for k, v := range headers {

gateway/operations/getobject.go

@@ -99,6 +99,9 @@ func (controller *GetObject) Handle(o *PathOperation) {
 	if rng.StartOffset != -1 {
 		o.SetHeader("Content-Range", fmt.Sprintf("bytes %d-%d/%d", rng.StartOffset, rng.EndOffset, entry.Size))
 	}
+	// Delete the default content-type header so http.Server will detect it from contents
+	// TODO(ariels): After/if we add content-type support to adapter, use *that*.
+	o.DeleteHeader("Content-Type")
 	_, err = io.Copy(o.ResponseWriter, data)
 	if err != nil {
 		o.Log().WithError(err).Error("could not write response body for object")

gateway/operations/headobject.go

@@ -37,12 +37,18 @@ func (controller *HeadObject) Handle(o *PathOperation) {
 		o.EncodeError(gatewayerrors.Codes.ToAPIErr(gatewayerrors.ErrInternalError))
 		return
 	}
-	o.SetHeader("Accept-Ranges", "bytes")
-	o.SetHeader("Last-Modified", httputil.HeaderTimestamp(entry.CreationDate))
-	o.SetHeader("ETag", httputil.ETag(entry.Checksum))
-	o.SetHeader("Content-Length", fmt.Sprintf("%d", entry.Size))
 	if entry.Expired {
 		o.Log().WithError(err).Info("querying expired object")
 		o.EncodeError(gatewayerrors.Codes.ToAPIErr(gatewayerrors.ErrNoSuchVersion))
+		return
 	}
+
+	o.SetHeader("Accept-Ranges", "bytes")
+	o.SetHeader("Last-Modified", httputil.HeaderTimestamp(entry.CreationDate))
+	o.SetHeader("ETag", httputil.ETag(entry.Checksum))
+	o.SetHeader("Content-Length", fmt.Sprintf("%d", entry.Size))
+
+	// Delete the default content-type header so http.Server will detect it from contents
+	// TODO(ariels): After/if we add content-type support to adapter, use *that*.
+	o.DeleteHeader("Content-Type")
 }