lfs: run e2e and fix minor issues (#6059)

internal/cmd/web.go

@@ -165,7 +165,7 @@ func newMacaron() *macaron.Macaron {
 	}))
 	m.Use(toolbox.Toolboxer(m, toolbox.Options{
 		HealthCheckFuncs: []*toolbox.HealthCheckFuncDesc{
-			&toolbox.HealthCheckFuncDesc{
+			{
 				Desc: "Database connection",
 				Func: db.Ping,
 			},

internal/lfsutil/oid.go

@@ -5,26 +5,17 @@
 package lfsutil
 
 import (
-	"strings"
+	"gogs.io/gogs/internal/lazyregexp"
 )
 
 // OID is an LFS object ID.
 type OID string
 
-// ValidOID returns true if given oid is valid according to spec:
-// https://github.com/git-lfs/git-lfs/blob/master/docs/spec.md
-func ValidOID(oid OID) bool {
-	fields := strings.SplitN(string(oid), ":", 2)
-	if len(fields) != 2 {
-		return false
-	}
-	method := fields[0]
-	hash := fields[1]
+// An OID is a 64-char lower case hexadecimal, produced by SHA256.
+// Spec: https://github.com/git-lfs/git-lfs/blob/master/docs/spec.md
+var oidRe = lazyregexp.New("^[a-f0-9]{64}$")
 
-	switch method {
-	case "sha256":
-		// SHA256 produces 64-char lower case hexadecimal hash
-		return len(hash) == 64 && strings.ToLower(hash) == hash
-	}
-	return false
+// ValidOID returns true if given oid is valid.
+func ValidOID(oid OID) bool {
+	return oidRe.MatchString(string(oid))
 }

internal/lfsutil/oid_test.go

@@ -18,24 +18,16 @@ func TestValidOID(t *testing.T) {
 	}{
 		{
 			name: "malformed",
-			oid:  OID("12345678"),
+			oid:  OID("7c222fb2927d828af22f592134e8932480637c0d"),
 		},
 		{
-			name: "unknown method",
-			oid:  OID("sha1:7c222fb2927d828af22f592134e8932480637c0d"),
+			name: "not all lower cased",
+			oid:  OID("EF797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"),
 		},
 
 		{
-			name: "sha256: malformed",
-			oid:  OID("sha256:7c222fb2927d828af22f592134e8932480637c0d"),
-		},
-		{
-			name: "sha256: not all lower cased",
-			oid:  OID("sha256:EF797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"),
-		},
-		{
-			name:   "sha256: valid",
-			oid:    OID("sha256:ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"),
+			name:   "valid",
+			oid:    OID("ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"),
 			expVal: true,
 		},
 	}

internal/lfsutil/storage.go

@@ -6,7 +6,6 @@ package lfsutil
 
 import (
 	"path/filepath"
-	"strings"
 )
 
 // Storage is the storage type of an LFS object.
@@ -23,7 +22,5 @@ func StorageLocalPath(root string, oid OID) string {
 		return ""
 	}
 
-	// Valid OID is guaranteed to have second element as hash.
-	hash := strings.SplitN(string(oid), ":", 2)[1]
-	return filepath.Join(root, string(hash[0]), string(hash[1]), hash)
+	return filepath.Join(root, string(oid[0]), string(oid[1]), string(oid))
 }

internal/lfsutil/storage_test.go

@@ -31,7 +31,7 @@ func TestStorageLocalPath(t *testing.T) {
 		{
 			name:    "valid oid",
 			root:    "/lfs-objects",
-			oid:     OID("sha256:ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"),
+			oid:     OID("ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"),
 			expPath: "/lfs-objects/e/f/ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f",
 		},
 	}

internal/route/lfs/basic.go

@@ -52,14 +52,13 @@ func serveBasicDownload(c *context.Context, repo *db.Repository, oid lfsutil.OID
 
 	c.Header().Set("Content-Type", "application/octet-stream")
 	c.Header().Set("Content-Length", strconv.FormatInt(object.Size, 10))
+	c.Status(http.StatusOK)
 
 	_, err = io.Copy(c.Resp, r)
 	if err != nil {
-		c.Status(http.StatusInternalServerError)
 		log.Error("Failed to copy object file: %v", err)
 		return
 	}
-	c.Status(http.StatusOK)
 }
 
 // PUT /{owner}/{repo}.git/info/lfs/object/basic/{oid}

internal/route/lfs/batch.go

@@ -166,13 +166,13 @@ const contentType = "application/vnd.git-lfs+json"
 
 func responseJSON(w http.ResponseWriter, status int, v interface{}) {
 	w.Header().Set("Content-Type", contentType)
+	w.WriteHeader(status)
 
 	err := jsoniter.NewEncoder(w).Encode(v)
 	if err != nil {
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		log.Error("Failed to encode JSON: %v", err)
 		return
 	}
-	w.WriteHeader(status)
 }
 
 func internalServerError(w http.ResponseWriter) {

internal/route/lfs/route.go

@@ -134,11 +134,11 @@ func authorize(mode db.AccessMode) macaron.Handler {
 	}
 }
 
-// verifyHeader checks if the HTTP header value is matching.
+// verifyHeader checks if the HTTP header contains given value.
 // When not, response given "failCode" as status code.
 func verifyHeader(key, value string, failCode int) macaron.Handler {
 	return func(c *context.Context) {
-		if c.Req.Header.Get(key) != value {
+		if !strings.Contains(c.Req.Header.Get(key), value) {
 			c.Status(failCode)
 			return
 		}