diff --git a/changelog/unreleased/tune-download.md b/changelog/unreleased/tune-download.md new file mode 100644 index 0000000000..17379f5d0d --- /dev/null +++ b/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 \ No newline at end of file diff --git a/internal/http/services/datagateway/datagateway.go b/internal/http/services/datagateway/datagateway.go index 382ef6c978..814e479db8 100644 --- a/internal/http/services/datagateway/datagateway.go +++ b/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") diff --git a/internal/http/services/owncloud/ocdav/get.go b/internal/http/services/owncloud/ocdav/get.go index 6922b9cecb..e5750dbaa7 100644 --- a/internal/http/services/owncloud/ocdav/get.go +++ b/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() diff --git a/internal/http/services/owncloud/ocdav/net/builders.go b/internal/http/services/owncloud/ocdav/net/builders.go new file mode 100644 index 0000000000..0498072b6c --- /dev/null +++ b/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) +} diff --git a/internal/http/services/owncloud/ocdav/net/headers.go b/internal/http/services/owncloud/ocdav/net/headers.go index 05dfebd12f..b22af9a127 100644 --- a/internal/http/services/owncloud/ocdav/net/headers.go +++ b/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" diff --git a/pkg/rhttp/datatx/utils/download/download.go b/pkg/rhttp/datatx/utils/download/download.go index 7374edb2af..aa34bd73dd 100644 --- a/pkg/rhttp/datatx/utils/download/download.go +++ b/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) diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md index 49d103a575..c5b7a34af6 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/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) diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md index c9468754ed..ffbabd51cb 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/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)