make dataprovider send headers (#3080)

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

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)