make dataprovider send headers

changelog/unreleased/tune-download.md

@@ -0,0 +1,5 @@
+Bugfix: Make dataproviders return more headers
+
+Instead of ocdav doing an additional Stat request we now rely on the dataprovider to return the necessary metadata information as headers.
+
+https://github.com/owncloud/reva/issues/3080

internal/http/services/datagateway/datagateway.go

@@ -376,15 +376,14 @@ func (s *svc) doPatch(w http.ResponseWriter, r *http.Request) {
 	defer httpRes.Body.Close()
 
 	copyHeader(w.Header(), httpRes.Header)
-
-	if httpRes.StatusCode != http.StatusOK {
+	if httpRes.StatusCode != http.StatusOK && httpRes.StatusCode != http.StatusPartialContent {
 		// swallow the body and set content-length to 0 to prevent reverse proxies from trying to read from it
 		w.Header().Set("Content-Length", "0")
 		w.WriteHeader(httpRes.StatusCode)
 		return
 	}
 
-	w.WriteHeader(http.StatusOK)
+	w.WriteHeader(httpRes.StatusCode)
 	_, err = io.Copy(w, httpRes.Body)
 	if err != nil {
 		log.Err(err).Msg("error writing body after header were set")

internal/http/services/owncloud/ocdav/get.go

@@ -20,25 +20,19 @@ package ocdav
 
 import (
 	"context"
-	"fmt"
 	"io"
 	"net/http"
 	"path"
 	"strconv"
-	"strings"
-	"time"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
-	"github.com/cs3org/reva/v2/internal/grpc/services/storageprovider"
 	"github.com/cs3org/reva/v2/internal/http/services/datagateway"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/spacelookup"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/rhttp"
-	"github.com/cs3org/reva/v2/pkg/storagespace"
-	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/rs/zerolog"
 )
 
@@ -78,22 +72,6 @@ func (s *svc) handleGet(ctx context.Context, w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	sReq := &provider.StatRequest{Ref: ref}
-	sRes, err := client.Stat(ctx, sReq)
-	switch {
-	case err != nil:
-		log.Error().Err(err).Msg("error sending grpc stat request")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	case sRes.Status.Code != rpc.Code_CODE_OK:
-		errors.HandleErrorStatus(&log, w, sRes.Status)
-		return
-	case sRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER:
-		log.Warn().Msg("resource is a folder and cannot be downloaded")
-		w.WriteHeader(http.StatusNotImplemented)
-		return
-	}
-
 	dReq := &provider.InitiateFileDownloadRequest{Ref: ref}
 	dRes, err := client.InitiateFileDownload(ctx, dReq)
 	if err != nil {
@@ -134,33 +112,15 @@ func (s *svc) handleGet(ctx context.Context, w http.ResponseWriter, r *http.Requ
 	}
 	defer httpRes.Body.Close()
 
+	copyHeader(w.Header(), httpRes.Header)
+	w.WriteHeader(httpRes.StatusCode)
+
 	if httpRes.StatusCode != http.StatusOK && httpRes.StatusCode != http.StatusPartialContent {
-		w.WriteHeader(httpRes.StatusCode)
+		// swallow the body and set content-length to 0 to prevent reverse proxies from trying to read from it
+		w.Header().Set("Content-Length", "0")
 		return
 	}
 
-	info := sRes.Info
-
-	w.Header().Set(net.HeaderContentType, info.MimeType)
-	w.Header().Set(net.HeaderContentDisposistion, "attachment; filename*=UTF-8''"+
-		path.Base(info.Path)+"; filename=\""+path.Base(info.Path)+"\"")
-	w.Header().Set(net.HeaderETag, info.Etag)
-	w.Header().Set(net.HeaderOCFileID, storagespace.FormatResourceID(*info.Id))
-	w.Header().Set(net.HeaderOCETag, info.Etag)
-	t := utils.TSToTime(info.Mtime).UTC()
-	lastModifiedString := t.Format(time.RFC1123Z)
-	w.Header().Set(net.HeaderLastModified, lastModifiedString)
-
-	if httpRes.StatusCode == http.StatusPartialContent {
-		w.Header().Set(net.HeaderContentRange, httpRes.Header.Get(net.HeaderContentRange))
-		w.Header().Set(net.HeaderContentLength, httpRes.Header.Get(net.HeaderContentLength))
-		w.WriteHeader(http.StatusPartialContent)
-	} else {
-		w.Header().Set(net.HeaderContentLength, strconv.FormatUint(info.Size, 10))
-	}
-	if info.Checksum != nil {
-		w.Header().Set(net.HeaderOCChecksum, fmt.Sprintf("%s:%s", strings.ToUpper(string(storageprovider.GRPC2PKGXS(info.Checksum.Type))), info.Checksum.Sum))
-	}
 	var c int64
 	if c, err = io.Copy(w, httpRes.Body); err != nil {
 		log.Error().Err(err).Msg("error finishing copying data to response")
@@ -177,6 +137,14 @@ func (s *svc) handleGet(ctx context.Context, w http.ResponseWriter, r *http.Requ
 	// TODO we need to send the If-Match etag in the GET to the datagateway to prevent race conditions between stating and reading the file
 }
 
+func copyHeader(dst, src http.Header) {
+	for key, values := range src {
+		for i := range values {
+			dst.Add(key, values[i])
+		}
+	}
+}
+
 func (s *svc) handleSpacesGet(w http.ResponseWriter, r *http.Request, spaceID string) {
 	ctx, span := s.tracerProvider.Tracer(tracerName).Start(r.Context(), "spaces_get")
 	defer span.End()

internal/http/services/owncloud/ocdav/net/builders.go

@@ -0,0 +1,37 @@
+// Copyright 2018-2022 CERN
+//
+// 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.
+//
+// In applying this license, CERN does not waive the privileges and immunities
+// granted to it by virtue of its status as an Intergovernmental Organization
+// or submit itself to any jurisdiction.
+
+package net
+
+import (
+	"time"
+
+	cs3types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+	"github.com/cs3org/reva/v2/pkg/utils"
+)
+
+// ContentDispositionAttachment builds a ContentDisposition Attachment header with various filename encodings
+func ContentDispositionAttachment(filename string) string {
+	return "attachment; filename*=UTF-8''" + filename + "; filename=\"" + filename + "\""
+}
+
+// RFC1123Z formats a CS3 Timestamp to be used in HTTP headers like Last-Modified
+func RFC1123Z(ts *cs3types.Timestamp) string {
+	t := utils.TSToTime(ts).UTC()
+	return t.Format(time.RFC1123Z)
+}

internal/http/services/owncloud/ocdav/net/headers.go

@@ -24,6 +24,7 @@ const (
 	HeaderAccessControlAllowHeaders  = "Access-Control-Allow-Headers"
 	HeaderAccessControlExposeHeaders = "Access-Control-Expose-Headers"
 	HeaderContentDisposistion        = "Content-Disposition"
+	HeaderContentEncoding            = "Content-Encoding"
 	HeaderContentLength              = "Content-Length"
 	HeaderContentRange               = "Content-Range"
 	HeaderContentType                = "Content-Type"

pkg/rhttp/datatx/utils/download/download.go

@@ -26,8 +26,11 @@ import (
 	"net/http"
 	"path"
 	"strconv"
+	"strings"
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/cs3org/reva/v2/internal/grpc/services/storageprovider"
+	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
 	"github.com/cs3org/reva/v2/pkg/storage"
@@ -124,6 +127,8 @@ func GetOrHeadFile(w http.ResponseWriter, r *http.Request, fs storage.FS, spaceI
 			return
 		}
 
+		code = http.StatusPartialContent
+
 		switch {
 		case len(ranges) == 1:
 			// RFC 7233, Section 4.1:
@@ -144,11 +149,9 @@ func GetOrHeadFile(w http.ResponseWriter, r *http.Request, fs storage.FS, spaceI
 				return
 			}
 			sendSize = ra.Length
-			code = http.StatusPartialContent
 			w.Header().Set("Content-Range", ra.ContentRange(int64(md.Size)))
 		case len(ranges) > 1:
 			sendSize = RangesMIMESize(ranges, md.MimeType, int64(md.Size))
-			code = http.StatusPartialContent
 
 			pr, pw := io.Pipe()
 			mw := multipart.NewWriter(pw)
@@ -177,8 +180,19 @@ func GetOrHeadFile(w http.ResponseWriter, r *http.Request, fs storage.FS, spaceI
 		}
 	}
 
-	if w.Header().Get("Content-Encoding") == "" {
-		w.Header().Set("Content-Length", strconv.FormatInt(sendSize, 10))
+	if w.Header().Get(net.HeaderContentEncoding) == "" {
+		w.Header().Set(net.HeaderContentLength, strconv.FormatInt(sendSize, 10))
+	}
+
+	w.Header().Set(net.HeaderContentType, md.MimeType)
+	w.Header().Set(net.HeaderContentDisposistion, net.ContentDispositionAttachment(path.Base(md.Path)))
+	w.Header().Set(net.HeaderETag, md.Etag)
+	w.Header().Set(net.HeaderOCFileID, storagespace.FormatResourceID(*md.Id))
+	w.Header().Set(net.HeaderOCETag, md.Etag)
+	w.Header().Set(net.HeaderLastModified, net.RFC1123Z(md.Mtime))
+
+	if md.Checksum != nil {
+		w.Header().Set(net.HeaderOCChecksum, fmt.Sprintf("%s:%s", strings.ToUpper(string(storageprovider.GRPC2PKGXS(md.Checksum.Type))), md.Checksum.Sum))
 	}
 
 	w.WriteHeader(code)

tests/acceptance/expected-failures-on-OCIS-storage.md

@@ -803,8 +803,6 @@ _ocdav: api compatibility, return correct status code_
 
 #### [Using double slash in URL to access a folder gives 501 and other status codes](https://github.com/owncloud/ocis/issues/1667)
 - [apiAuthWebDav/webDavSpecialURLs.feature:24](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L24)
-- [apiAuthWebDav/webDavSpecialURLs.feature:34](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L34)
-- [apiAuthWebDav/webDavSpecialURLs.feature:45](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L45)
 - [apiAuthWebDav/webDavSpecialURLs.feature:121](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L121)
 - [apiAuthWebDav/webDavSpecialURLs.feature:132](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L132)
 - [apiAuthWebDav/webDavSpecialURLs.feature:163](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L163)

tests/acceptance/expected-failures-on-S3NG-storage.md

@@ -814,8 +814,6 @@ _ocdav: api compatibility, return correct status code_
 
 #### [Using double slash in URL to access a folder gives 501 and other status codes](https://github.com/owncloud/ocis/issues/1667)
 - [apiAuthWebDav/webDavSpecialURLs.feature:24](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L24)
-- [apiAuthWebDav/webDavSpecialURLs.feature:34](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L34)
-- [apiAuthWebDav/webDavSpecialURLs.feature:45](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L45)
 - [apiAuthWebDav/webDavSpecialURLs.feature:121](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L121)
 - [apiAuthWebDav/webDavSpecialURLs.feature:132](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L132)
 - [apiAuthWebDav/webDavSpecialURLs.feature:163](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavSpecialURLs.feature#L163)