diff --git a/changelog/unreleased/cleanup-code.md b/changelog/unreleased/cleanup-code.md new file mode 100644 index 00000000000..3bdbdb72602 --- /dev/null +++ b/changelog/unreleased/cleanup-code.md @@ -0,0 +1,6 @@ +Enhancement: Cleaned up some code + +- Reduced type conversions []byte <-> string +- pre-compile chunking regex + +https://github.com/cs3org/reva/pull/2516 diff --git a/internal/http/services/owncloud/ocdav/errors/error.go b/internal/http/services/owncloud/ocdav/errors/error.go index bb395d21975..b7fde0d3453 100644 --- a/internal/http/services/owncloud/ocdav/errors/error.go +++ b/internal/http/services/owncloud/ocdav/errors/error.go @@ -19,6 +19,7 @@ package errors import ( + "bytes" "encoding/xml" "net/http" @@ -76,9 +77,12 @@ func Marshal(code code, message string, header string) ([]byte, error) { Header: header, }) if err != nil { - return []byte(""), err + return nil, err } - return []byte(xml.Header + string(xmlstring)), err + var buf bytes.Buffer + buf.WriteString(xml.Header) + buf.Write(xmlstring) + return buf.Bytes(), err } // ErrorXML holds the xml representation of an error diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 2af6777e9f7..874ba81e80d 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -19,6 +19,7 @@ package propfind import ( + "bytes" "context" "encoding/json" "encoding/xml" @@ -218,7 +219,7 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r } w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write([]byte(propRes)); err != nil { + if _, err := w.Write(propRes); err != nil { log.Err(err).Msg("error writing response") } } @@ -545,24 +546,26 @@ func ReadPropfind(r io.Reader) (pf XML, status int, err error) { } // MultistatusResponse converts a list of resource infos into a multistatus response string -func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}) (string, error) { +func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}) ([]byte, error) { responses := make([]*ResponseXML, 0, len(mds)) for i := range mds { res, err := mdToPropResponse(ctx, pf, mds[i], publicURL, ns, linkshares) if err != nil { - return "", err + return nil, err } responses = append(responses, res) } responsesXML, err := xml.Marshal(&responses) if err != nil { - return "", err + return nil, err } - msg := `` - msg += string(responsesXML) + `` - return msg, nil + var buf bytes.Buffer + buf.WriteString(``) + buf.Write(responsesXML) + buf.WriteString(``) + return buf.Bytes(), nil } // mdToPropResponse converts the CS3 metadata into a webdav PropResponse @@ -590,7 +593,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p // -2 indicates unknown (default) // -3 indicates unlimited quota := net.PropQuotaUnknown - size := fmt.Sprintf("%d", md.Size) + size := strconv.FormatUint(md.Size, 10) // TODO refactor helper functions: GetOpaqueJSONEncoded(opaque, key string, *struct) err, GetOpaquePlainEncoded(opaque, key) value, err // or use ok like pattern and return bool? if md.Opaque != nil && md.Opaque.Map != nil { @@ -693,7 +696,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } else { checksums.WriteString(" MD5:") } - checksums.WriteString(string(e.Value)) + checksums.Write(e.Value) } if e, ok := md.Opaque.Map["adler32"]; ok { if checksums.Len() == 0 { @@ -701,7 +704,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } else { checksums.WriteString(" ADLER32:") } - checksums.WriteString(string(e.Value)) + checksums.Write(e.Value) } } if checksums.Len() > 0 { @@ -861,7 +864,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } else { checksums.WriteString(" MD5:") } - checksums.WriteString(string(e.Value)) + checksums.Write(e.Value) } if e, ok := md.Opaque.Map["adler32"]; ok { if checksums.Len() == 0 { @@ -869,7 +872,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } else { checksums.WriteString(" ADLER32:") } - checksums.WriteString(string(e.Value)) + checksums.Write(e.Value) } } if checksums.Len() > 13 { diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index 0846579e28c..9724a26a47b 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -19,6 +19,7 @@ package ocdav import ( + "bytes" "context" "encoding/xml" "fmt" @@ -309,12 +310,12 @@ func (s *svc) handleProppatchResponse(ctx context.Context, w http.ResponseWriter w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write([]byte(propRes)); err != nil { + if _, err := w.Write(propRes); err != nil { log.Err(err).Msg("error writing response") } } -func (s *svc) formatProppatchResponse(ctx context.Context, acceptedProps []xml.Name, removedProps []xml.Name, ref string) (string, error) { +func (s *svc) formatProppatchResponse(ctx context.Context, acceptedProps []xml.Name, removedProps []xml.Name, ref string) ([]byte, error) { responses := make([]propfind.ResponseXML, 0, 1) response := propfind.ResponseXML{ Href: net.EncodePath(ref), @@ -346,13 +347,15 @@ func (s *svc) formatProppatchResponse(ctx context.Context, acceptedProps []xml.N responses = append(responses, response) responsesXML, err := xml.Marshal(&responses) if err != nil { - return "", err + return nil, err } - msg := `` - msg += string(responsesXML) + `` - return msg, nil + var buf bytes.Buffer + buf.WriteString(``) + buf.Write(responsesXML) + buf.WriteString(``) + return buf.Bytes(), nil } func (s *svc) isBooleanProperty(prop string) bool { diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index 512a9e39ba8..1cf12b7e2fc 100644 --- a/internal/http/services/owncloud/ocdav/publicfile.go +++ b/internal/http/services/owncloud/ocdav/publicfile.go @@ -122,7 +122,7 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write([]byte(propRes)); err != nil { + if _, err := w.Write(propRes); err != nil { sublog.Err(err).Msg("error writing response") } } diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 9dc53e38a89..aa3d2c2f2b0 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -298,12 +298,7 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ return } - ok, err := chunking.IsChunked(ref.Path) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return - } - if ok { + if chunking.IsChunked(ref.Path) { chunk, err := chunking.GetChunkBLOBInfo(ref.Path) if err != nil { w.WriteHeader(http.StatusInternalServerError) diff --git a/internal/http/services/owncloud/ocdav/report.go b/internal/http/services/owncloud/ocdav/report.go index 8670074d111..89785f92944 100644 --- a/internal/http/services/owncloud/ocdav/report.go +++ b/internal/http/services/owncloud/ocdav/report.go @@ -124,7 +124,7 @@ func (s *svc) doFilterFiles(w http.ResponseWriter, r *http.Request, ff *reportFi w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) - if _, err := w.Write([]byte(responsesXML)); err != nil { + if _, err := w.Write(responsesXML); err != nil { log.Err(err).Msg("error writing response") } } diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go index 49470f73811..6bf159160e3 100644 --- a/internal/http/services/owncloud/ocdav/trashbin.go +++ b/internal/http/services/owncloud/ocdav/trashbin.go @@ -19,6 +19,7 @@ package ocdav import ( + "bytes" "context" "encoding/xml" "fmt" @@ -197,7 +198,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) - _, err = w.Write([]byte(propRes)) + _, err = w.Write(propRes) if err != nil { sublog.Error().Err(err).Msg("error writing body") return @@ -302,14 +303,14 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) - _, err = w.Write([]byte(propRes)) + _, err = w.Write(propRes) if err != nil { sublog.Error().Err(err).Msg("error writing body") return } } -func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *userpb.User, pf *propfind.XML, items []*provider.RecycleItem) (string, error) { +func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *userpb.User, pf *propfind.XML, items []*provider.RecycleItem) ([]byte, error) { responses := make([]*propfind.ResponseXML, 0, len(items)+1) // add trashbin dir . entry responses = append(responses, &propfind.ResponseXML{ @@ -336,19 +337,21 @@ func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *us for i := range items { res, err := h.itemToPropResponse(ctx, s, u, pf, items[i]) if err != nil { - return "", err + return nil, err } responses = append(responses, res) } responsesXML, err := xml.Marshal(&responses) if err != nil { - return "", err + return nil, err } - msg := `` - msg += string(responsesXML) + `` - return msg, nil + var buf bytes.Buffer + buf.WriteString(``) + buf.Write(responsesXML) + buf.WriteString(``) + return buf.Bytes(), nil } // itemToPropResponse needs to create a listing that contains a key and destination diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index ebe609a925d..9a2d756d310 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -174,7 +174,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, w.Header().Set(net.HeaderDav, "1, 3, extended-mkcol") w.Header().Set(net.HeaderContentType, "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) - _, err = w.Write([]byte(propRes)) + _, err = w.Write(propRes) if err != nil { sublog.Error().Err(err).Msg("error writing body") return diff --git a/internal/http/services/owncloud/ocs/response/response.go b/internal/http/services/owncloud/ocs/response/response.go index f92af90875f..a13fb851bb6 100644 --- a/internal/http/services/owncloud/ocs/response/response.go +++ b/internal/http/services/owncloud/ocs/response/response.go @@ -189,7 +189,7 @@ func encodeXML(res Response) ([]byte, error) { return nil, err } b := new(bytes.Buffer) - b.Write([]byte(xml.Header)) + b.WriteString(xml.Header) b.Write(marshalled) return b.Bytes(), nil } diff --git a/pkg/storage/fs/owncloudsql/upload.go b/pkg/storage/fs/owncloudsql/upload.go index da167efe91a..cb03dcca002 100644 --- a/pkg/storage/fs/owncloudsql/upload.go +++ b/pkg/storage/fs/owncloudsql/upload.go @@ -55,11 +55,7 @@ func (fs *owncloudsqlfs) Upload(ctx context.Context, ref *provider.Reference, r uploadInfo := upload.(*fileUpload) p := uploadInfo.info.Storage["InternalDestination"] - ok, err := chunking.IsChunked(p) - if err != nil { - return errors.Wrap(err, "owncloudsql: error checking path") - } - if ok { + if chunking.IsChunked(p) { var assembledFile string p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r) if err != nil { diff --git a/pkg/storage/utils/chunking/chunking.go b/pkg/storage/utils/chunking/chunking.go index 656ec14fe91..f7df79fdf6e 100644 --- a/pkg/storage/utils/chunking/chunking.go +++ b/pkg/storage/utils/chunking/chunking.go @@ -29,10 +29,14 @@ import ( "strings" ) +var ( + chunkingPathRE = regexp.MustCompile(`-chunking-\w+-[0-9]+-[0-9]+$`) +) + // IsChunked checks if a given path refers to a chunk or not -func IsChunked(fn string) (bool, error) { +func IsChunked(fn string) bool { // FIXME: also need to check whether the OC-Chunked header is set - return regexp.MatchString(`-chunking-\w+-[0-9]+-[0-9]+$`, fn) + return chunkingPathRE.MatchString(fn) } // ChunkBLOBInfo stores info about a particular chunk diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index 1b631e42ae3..ad862808d46 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/pkg/storage/utils/decomposedfs/upload.go @@ -64,11 +64,7 @@ func (fs *Decomposedfs) Upload(ctx context.Context, ref *provider.Reference, r i uploadInfo := upload.(*fileUpload) p := uploadInfo.info.Storage["NodeName"] - ok, err := chunking.IsChunked(p) // check chunking v1 - if err != nil { - return errors.Wrap(err, "Decomposedfs: error checking path") - } - if ok { + if chunking.IsChunked(p) { // check chunking v1 var assembledFile string p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r) if err != nil { @@ -360,10 +356,7 @@ func (fs *Decomposedfs) GetUpload(ctx context.Context, id string) (tusd.Upload, // This method can also handle lookups for paths which contain chunking information. func (fs *Decomposedfs) lookupNode(ctx context.Context, spaceRoot *node.Node, path string) (*node.Node, error) { p := path - isChunked, err := chunking.IsChunked(path) - if err != nil { - return nil, err - } + isChunked := chunking.IsChunked(path) if isChunked { chunkInfo, err := chunking.GetChunkBLOBInfo(path) if err != nil { diff --git a/pkg/storage/utils/eosfs/upload.go b/pkg/storage/utils/eosfs/upload.go index 9d20e01194f..a1c6d61a141 100644 --- a/pkg/storage/utils/eosfs/upload.go +++ b/pkg/storage/utils/eosfs/upload.go @@ -39,11 +39,7 @@ func (fs *eosfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadC return errtypes.PermissionDenied("eos: cannot upload under the virtual share folder") } - ok, err := chunking.IsChunked(p) - if err != nil { - return errors.Wrap(err, "eos: error checking path") - } - if ok { + if chunking.IsChunked(p) { var assembledFile string p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r) if err != nil { diff --git a/pkg/storage/utils/localfs/upload.go b/pkg/storage/utils/localfs/upload.go index 36963e7bfb4..fe5036e0dbb 100644 --- a/pkg/storage/utils/localfs/upload.go +++ b/pkg/storage/utils/localfs/upload.go @@ -49,11 +49,7 @@ func (fs *localfs) Upload(ctx context.Context, ref *provider.Reference, r io.Rea uploadInfo := upload.(*fileUpload) p := uploadInfo.info.Storage["InternalDestination"] - ok, err := chunking.IsChunked(p) - if err != nil { - return errors.Wrap(err, "localfs: error checking path") - } - if ok { + if chunking.IsChunked(p) { var assembledFile string p, assembledFile, err = fs.chunkHandler.WriteChunk(p, r) if err != nil {