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

Author: unknwon

CHANGELOG.md

@@ -22,6 +22,7 @@ All notable changes to Gogs are documented in this file.
 - Commit message contains keywords look like an issue reference no longer fails the push entirely. [#6289](https://github.com/gogs/gogs/issues/6289)
 - _Regression:_ When running Gogs on Windows, push commits no longer fail on a daily basis with the error "pre-receive hook declined". [#6316](https://github.com/gogs/gogs/issues/6316)
 - Auto-linked commit SHAs now have correct links. [#6300](https://github.com/gogs/gogs/issues/6300)
+- Git LFS client (with version >= 2.5.0) wasn't able to upload files with known format (e.g. PNG, JPEG), and the server is expecting the HTTP Header `Content-Type` to be `application/octet-stream`. The server now tells the LFS client to always use `Content-Type: application/octet-stream` when upload files.
 
 ### Removed
 

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 {

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())
 		})
 	}
 }

internal/route/lfs/route.go

@@ -135,6 +135,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)
 	}
@@ -144,10 +146,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)
 	}
 }