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>

3 files changed, 21 insertions, 29 deletions

diff --git a/modules/lfs/content_store.go b/modules/lfs/content_store.go
index b4f2bb6044..520caa4c99 100644
--- 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 c6fbf090e5..692c81f583 100644
--- 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 45cba9d9b7..f45423b851 100644
--- 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")