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

CHANGELOG.md

@@ -19,6 +19,7 @@ All notable changes to Gogs are documented in this file.
 
 - _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
 

go.sum

@@ -157,8 +157,6 @@ github.com/jackc/pgconn v0.0.0-20190831204454-2fabfa3c18b7/go.mod h1:ZJKsE/KZfsU
 github.com/jackc/pgconn v1.4.0/go.mod h1:Y2O3ZDF0q4mMacyWV3AstPJpeHXWGEetiFttmq5lahk=
 github.com/jackc/pgconn v1.5.0/go.mod h1:QeD3lBfpTFe8WUnPZWN5KY/mB8FGMIYRdd8P8Jr0fAI=
 github.com/jackc/pgconn v1.5.1-0.20200601181101-fa742c524853/go.mod h1:QeD3lBfpTFe8WUnPZWN5KY/mB8FGMIYRdd8P8Jr0fAI=
-github.com/jackc/pgconn v1.6.4 h1:S7T6cx5o2OqmxdHaXLH1ZeD1SbI8jBznyYE9Ec0RCQ8=
-github.com/jackc/pgconn v1.6.4/go.mod h1:w2pne1C2tZgP+TvjqLpOigGzNqjBgQW9dUw/4Chex78=
 github.com/jackc/pgconn v1.7.0 h1:pwjzcYyfmz/HQOQlENvG1OcDqauTGaqlVahq934F0/U=
 github.com/jackc/pgconn v1.7.0/go.mod h1:sF/lPpNEMEOp+IYhyQGdAvrG20gWf6A1tKlr0v7JMeA=
 github.com/jackc/pgio v1.0.0 h1:g12B9UwVnzGhueNavwioyEEpAmqMe1E/BN9ES+8ovkE=
@@ -174,8 +172,6 @@ github.com/jackc/pgproto3/v2 v2.0.0-alpha1.0.20190609003834-432c2951c711/go.mod
 github.com/jackc/pgproto3/v2 v2.0.0-rc3/go.mod h1:ryONWYqW6dqSg1Lw6vXNMXoBJhpzvWKnT95C46ckYeM=
 github.com/jackc/pgproto3/v2 v2.0.0-rc3.0.20190831210041-4c03ce451f29/go.mod h1:ryONWYqW6dqSg1Lw6vXNMXoBJhpzvWKnT95C46ckYeM=
 github.com/jackc/pgproto3/v2 v2.0.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
-github.com/jackc/pgproto3/v2 v2.0.2 h1:q1Hsy66zh4vuNsajBUF2PNqfAMMfxU5mk594lPE9vjY=
-github.com/jackc/pgproto3/v2 v2.0.2/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
 github.com/jackc/pgproto3/v2 v2.0.5 h1:NUbEWPmCQZbMmYlTjVoNPhc0CfnYyz2bfUAh6A5ZVJM=
 github.com/jackc/pgproto3/v2 v2.0.5/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
 github.com/jackc/pgservicefile v0.0.0-20200307190119-3430c5407db8/go.mod h1:vsD4gTJCa9TptPL8sPkXrLZ+hDuNrZCnj29CQpr4X1E=
@@ -187,8 +183,6 @@ github.com/jackc/pgtype v0.0.0-20190828014616-a8802b16cc59/go.mod h1:MWlu30kVJrU
 github.com/jackc/pgtype v1.2.0/go.mod h1:5m2OfMh1wTK7x+Fk952IDmI4nw3nPrvtQdM0ZT4WpC0=
 github.com/jackc/pgtype v1.3.1-0.20200510190516-8cd94a14c75a/go.mod h1:vaogEUkALtxZMCH411K+tKzNpwzCKU+AnPzBKZ+I+Po=
 github.com/jackc/pgtype v1.3.1-0.20200606141011-f6355165a91c/go.mod h1:cvk9Bgu/VzJ9/lxTO5R5sf80p0DiucVtN7ZxvaC4GmQ=
-github.com/jackc/pgtype v1.4.2 h1:t+6LWm5eWPLX1H5Se702JSBcirq6uWa4jiG4wV1rAWY=
-github.com/jackc/pgtype v1.4.2/go.mod h1:JCULISAZBFGrHaOXIIFiyfzW5VY0GRitRr8NeJsrdig=
 github.com/jackc/pgtype v1.5.0 h1:jzBqRk2HFG2CV4AIwgCI2PwTgm6UUoCAK2ofHHRirtc=
 github.com/jackc/pgtype v1.5.0/go.mod h1:JCULISAZBFGrHaOXIIFiyfzW5VY0GRitRr8NeJsrdig=
 github.com/jackc/pgx/v4 v4.0.0-20190420224344-cc3461e65d96/go.mod h1:mdxmSJJuR08CZQyj1PVQBHy9XOp5p8/SHH6a0psbY9Y=
