Close file on invalid range (Addition to #15166) (#15268)

* Close file on invalid range.

* Close on seek error

Signed-off-by: Andrew Thornton <art27@cantab.net>
* Moved 'Seek' into server.

* io.ReadSeekCloser is only available in Go 1.16

Co-authored-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

diff --git a/modules/lfs/content_store.go b/modules/lfs/content_store.go
index b4f2bb6044fa4ba737d9356c0ecff7b524ecbddb..520caa4c99033233f0e9467d03e61c6cbc59a0c1 100644 (file)
--- a/modules/lfs/content_store.go
+++ b/modules/lfs/content_store.go
@@ -44,32 +44,13 @@ type ContentStore struct {
 }
 
 // Get takes a Meta object and retrieves the content from the store, returning
-// it as an io.Reader. If fromByte > 0, the reader starts from that byte
-func (s *ContentStore) Get(meta *models.LFSMetaObject, fromByte int64) (io.ReadCloser, error) {
+// it as an io.ReadSeekCloser.
+func (s *ContentStore) Get(meta *models.LFSMetaObject) (storage.Object, error) {
        f, err := s.Open(meta.RelativePath())
        if err != nil {
                log.Error("Whilst trying to read LFS OID[%s]: Unable to open Error: %v", meta.Oid, err)
                return nil, err
        }
-       if fromByte > 0 {
-               if fromByte >= meta.Size {
-                       err = f.Close()
-                       if err != nil {
-                               log.Error("Whilst trying to read LFS OID[%s]: Unable to close Error: %v", meta.Oid, err)
-                       }
-                       return nil, ErrRangeNotSatisfiable{
-                               FromByte: fromByte,
-                       }
-               }
-               _, err = f.Seek(fromByte, io.SeekStart)
-               if err != nil {
-                       log.Error("Whilst trying to read LFS OID[%s]: Unable to seek to %d Error: %v", meta.Oid, fromByte, err)
-                       errClose := f.Close()
-                       if errClose != nil {
-                               log.Error("Whilst trying to read LFS OID[%s]: Unable to close Error: %v", meta.Oid, errClose)
-                       }
-               }
-       }
        return f, err
 }
 

diff --git a/modules/lfs/pointers.go b/modules/lfs/pointers.go
index c6fbf090e51640ad704b6b9ebddb852a57de24cf..692c81f5832cfeaf40c28255ffb5a4b7ae2d87d2 100644 (file)
--- a/modules/lfs/pointers.go
+++ b/modules/lfs/pointers.go
@@ -67,5 +67,5 @@ func IsPointerFile(buf *[]byte) *models.LFSMetaObject {
 // ReadMetaObject will read a models.LFSMetaObject and return a reader
 func ReadMetaObject(meta *models.LFSMetaObject) (io.ReadCloser, error) {
        contentStore := &ContentStore{ObjectStorage: storage.LFS}
-       return contentStore.Get(meta, 0)
+       return contentStore.Get(meta)
 }

diff --git a/modules/lfs/server.go b/modules/lfs/server.go
index 45cba9d9b75122363048c2566cff1241ad62bd87..f45423b85108ed25decf084c3b18d39d459e207b 100644 (file)
--- a/modules/lfs/server.go
+++ b/modules/lfs/server.go
@@ -175,6 +175,11 @@ func getContentHandler(ctx *context.Context) {
                        statusCode = 206
                        fromByte, _ = strconv.ParseInt(match[1], 10, 32)
 
+                       if fromByte >= meta.Size {
+                               writeStatus(ctx, http.StatusRequestedRangeNotSatisfiable)
+                               return
+                       }
+
                        if match[2] != "" {
                                _toByte, _ := strconv.ParseInt(match[2], 10, 32)
                                if _toByte >= fromByte && _toByte < toByte {
@@ -188,18 +193,24 @@ func getContentHandler(ctx *context.Context) {
        }
 
        contentStore := &ContentStore{ObjectStorage: storage.LFS}
-       content, err := contentStore.Get(meta, fromByte)
+       content, err := contentStore.Get(meta)
        if err != nil {
-               if IsErrRangeNotSatisfiable(err) {
-                       writeStatus(ctx, http.StatusRequestedRangeNotSatisfiable)
-               } else {
-                       // Errors are logged in contentStore.Get
-                       writeStatus(ctx, 404)
-               }
+               // Errors are logged in contentStore.Get
+               writeStatus(ctx, http.StatusNotFound)
                return
        }
        defer content.Close()
 
+       if fromByte > 0 {
+               _, err = content.Seek(fromByte, io.SeekStart)
+               if err != nil {
+                       log.Error("Whilst trying to read LFS OID[%s]: Unable to seek to %d Error: %v", meta.Oid, fromByte, err)
+
+                       writeStatus(ctx, http.StatusInternalServerError)
+                       return
+               }
+       }
+
        contentLength := toByte + 1 - fromByte
        ctx.Resp.Header().Set("Content-Length", strconv.FormatInt(contentLength, 10))
        ctx.Resp.Header().Set("Content-Type", "application/octet-stream")