lfs: ask client to always send the same value for the HTTP header (#6369)

3 files changed, 72 insertions, 8 deletions

diff --git a/internal/route/lfs/batch.go b/internal/route/lfs/batch.go
index 4f720948..bfc364c2 100644
--- a/internal/route/lfs/batch.go
+++ b/internal/route/lfs/batch.go
@@ -44,6 +44,11 @@ func serveBatch(c *macaron.Context, owner *db.User, repo *db.Repository) {
 				actions = batchActions{
 					Upload: &batchAction{
 						Href: fmt.Sprintf("%s/%s", baseHref, obj.Oid),
+						Header: map[string]string{
+							// NOTE: git-lfs v2.5.0 sets the Content-Type based on the uploaded file.
+							// This ensures that the client always uses the designated value for the header.
+							"Content-Type": "application/octet-stream",
+						},
 					},
 					Verify: &batchAction{
 						Href: fmt.Sprintf("%s/verify", baseHref),
@@ -136,7 +141,8 @@ type batchError struct {
 }
 
 type batchAction struct {
-	Href string `json:"href"`
+	Href   string            `json:"href"`
+	Header map[string]string `json:"header,omitempty"`
 }
 
 type batchActions struct {
diff --git a/internal/route/lfs/batch_test.go b/internal/route/lfs/batch_test.go
index 61cfb562..67b85eeb 100644
--- a/internal/route/lfs/batch_test.go
+++ b/internal/route/lfs/batch_test.go
@@ -6,6 +6,7 @@ package lfs
 
 import (
 	"bytes"
+	"encoding/json"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -42,7 +43,7 @@ func Test_serveBatch(t *testing.T) {
 			name:          "unrecognized operation",
 			body:          `{"operation": "update"}`,
 			expStatusCode: http.StatusBadRequest,
-			expBody:       `{"message":"Operation not recognized"}` + "\n",
+			expBody:       `{"message": "Operation not recognized"}` + "\n",
 		},
 		{
 			name: "upload: contains invalid oid",
@@ -53,7 +54,25 @@ func Test_serveBatch(t *testing.T) {
 	{"oid": "ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f", "size": 123}
 ]}`,
 			expStatusCode: http.StatusOK,
-			expBody:       `{"transfer":"basic","objects":[{"oid":"bad_oid","size":123,"actions":{"error":{"code":422,"message":"Object has invalid oid"}}},{"oid":"ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f","size":123,"actions":{"upload":{"href":"https://gogs.example.com/owner/repo.git/info/lfs/objects/basic/ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"},"verify":{"href":"https://gogs.example.com/owner/repo.git/info/lfs/objects/basic/verify"}}}]}` + "\n",
+			expBody: `{
+	"transfer": "basic",
+	"objects": [
+		{"oid": "bad_oid", "size":123, "actions": {"error": {"code": 422, "message": "Object has invalid oid"}}},
+		{
+			"oid": "ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f",
+			"size": 123,
+			"actions": {
+				"upload": {
+					"href": "https://gogs.example.com/owner/repo.git/info/lfs/objects/basic/ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f",
+					"header": {"Content-Type": "application/octet-stream"}
+				},
+				"verify": {
+					"href": "https://gogs.example.com/owner/repo.git/info/lfs/objects/basic/verify"
+				}
+			}
+		}
+	]
+}` + "\n",
 		},
 		{
 			name: "download: contains non-existent oid and mismatched size",
@@ -78,7 +97,26 @@ func Test_serveBatch(t *testing.T) {
 				},
 			},
 			expStatusCode: http.StatusOK,
-			expBody:       `{"transfer":"basic","objects":[{"oid":"bad_oid","size":123,"actions":{"error":{"code":404,"message":"Object does not exist"}}},{"oid":"ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f","size":123,"actions":{"error":{"code":422,"message":"Object size mismatch"}}},{"oid":"5cac0a318669fadfee734fb340a5f5b70b428ac57a9f4b109cb6e150b2ba7e57","size":456,"actions":{"download":{"href":"https://gogs.example.com/owner/repo.git/info/lfs/objects/basic/5cac0a318669fadfee734fb340a5f5b70b428ac57a9f4b109cb6e150b2ba7e57"}}}]}` + "\n",
+			expBody: `{
+	"transfer": "basic",
+	"objects": [
+		{"oid": "bad_oid", "size": 123, "actions": {"error": {"code": 404, "message": "Object does not exist"}}},
+		{
+			"oid": "ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f",
+			"size": 123,
+			"actions": {"error": {"code": 422, "message": "Object size mismatch"}}
+		},
+		{
+			"oid": "5cac0a318669fadfee734fb340a5f5b70b428ac57a9f4b109cb6e150b2ba7e57",
+			"size": 456,
+			"actions": {
+				"download": {
+					"href": "https://gogs.example.com/owner/repo.git/info/lfs/objects/basic/5cac0a318669fadfee734fb340a5f5b70b428ac57a9f4b109cb6e150b2ba7e57"
+				}
+			}
+		}
+	]
+}` + "\n",
 		},
 	}
 	for _, test := range tests {
@@ -100,7 +138,20 @@ func Test_serveBatch(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			assert.Equal(t, test.expBody, string(body))
+
+			var expBody bytes.Buffer
+			err = json.Indent(&expBody, []byte(test.expBody), "", "  ")
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var gotBody bytes.Buffer
+			err = json.Indent(&gotBody, body, "", "  ")
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			assert.Equal(t, expBody.String(), gotBody.String())
 		})
 	}
 }
diff --git a/internal/route/lfs/route.go b/internal/route/lfs/route.go
index f254423a..40cb95eb 100644
--- a/internal/route/lfs/route.go
+++ b/internal/route/lfs/route.go
@@ -136,6 +136,8 @@ func authorize(mode db.AccessMode) macaron.Handler {
 			return
 		}
 
+		log.Trace("[LFS] Authorized user %q to %q", actor.Name, username+"/"+reponame)
+
 		c.Map(owner) // NOTE: Override actor
 		c.Map(repo)
 	}
@@ -145,10 +147,15 @@ func authorize(mode db.AccessMode) macaron.Handler {
 // When not, response given "failCode" as status code.
 func verifyHeader(key, value string, failCode int) macaron.Handler {
 	return func(c *macaron.Context) {
-		if !strings.Contains(c.Req.Header.Get(key), value) {
-			c.Status(failCode)
-			return
+		vals := c.Req.Header.Values(key)
+		for _, val := range vals {
+			if strings.Contains(val, value) {
+				return
+			}
 		}
+
+		log.Trace("[LFS] HTTP header %q does not contain value %q", key, value)
+		c.Status(failCode)
 	}
 }