@@ -197,8 +191,6 @@ github.com/jackc/pgx/v4 v4.0.0-pre1.0.20190824185557-6972a5742186/go.mod h1:X+GQ
 github.com/jackc/pgx/v4 v4.5.0/go.mod h1:EpAKPLdnTorwmPUUsqrPxy5fphV18j9q3wrfRXgo+kA=
 github.com/jackc/pgx/v4 v4.6.1-0.20200510190926-94ba730bb1e9/go.mod h1:t3/cdRQl6fOLDxqtlyhe9UWgfIi9R8+8v8GKV5TRA/o=
 github.com/jackc/pgx/v4 v4.6.1-0.20200606145419-4e5062306904/go.mod h1:ZDaNWkt9sW1JMiNn0kdYBaLelIhw7Pg4qd+Vk6tw7Hg=
-github.com/jackc/pgx/v4 v4.8.1 h1:SUbCLP2pXvf/Sr/25KsuI4aTxiFYIvpfk4l6aTSdyCw=
-github.com/jackc/pgx/v4 v4.8.1/go.mod h1:4HOLxrl8wToZJReD04/yB20GDwf4KBYETvlHciCnwW0=
 github.com/jackc/pgx/v4 v4.9.0 h1:6STjDqppM2ROy5p1wNDcsC7zJTjSHeuCsguZmXyzx7c=
 github.com/jackc/pgx/v4 v4.9.0/go.mod h1:MNGWmViCgqbZck9ujOOBN63gK9XVGILXWCvKLGKmnms=
 github.com/jackc/puddle v0.0.0-20190413234325-e4ced69a3a2b/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
@@ -543,19 +535,14 @@ gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gorm.io/driver/mysql v1.0.1 h1:omJoilUzyrAp0xNoio88lGJCroGdIOen9hq2A/+3ifw=
-gorm.io/driver/mysql v1.0.1/go.mod h1:KtqSthtg55lFp3S5kUXqlGaelnWpKitn4k1xZTnoiPw=
 gorm.io/driver/mysql v1.0.2 h1:xm21Um8cR/Cg+nMwSrajf8aBUxOIC+WmH72ir/ByYR8=
 gorm.io/driver/mysql v1.0.2/go.mod h1:T+Fv7Rq/8+lpS3X1KKVUbj8Y/SzbPa5esK9KpPAKXR8=
-gorm.io/driver/postgres v1.0.1 h1:jRfDNUxpxNrea/97kbcscAQGmiks4UCKAYXsvh4rhOQ=
-gorm.io/driver/postgres v1.0.1/go.mod h1:pv4dVhHvEVrP7k/UYqdBIllbdbpB5VTz89X1O0uOrCA=
 gorm.io/driver/postgres v1.0.2 h1:mB5JjD4QglbCTdMT1aZDxQzHr87XDK1qh0MKIU3P96g=
 gorm.io/driver/postgres v1.0.2/go.mod h1:FvRSYfBI9jEp6ZSjlpS9qNcSjxwYxFc03UOTrHdvvYA=
 gorm.io/driver/sqlite v1.1.3 h1:BYfdVuZB5He/u9dt4qDpZqiqDJ6KhPqs5QUqsr/Eeuc=
 gorm.io/driver/sqlite v1.1.3/go.mod h1:AKDgRWk8lcSQSw+9kxCJnX/yySj8G3rdwYlU57cB45c=
 gorm.io/driver/sqlserver v1.0.4 h1:V15fszi0XAo7fbx3/cF50ngshDSN4QT0MXpWTylyPTY=
 gorm.io/driver/sqlserver v1.0.4/go.mod h1:ciEo5btfITTBCj9BkoUVDvgQbUdLWQNqdFY5OGuGnRg=
-gorm.io/gorm v1.9.19/go.mod h1:0HFTzE/SqkGTzK6TlDPPQbAYCluiVvhzoA1+aVyzenw=
 gorm.io/gorm v1.20.0 h1:qfIlyaZvrF7kMWY3jBdEBXkXJ2M5MFYMTppjILxS3fQ=
 gorm.io/gorm v1.20.0/go.mod h1:0HFTzE/SqkGTzK6TlDPPQbAYCluiVvhzoA1+aVyzenw=
 gorm.io/gorm v1.20.1 h1:+hOwlHDqvqmBIMflemMVPLJH7tZYK4RxFDBHEfJTup0=

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

@@ -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) {
+		vals := c.Req.Header.Values(key)
-			c.Status(failCode)
+		for _, val := range vals {
-			return
+			if strings.Contains(val, value) {
+				return
+			}
 		}
+
+		log.Trace("[LFS] HTTP header %q does not contain value %q", key, value)
+		c.Status(failCode)
 	}
 }