From 42814131f68c17ff298313d46ba9e923b70d4b88 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 13 Apr 2021 16:15:00 +0200 Subject: [PATCH 01/30] Use constants. --- services/lfs/server.go | 64 +++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index cd9a3fd7a1594..0fa8bd812035d 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -59,7 +59,7 @@ func isOidValid(oid string) bool { func ObjectOidHandler(ctx *context.Context) { if !setting.LFS.StartServer { log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } @@ -77,20 +77,20 @@ func ObjectOidHandler(ctx *context.Context) { } log.Warn("Unhandled LFS method: %s for %s/%s OID[%s]", ctx.Req.Method, ctx.Params("username"), ctx.Params("reponame"), ctx.Params("oid")) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) } func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) (*models.LFSMetaObject, *models.Repository) { if !isOidValid(p.Oid) { log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return nil, nil } repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return nil, nil } @@ -102,7 +102,7 @@ func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs meta, err := repository.GetLFSMetaObjectByOid(p.Oid) if err != nil { log.Error("Unable to get LFS OID[%s] Error: %v", p.Oid, err) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return nil, nil } @@ -122,12 +122,12 @@ func getContentHandler(ctx *context.Context) { // Support resume download using Range header var fromByte, toByte int64 toByte = meta.Size - 1 - statusCode := 200 + statusCode := http.StatusOK if rangeHdr := ctx.Req.Header.Get("Range"); rangeHdr != "" { regex := regexp.MustCompile(`bytes=(\d+)\-(\d*).*`) match := regex.FindStringSubmatch(rangeHdr) if len(match) > 1 { - statusCode = 206 + statusCode = http.StatusPartialContent fromByte, _ = strconv.ParseInt(match[1], 10, 32) if fromByte >= meta.Size { @@ -206,20 +206,20 @@ func getMetaHandler(ctx *context.Context) { } } - logRequest(ctx.Req, 200) + logRequest(ctx.Req, http.StatusOK) } // PostHandler instructs the client how to upload data func PostHandler(ctx *context.Context) { if !setting.LFS.StartServer { log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } if !MetaMatcher(ctx.Req) { log.Info("Attempt to POST without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, 400) + writeStatus(ctx, http.StatusBadRequest) return } @@ -228,7 +228,7 @@ func PostHandler(ctx *context.Context) { repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } @@ -239,35 +239,35 @@ func PostHandler(ctx *context.Context) { if !isOidValid(p.Oid) { log.Info("Invalid LFS OID[%s] attempt to POST in %s/%s", p.Oid, rc.User, rc.Repo) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } if setting.LFS.MaxFileSize > 0 && p.Size > setting.LFS.MaxFileSize { log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", p.Oid, p.Size, rc.User, rc.Repo, setting.LFS.MaxFileSize) - writeStatus(ctx, 413) + writeStatus(ctx, http.StatusRequestEntityTooLarge) return } meta, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) if err != nil { log.Error("Unable to write LFS OID[%s] size %d meta object in %v/%v to database. Error: %v", p.Oid, p.Size, rc.User, rc.Repo, err) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) - sentStatus := 202 + sentStatus := http.StatusAccepted contentStore := lfs_module.NewContentStore() exist, err := contentStore.Exists(p) if err != nil { log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", p.Oid, rc.User, rc.Repo, err) - writeStatus(ctx, 500) + writeStatus(ctx, http.StatusInternalServerError) return } if meta.Existing && exist { - sentStatus = 200 + sentStatus = http.StatusOK } ctx.Resp.WriteHeader(sentStatus) @@ -283,13 +283,13 @@ func PostHandler(ctx *context.Context) { func BatchHandler(ctx *context.Context) { if !setting.LFS.StartServer { log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } if !MetaMatcher(ctx.Req) { log.Info("Attempt to BATCH without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, 400) + writeStatus(ctx, http.StatusBadRequest) return } @@ -313,7 +313,7 @@ func BatchHandler(ctx *context.Context) { repository, err := models.GetRepositoryByOwnerAndName(reqCtx.User, reqCtx.Repo) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", reqCtx.User, reqCtx.Repo, err) - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } @@ -334,7 +334,7 @@ func BatchHandler(ctx *context.Context) { exist, err := contentStore.Exists(meta.Pointer) if err != nil { log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) - writeStatus(ctx, 500) + writeStatus(ctx, http.StatusInternalServerError) return } if exist { @@ -345,7 +345,7 @@ func BatchHandler(ctx *context.Context) { if requireWrite && setting.LFS.MaxFileSize > 0 && object.Size > setting.LFS.MaxFileSize { log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) - writeStatus(ctx, 413) + writeStatus(ctx, http.StatusRequestEntityTooLarge) return } @@ -355,7 +355,7 @@ func BatchHandler(ctx *context.Context) { exist, err := contentStore.Exists(meta.Pointer) if err != nil { log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) - writeStatus(ctx, 500) + writeStatus(ctx, http.StatusInternalServerError) return } responseObjects = append(responseObjects, represent(reqCtx, meta.Pointer, meta.Existing, !exist)) @@ -373,7 +373,7 @@ func BatchHandler(ctx *context.Context) { if err := enc.Encode(respobj); err != nil { log.Error("Failed to encode representation as json. Error: %v", err) } - logRequest(ctx.Req, 200) + logRequest(ctx.Req, http.StatusOK) } // PutHandler receives data from the client and puts it into the content store @@ -390,7 +390,7 @@ func PutHandler(ctx *context.Context) { defer ctx.Req.Body.Close() if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil { // Put will log the error itself - ctx.Resp.WriteHeader(500) + ctx.Resp.WriteHeader(http.StatusInternalServerError) if err == lfs_module.ErrSizeMismatch || err == lfs_module.ErrHashMismatch { fmt.Fprintf(ctx.Resp, `{"message":"%s"}`, err) } else { @@ -402,20 +402,20 @@ func PutHandler(ctx *context.Context) { return } - logRequest(ctx.Req, 200) + logRequest(ctx.Req, http.StatusOK) } // VerifyHandler verify oid and its size from the content store func VerifyHandler(ctx *context.Context) { if !setting.LFS.StartServer { log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, 404) + writeStatus(ctx, http.StatusNotFound) return } if !MetaMatcher(ctx.Req) { log.Info("Attempt to VERIFY without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, 400) + writeStatus(ctx, http.StatusBadRequest) return } @@ -431,16 +431,16 @@ func VerifyHandler(ctx *context.Context) { ok, err := contentStore.Verify(meta.Pointer) if err != nil { // Error will be logged in Verify - ctx.Resp.WriteHeader(500) + ctx.Resp.WriteHeader(http.StatusInternalServerError) fmt.Fprintf(ctx.Resp, `{"message":"Internal Server Error"}`) return } if !ok { - writeStatus(ctx, 422) + writeStatus(ctx, http.StatusUnprocessableEntity) return } - logRequest(ctx.Req, 200) + logRequest(ctx.Req, http.StatusOK) } // represent takes a requestContext and Meta and turns it into a ObjectResponse suitable @@ -662,5 +662,5 @@ func parseToken(authorization string) (*models.User, *models.Repository, string, func requireAuth(ctx *context.Context) { ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") - writeStatus(ctx, 401) + writeStatus(ctx, http.StatusUnauthorized) } From 9b8936a7a1fa67d065a163e9270d27c37cb80566 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 13 Apr 2021 14:30:03 +0000 Subject: [PATCH 02/30] Use IsValid instead of isOidValid --- services/lfs/server.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 0fa8bd812035d..260ddf5f44efd 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -49,12 +49,6 @@ func (rc *requestContext) VerifyLink() string { return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify") } -var oidRegExp = regexp.MustCompile(`^[A-Fa-f0-9]+$`) - -func isOidValid(oid string) bool { - return oidRegExp.MatchString(oid) -} - // ObjectOidHandler is the main request routing entry point into LFS server functions func ObjectOidHandler(ctx *context.Context) { if !setting.LFS.StartServer { @@ -81,7 +75,7 @@ func ObjectOidHandler(ctx *context.Context) { } func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) (*models.LFSMetaObject, *models.Repository) { - if !isOidValid(p.Oid) { + if !p.IsValid() { log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) writeStatus(ctx, http.StatusNotFound) return nil, nil @@ -237,7 +231,7 @@ func PostHandler(ctx *context.Context) { return } - if !isOidValid(p.Oid) { + if !p.IsValid() { log.Info("Invalid LFS OID[%s] attempt to POST in %s/%s", p.Oid, rc.User, rc.Repo) writeStatus(ctx, http.StatusNotFound) return @@ -305,7 +299,7 @@ func BatchHandler(ctx *context.Context) { // Create a response object for _, object := range bv.Objects { - if !isOidValid(object.Oid) { + if !object.IsValid() { log.Info("Invalid LFS OID[%s] attempt to BATCH in %s/%s", object.Oid, reqCtx.User, reqCtx.Repo) continue } From 6a8a4a94fa57e6b941be4319245489f2b30e34b5 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 13 Apr 2021 14:50:20 +0000 Subject: [PATCH 03/30] Restructured code. Moved static checks out of loop. --- services/lfs/server.go | 57 +++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 260ddf5f44efd..9c89ae5080c40 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -295,6 +295,25 @@ func BatchHandler(ctx *context.Context) { Authorization: ctx.Req.Header.Get("Authorization"), } + repository, err := models.GetRepositoryByOwnerAndName(reqCtx.User, reqCtx.Repo) + if err != nil { + log.Error("Unable to get repository: %s/%s Error: %v", reqCtx.User, reqCtx.Repo, err) + writeStatus(ctx, http.StatusNotFound) + return + } + + requireWrite := false + if bv.Operation == "upload" { + requireWrite = true + } + + if !authenticate(ctx, repository, reqCtx.Authorization, requireWrite) { + requireAuth(ctx) + return + } + + contentStore := lfs_module.NewContentStore() + var responseObjects []*lfs_module.ObjectResponse // Create a response object @@ -304,54 +323,30 @@ func BatchHandler(ctx *context.Context) { continue } - repository, err := models.GetRepositoryByOwnerAndName(reqCtx.User, reqCtx.Repo) - if err != nil { - log.Error("Unable to get repository: %s/%s Error: %v", reqCtx.User, reqCtx.Repo, err) - writeStatus(ctx, http.StatusNotFound) + if requireWrite && setting.LFS.MaxFileSize > 0 && object.Size > setting.LFS.MaxFileSize { + log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) + writeStatus(ctx, http.StatusRequestEntityTooLarge) return } - requireWrite := false - if bv.Operation == "upload" { - requireWrite = true - } - - if !authenticate(ctx, repository, reqCtx.Authorization, requireWrite) { - requireAuth(ctx) + exist, err := contentStore.Exists(object) + if err != nil { + log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) + writeStatus(ctx, http.StatusInternalServerError) return } - contentStore := lfs_module.NewContentStore() - meta, err := repository.GetLFSMetaObjectByOid(object.Oid) if err == nil { // Object is found and exists - exist, err := contentStore.Exists(meta.Pointer) - if err != nil { - log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) - writeStatus(ctx, http.StatusInternalServerError) - return - } if exist { responseObjects = append(responseObjects, represent(reqCtx, meta.Pointer, true, false)) continue } } - if requireWrite && setting.LFS.MaxFileSize > 0 && object.Size > setting.LFS.MaxFileSize { - log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) - writeStatus(ctx, http.StatusRequestEntityTooLarge) - return - } - // Object is not found meta, err = models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: object, RepositoryID: repository.ID}) if err == nil { - exist, err := contentStore.Exists(meta.Pointer) - if err != nil { - log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) - writeStatus(ctx, http.StatusInternalServerError) - return - } responseObjects = append(responseObjects, represent(reqCtx, meta.Pointer, meta.Existing, !exist)) } else { log.Error("Unable to write LFS OID[%s] size %d meta object in %v/%v to database. Error: %v", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, err) From 5f68237aeeb43ba4a36f83689256c858a1dd76fe Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Wed, 14 Apr 2021 21:24:43 +0000 Subject: [PATCH 04/30] Made method private. --- services/lfs/locks.go | 2 +- services/lfs/server.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 6bbe43d36bbb2..6d33f291baecf 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -26,7 +26,7 @@ func checkIsValidRequest(ctx *context.Context) bool { writeStatus(ctx, http.StatusNotFound) return false } - if !MetaMatcher(ctx.Req) { + if !isValidAccept(ctx.Req) { log.Info("Attempt access LOCKs without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) return false diff --git a/services/lfs/server.go b/services/lfs/server.go index 9c89ae5080c40..5e5ec3bd9bd26 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -58,7 +58,7 @@ func ObjectOidHandler(ctx *context.Context) { } if ctx.Req.Method == "GET" || ctx.Req.Method == "HEAD" { - if MetaMatcher(ctx.Req) { + if isValidAccept(ctx.Req) { getMetaHandler(ctx) return } @@ -211,7 +211,7 @@ func PostHandler(ctx *context.Context) { return } - if !MetaMatcher(ctx.Req) { + if !isValidAccept(ctx.Req) { log.Info("Attempt to POST without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) return @@ -281,7 +281,7 @@ func BatchHandler(ctx *context.Context) { return } - if !MetaMatcher(ctx.Req) { + if !isValidAccept(ctx.Req) { log.Info("Attempt to BATCH without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) return @@ -402,7 +402,7 @@ func VerifyHandler(ctx *context.Context) { return } - if !MetaMatcher(ctx.Req) { + if !isValidAccept(ctx.Req) { log.Info("Attempt to VERIFY without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) return @@ -473,9 +473,9 @@ func represent(rc *requestContext, pointer lfs_module.Pointer, download, upload return rep } -// MetaMatcher provides a mux.MatcherFunc that only allows requests that contain +// isValidAccept provides a mux.MatcherFunc that only allows requests that contain // an Accept header with the lfs_module.MediaType -func MetaMatcher(r *http.Request) bool { +func isValidAccept(r *http.Request) bool { mediaParts := strings.Split(r.Header.Get("Accept"), ";") mt := mediaParts[0] return mt == lfs_module.MediaType From 7d0e8d85afa5a41e2b84a3b98ef84860218b13ff Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Wed, 14 Apr 2021 23:02:42 +0000 Subject: [PATCH 05/30] Restructured batch api. Add support for individual errors. --- routers/routes/web.go | 2 +- services/lfs/server.go | 132 ++++++++++++++++++++++------------------- 2 files changed, 72 insertions(+), 62 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 4815680c99e7d..cf80ecac682c0 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -1098,7 +1098,7 @@ func RegisterRoutes(m *web.Route) { m.Get("/objects/{oid}/{filename}", lfs.ObjectOidHandler) m.Any("/objects/{oid}", lfs.ObjectOidHandler) m.Post("/objects", lfs.PostHandler) - m.Post("/verify", lfs.VerifyHandler) + m.Post("/verify/{oid}", lfs.VerifyHandler) m.Group("/locks", func() { m.Get("/", lfs.GetListLockHandler) m.Post("/", lfs.PostLockHandler) diff --git a/services/lfs/server.go b/services/lfs/server.go index 5e5ec3bd9bd26..7d8548f7908b9 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -45,8 +45,8 @@ func (rc *requestContext) ObjectLink(oid string) string { } // VerifyLink builds a URL for verifying the object. -func (rc *requestContext) VerifyLink() string { - return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify") +func (rc *requestContext) VerifyLink(oid string) string { + return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify", oid) } // ObjectOidHandler is the main request routing entry point into LFS server functions @@ -195,7 +195,7 @@ func getMetaHandler(ctx *context.Context) { if ctx.Req.Method == "GET" { json := jsoniter.ConfigCompatibleWithStandardLibrary enc := json.NewEncoder(ctx.Resp) - if err := enc.Encode(represent(rc, meta.Pointer, true, false)); err != nil { + if err := enc.Encode(buildObjectResponse(rc, meta.Pointer, true, false, 0)); err != nil { log.Error("Failed to encode representation as json. Error: %v", err) } } @@ -267,7 +267,7 @@ func PostHandler(ctx *context.Context) { json := jsoniter.ConfigCompatibleWithStandardLibrary enc := json.NewEncoder(ctx.Resp) - if err := enc.Encode(represent(rc, meta.Pointer, meta.Existing, true)); err != nil { + if err := enc.Encode(buildObjectResponse(rc, meta.Pointer, meta.Existing, true, 0)); err != nil { log.Error("Failed to encode representation as json. Error: %v", err) } logRequest(ctx.Req, sentStatus) @@ -289,6 +289,17 @@ func BatchHandler(ctx *context.Context) { bv := unpackbatch(ctx) + var isUpload bool + if bv.Operation == "upload" { + isUpload = true + } else if bv.Operation == "download" { + isUpload = false + } else { + log.Info("Attempt to BATCH with invalid operation: %s", bv.Operation) + writeStatus(ctx, http.StatusBadRequest) + return + } + reqCtx := &requestContext{ User: ctx.Params("username"), Repo: strings.TrimSuffix(ctx.Params("reponame"), ".git"), @@ -302,12 +313,7 @@ func BatchHandler(ctx *context.Context) { return } - requireWrite := false - if bv.Operation == "upload" { - requireWrite = true - } - - if !authenticate(ctx, repository, reqCtx.Authorization, requireWrite) { + if !authenticate(ctx, repository, reqCtx.Authorization, isUpload) { requireAuth(ctx) return } @@ -316,41 +322,57 @@ func BatchHandler(ctx *context.Context) { var responseObjects []*lfs_module.ObjectResponse - // Create a response object for _, object := range bv.Objects { if !object.IsValid() { - log.Info("Invalid LFS OID[%s] attempt to BATCH in %s/%s", object.Oid, reqCtx.User, reqCtx.Repo) + responseObjects = append(responseObjects, buildDownloadObjectResponse(reqCtx, object, http.StatusUnprocessableEntity)) continue } - if requireWrite && setting.LFS.MaxFileSize > 0 && object.Size > setting.LFS.MaxFileSize { - log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) - writeStatus(ctx, http.StatusRequestEntityTooLarge) + exist, err := contentStore.Exists(object) + if err != nil { + log.Error("Unable to check if LFS OID[%s] exist on %s/%s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) + writeStatus(ctx, http.StatusInternalServerError) return } - exist, err := contentStore.Exists(object) - if err != nil { - log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) + meta, metaErr := repository.GetLFSMetaObjectByOid(object.Oid) + if metaErr != nil && metaErr != models.ErrLFSObjectNotExist { + log.Error("Unable to get LFS MetaObject [%s] for %s/%s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, metaErr) writeStatus(ctx, http.StatusInternalServerError) return } - meta, err := repository.GetLFSMetaObjectByOid(object.Oid) - if err == nil { // Object is found and exists - if exist { - responseObjects = append(responseObjects, represent(reqCtx, meta.Pointer, true, false)) - continue + var responseObject *lfs_module.ObjectResponse + if isUpload { + if !exists && setting.LFS.MaxFileSize > 0 && object.Size > setting.LFS.MaxFileSize { + log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) + writeStatus(ctx, http.StatusRequestEntityTooLarge) + return } - } - // Object is not found - meta, err = models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: object, RepositoryID: repository.ID}) - if err == nil { - responseObjects = append(responseObjects, represent(reqCtx, meta.Pointer, meta.Existing, !exist)) + if exists { + if meta == nil { + _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: object, RepositoryID: repository.ID}) + if err != nil { + log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, metaErr) + writeStatus(ctx, http.StatusInternalServerError) + return + } + } + } + + responseObject = buildObjectResponse(reqCtx, object, false, !exists, 0) } else { - log.Error("Unable to write LFS OID[%s] size %d meta object in %v/%v to database. Error: %v", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, err) + errorCode := 0 + if !exist || meta == nil { + errorCode = http.StatusNotFound + } else if meta.Size != object.Size { + errorCode = http.StatusUnprocessableEntity + } + + responseObject = buildObjectResponse(reqCtx, object, true, false, errorCode) } + responseObjects = append(responseObjects, responseObject) } ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) @@ -432,44 +454,32 @@ func VerifyHandler(ctx *context.Context) { logRequest(ctx.Req, http.StatusOK) } -// represent takes a requestContext and Meta and turns it into a ObjectResponse suitable -// for json encoding -func represent(rc *requestContext, pointer lfs_module.Pointer, download, upload bool) *lfs_module.ObjectResponse { - rep := &lfs_module.ObjectResponse{ - Pointer: pointer, - Actions: make(map[string]*lfs_module.Link), - } - - header := make(map[string]string) - - if rc.Authorization == "" { - //https://github.com/github/git-lfs/issues/1088 - header["Authorization"] = "Authorization: Basic dummy" +func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, download, upload bool, errorCode int) *lfs_module.ObjectResponse { + rep := &lfs_module.ObjectResponse{Pointer: pointer} + if errorCode > 0 { + rep.Error = &ObjectError{ + Code: errorCode, + Message: http.StatusText(errorCode), + } } else { - header["Authorization"] = rc.Authorization - } + rep.Actions = make(map[string]*lfs_module.Link) - if download { - rep.Actions["download"] = &lfs_module.Link{Href: rc.ObjectLink(pointer.Oid), Header: header} - } - - if upload { - rep.Actions["upload"] = &lfs_module.Link{Href: rc.ObjectLink(pointer.Oid), Header: header} - } - - if upload && !download { - // Force client side verify action while gitea lacks proper server side verification + header := make(map[string]string) verifyHeader := make(map[string]string) - for k, v := range header { - verifyHeader[k] = v + + if len(rc.Authorization) > 0 { + header["Authorization"] = rc.Authorization + verifyHeader["Authorization"] = rc.Authorization } - // This is only needed to workaround https://github.com/git-lfs/git-lfs/issues/3662 - verifyHeader["Accept"] = lfs_module.MediaType - - rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(), Header: verifyHeader} + if download { + rep.Actions["download"] = &lfs_module.Link{Href: rc.ObjectLink(pointer.Oid), Header: header} + } + if upload { + rep.Actions["upload"] = &lfs_module.Link{Href: rc.ObjectLink(pointer.Oid), Header: header} + rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(pointer.Oid), Header: verifyHeader} + } } - return rep } From c51ac320415120f546e157aae94ba28735b7f176 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 15 Apr 2021 16:13:00 +0200 Subject: [PATCH 06/30] Let router decide if LFS is enabled. --- routers/routes/web.go | 9 ++++++++- services/lfs/server.go | 24 ------------------------ 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index cf80ecac682c0..4cc07d7608535 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -331,6 +331,13 @@ func RegisterRoutes(m *web.Route) { } } + lfsServerEnabled := func(ctx *context.Context) { + if !setting.LFS.StartServer { + ctx.Error(http.StatusForbidden) + return + } + } + // FIXME: not all routes need go through same middleware. // Especially some AJAX requests, we can reduce middleware number to improve performance. // Routers. @@ -1108,7 +1115,7 @@ func RegisterRoutes(m *web.Route) { m.Any("/*", func(ctx *context.Context) { ctx.NotFound("", nil) }) - }, ignSignInAndCsrf) + }, ignSignInAndCsrf, lfsServerEnabled) m.Group("", func() { m.Post("/git-upload-pack", repo.ServiceUploadPack) diff --git a/services/lfs/server.go b/services/lfs/server.go index 7d8548f7908b9..587c8e6b1a824 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -51,12 +51,6 @@ func (rc *requestContext) VerifyLink(oid string) string { // ObjectOidHandler is the main request routing entry point into LFS server functions func ObjectOidHandler(ctx *context.Context) { - if !setting.LFS.StartServer { - log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, http.StatusNotFound) - return - } - if ctx.Req.Method == "GET" || ctx.Req.Method == "HEAD" { if isValidAccept(ctx.Req) { getMetaHandler(ctx) @@ -205,12 +199,6 @@ func getMetaHandler(ctx *context.Context) { // PostHandler instructs the client how to upload data func PostHandler(ctx *context.Context) { - if !setting.LFS.StartServer { - log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, http.StatusNotFound) - return - } - if !isValidAccept(ctx.Req) { log.Info("Attempt to POST without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) @@ -275,12 +263,6 @@ func PostHandler(ctx *context.Context) { // BatchHandler provides the batch api func BatchHandler(ctx *context.Context) { - if !setting.LFS.StartServer { - log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, http.StatusNotFound) - return - } - if !isValidAccept(ctx.Req) { log.Info("Attempt to BATCH without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) @@ -418,12 +400,6 @@ func PutHandler(ctx *context.Context) { // VerifyHandler verify oid and its size from the content store func VerifyHandler(ctx *context.Context) { - if !setting.LFS.StartServer { - log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, http.StatusNotFound) - return - } - if !isValidAccept(ctx.Req) { log.Info("Attempt to VERIFY without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) From 406127bc7e019b39c146acc6292ef53d61618016 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 15 Apr 2021 16:17:00 +0200 Subject: [PATCH 07/30] Renamed methods. --- routers/routes/web.go | 8 +++++--- services/lfs/server.go | 41 ++++++++++++++--------------------------- 2 files changed, 19 insertions(+), 30 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 4cc07d7608535..5cceaa0e5ad3d 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -1102,9 +1102,11 @@ func RegisterRoutes(m *web.Route) { m.Group("/{reponame}", func() { m.Group("/info/lfs", func() { m.Post("/objects/batch", lfs.BatchHandler) - m.Get("/objects/{oid}/{filename}", lfs.ObjectOidHandler) - m.Any("/objects/{oid}", lfs.ObjectOidHandler) - m.Post("/objects", lfs.PostHandler) + m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) + m.Get("/objects/{oid}", lfs.DownloadHandler) + m.Put("/objects/{oid}", lfs.UploadHandler) + m.Any("/objects/{oid}", lfs.LegacyMetaHandler) + m.Post("/objects", lfs.LegacyPostHandler) m.Post("/verify/{oid}", lfs.VerifyHandler) m.Group("/locks", func() { m.Get("/", lfs.GetListLockHandler) diff --git a/services/lfs/server.go b/services/lfs/server.go index 587c8e6b1a824..30bd3e122f0e9 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -49,25 +49,6 @@ func (rc *requestContext) VerifyLink(oid string) string { return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify", oid) } -// ObjectOidHandler is the main request routing entry point into LFS server functions -func ObjectOidHandler(ctx *context.Context) { - if ctx.Req.Method == "GET" || ctx.Req.Method == "HEAD" { - if isValidAccept(ctx.Req) { - getMetaHandler(ctx) - return - } - - getContentHandler(ctx) - return - } else if ctx.Req.Method == "PUT" { - PutHandler(ctx) - return - } - - log.Warn("Unhandled LFS method: %s for %s/%s OID[%s]", ctx.Req.Method, ctx.Params("username"), ctx.Params("reponame"), ctx.Params("oid")) - writeStatus(ctx, http.StatusNotFound) -} - func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) (*models.LFSMetaObject, *models.Repository) { if !p.IsValid() { log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) @@ -97,8 +78,8 @@ func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs return meta, repository } -// getContentHandler gets the content from the content store -func getContentHandler(ctx *context.Context) { +// DownloadHandler gets the content from the content store +func DownloadHandler(ctx *context.Context) { rc, p := unpack(ctx) meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, false) @@ -174,8 +155,14 @@ func getContentHandler(ctx *context.Context) { logRequest(ctx.Req, statusCode) } -// getMetaHandler retrieves metadata about the object -func getMetaHandler(ctx *context.Context) { +// LegacyMetaHandler retrieves metadata about the object +func LegacyMetaHandler(ctx *context.Context) { + if !isValidAccept(ctx.Req) { + log.Info("Attempt to call without accepting the correct media type: %s", lfs_module.MediaType) + writeStatus(ctx, http.StatusBadRequest) + return + } + rc, p := unpack(ctx) meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, false) @@ -197,8 +184,8 @@ func getMetaHandler(ctx *context.Context) { logRequest(ctx.Req, http.StatusOK) } -// PostHandler instructs the client how to upload data -func PostHandler(ctx *context.Context) { +// LegacyPostHandler instructs the client how to upload data +func LegacyPostHandler(ctx *context.Context) { if !isValidAccept(ctx.Req) { log.Info("Attempt to POST without accepting the correct media type: %s", lfs_module.MediaType) writeStatus(ctx, http.StatusBadRequest) @@ -369,8 +356,8 @@ func BatchHandler(ctx *context.Context) { logRequest(ctx.Req, http.StatusOK) } -// PutHandler receives data from the client and puts it into the content store -func PutHandler(ctx *context.Context) { +// UploadHandler receives data from the client and puts it into the content store +func UploadHandler(ctx *context.Context) { rc, p := unpack(ctx) meta, repository := getAuthenticatedRepoAndMeta(ctx, rc, p, true) From e5bcd0dbf3e77dc6c9a5355ae244a5a6ee2ed1a0 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 15 Apr 2021 14:40:21 +0000 Subject: [PATCH 08/30] Return correct status from verify handler. --- services/lfs/server.go | 64 ++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 30bd3e122f0e9..6feb7d66f1636 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -269,11 +269,7 @@ func BatchHandler(ctx *context.Context) { return } - reqCtx := &requestContext{ - User: ctx.Params("username"), - Repo: strings.TrimSuffix(ctx.Params("reponame"), ".git"), - Authorization: ctx.Req.Header.Get("Authorization"), - } + reqCtx := getRequestContext(ctx) repository, err := models.GetRepositoryByOwnerAndName(reqCtx.User, reqCtx.Repo) if err != nil { @@ -291,22 +287,22 @@ func BatchHandler(ctx *context.Context) { var responseObjects []*lfs_module.ObjectResponse - for _, object := range bv.Objects { + for _, p := range bv.Objects { if !object.IsValid() { - responseObjects = append(responseObjects, buildDownloadObjectResponse(reqCtx, object, http.StatusUnprocessableEntity)) + responseObjects = append(responseObjects, buildObjectResponse(reqCtx, p, false, false, http.StatusUnprocessableEntity)) continue } - exist, err := contentStore.Exists(object) + exists, err := contentStore.Exists(p) if err != nil { - log.Error("Unable to check if LFS OID[%s] exist on %s/%s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, err) + log.Error("Unable to check if LFS OID[%s] exist on %s/%s. Error: %v", p.Oid, reqCtx.User, reqCtx.Repo, err) writeStatus(ctx, http.StatusInternalServerError) return } - meta, metaErr := repository.GetLFSMetaObjectByOid(object.Oid) + meta, metaErr := repository.GetLFSMetaObjectByOid(p.Oid) if metaErr != nil && metaErr != models.ErrLFSObjectNotExist { - log.Error("Unable to get LFS MetaObject [%s] for %s/%s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, metaErr) + log.Error("Unable to get LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, reqCtx.User, reqCtx.Repo, metaErr) writeStatus(ctx, http.StatusInternalServerError) return } @@ -314,32 +310,32 @@ func BatchHandler(ctx *context.Context) { var responseObject *lfs_module.ObjectResponse if isUpload { if !exists && setting.LFS.MaxFileSize > 0 && object.Size > setting.LFS.MaxFileSize { - log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", object.Oid, object.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) + log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", p.Oid, p.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) writeStatus(ctx, http.StatusRequestEntityTooLarge) return } if exists { if meta == nil { - _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: object, RepositoryID: repository.ID}) + _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) if err != nil { - log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", object.Oid, reqCtx.User, reqCtx.Repo, metaErr) + log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, reqCtx.User, reqCtx.Repo, metaErr) writeStatus(ctx, http.StatusInternalServerError) return } } } - responseObject = buildObjectResponse(reqCtx, object, false, !exists, 0) + responseObject = buildObjectResponse(reqCtx, p, false, !exists, 0) } else { errorCode := 0 - if !exist || meta == nil { + if !exists || meta == nil { errorCode = http.StatusNotFound - } else if meta.Size != object.Size { + } else if meta.Size != p.Size { errorCode = http.StatusUnprocessableEntity } - responseObject = buildObjectResponse(reqCtx, object, true, false, errorCode) + responseObject = buildObjectResponse(reqCtx, p, true, false, errorCode) } responseObjects = append(responseObjects, responseObject) } @@ -397,7 +393,6 @@ func VerifyHandler(ctx *context.Context) { meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, true) if meta == nil { - // Status already written in getAuthenticatedRepoAndMeta return } @@ -409,19 +404,27 @@ func VerifyHandler(ctx *context.Context) { fmt.Fprintf(ctx.Resp, `{"message":"Internal Server Error"}`) return } + + status := http.StatusOK if !ok { - writeStatus(ctx, http.StatusUnprocessableEntity) - return + status = http.StatusUnprocessableEntity } + writeStatus(ctx, status) +} - logRequest(ctx.Req, http.StatusOK) +func getRequestContext(ctx *context.Context) *requestContext { + return &requestContext{ + User: ctx.Params("username"), + Repo: strings.TrimSuffix(ctx.Params("reponame"), ".git"), + Authorization: ctx.Req.Header.Get("Authorization"), + } } func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, download, upload bool, errorCode int) *lfs_module.ObjectResponse { rep := &lfs_module.ObjectResponse{Pointer: pointer} if errorCode > 0 { - rep.Error = &ObjectError{ - Code: errorCode, + rep.Error = &lfs_module.ObjectError{ + Code: errorCode, Message: http.StatusText(errorCode), } } else { @@ -429,7 +432,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa header := make(map[string]string) verifyHeader := make(map[string]string) - + if len(rc.Authorization) > 0 { header["Authorization"] = rc.Authorization verifyHeader["Authorization"] = rc.Authorization @@ -455,17 +458,12 @@ func isValidAccept(r *http.Request) bool { } func unpack(ctx *context.Context) (*requestContext, lfs_module.Pointer) { - r := ctx.Req - rc := &requestContext{ - User: ctx.Params("username"), - Repo: strings.TrimSuffix(ctx.Params("reponame"), ".git"), - Authorization: r.Header.Get("Authorization"), - } + rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.Params("oid")} - if r.Method == "POST" { // Maybe also check if +json + if ctx.Req.Method == "POST" { // Maybe also check if +json var p2 lfs_module.Pointer - bodyReader := r.Body + bodyReader := ctx.Req.Body defer bodyReader.Close() json := jsoniter.ConfigCompatibleWithStandardLibrary dec := json.NewDecoder(bodyReader) From 2b9daab90901f8ace8a3e801e226ac6f0214d1d2 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 15 Apr 2021 14:41:50 +0000 Subject: [PATCH 09/30] Unified media type check in router. --- routers/routes/web.go | 8 ++++---- services/lfs/server.go | 43 +++++++++++------------------------------- 2 files changed, 15 insertions(+), 36 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 5cceaa0e5ad3d..ddfafbb94d6c8 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -1101,13 +1101,13 @@ func RegisterRoutes(m *web.Route) { m.Group("/{reponame}", func() { m.Group("/info/lfs", func() { - m.Post("/objects/batch", lfs.BatchHandler) + m.Post("/objects/batch", lfs.CheckAcceptMediaType, lfs.BatchHandler) m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) m.Get("/objects/{oid}", lfs.DownloadHandler) m.Put("/objects/{oid}", lfs.UploadHandler) - m.Any("/objects/{oid}", lfs.LegacyMetaHandler) - m.Post("/objects", lfs.LegacyPostHandler) - m.Post("/verify/{oid}", lfs.VerifyHandler) + m.Any("/objects/{oid}", lfs.CheckAcceptMediaType, lfs.LegacyMetaHandler) + m.Post("/objects", lfs.CheckAcceptMediaType, lfs.LegacyPostHandler) + m.Post("/verify/{oid}", lfs.CheckAcceptMediaType, lfs.VerifyHandler) m.Group("/locks", func() { m.Get("/", lfs.GetListLockHandler) m.Post("/", lfs.PostLockHandler) diff --git a/services/lfs/server.go b/services/lfs/server.go index 6feb7d66f1636..0e02a77df2d16 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -49,6 +49,17 @@ func (rc *requestContext) VerifyLink(oid string) string { return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify", oid) } +// CheckAcceptMediaType checks if the client accepts the LFS media type. +func CheckAcceptMediaType(ctx *context.Context) { + mediaParts := strings.Split(r.Header.Get("Accept"), ";") + + if mediaParts[0] != lfs_module.MediaType { + log.Info("Calling a LFS method without accepting the correct media type: %s", lfs_module.MediaType) + writeStatus(ctx, http.StatusBadRequest) + return + } +} + func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) (*models.LFSMetaObject, *models.Repository) { if !p.IsValid() { log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) @@ -157,12 +168,6 @@ func DownloadHandler(ctx *context.Context) { // LegacyMetaHandler retrieves metadata about the object func LegacyMetaHandler(ctx *context.Context) { - if !isValidAccept(ctx.Req) { - log.Info("Attempt to call without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, http.StatusBadRequest) - return - } - rc, p := unpack(ctx) meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, false) @@ -186,12 +191,6 @@ func LegacyMetaHandler(ctx *context.Context) { // LegacyPostHandler instructs the client how to upload data func LegacyPostHandler(ctx *context.Context) { - if !isValidAccept(ctx.Req) { - log.Info("Attempt to POST without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, http.StatusBadRequest) - return - } - rc, p := unpack(ctx) repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) @@ -250,12 +249,6 @@ func LegacyPostHandler(ctx *context.Context) { // BatchHandler provides the batch api func BatchHandler(ctx *context.Context) { - if !isValidAccept(ctx.Req) { - log.Info("Attempt to BATCH without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, http.StatusBadRequest) - return - } - bv := unpackbatch(ctx) var isUpload bool @@ -383,12 +376,6 @@ func UploadHandler(ctx *context.Context) { // VerifyHandler verify oid and its size from the content store func VerifyHandler(ctx *context.Context) { - if !isValidAccept(ctx.Req) { - log.Info("Attempt to VERIFY without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, http.StatusBadRequest) - return - } - rc, p := unpack(ctx) meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, true) @@ -449,14 +436,6 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa return rep } -// isValidAccept provides a mux.MatcherFunc that only allows requests that contain -// an Accept header with the lfs_module.MediaType -func isValidAccept(r *http.Request) bool { - mediaParts := strings.Split(r.Header.Get("Accept"), ";") - mt := mediaParts[0] - return mt == lfs_module.MediaType -} - func unpack(ctx *context.Context) (*requestContext, lfs_module.Pointer) { rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.Params("oid")} From dad75219ca81febde5eb281fc107ffce62250c13 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 15 Apr 2021 15:00:49 +0000 Subject: [PATCH 10/30] Changed error code according to spec. --- services/lfs/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 0e02a77df2d16..e02ae79bd5255 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -55,7 +55,7 @@ func CheckAcceptMediaType(ctx *context.Context) { if mediaParts[0] != lfs_module.MediaType { log.Info("Calling a LFS method without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, http.StatusBadRequest) + writeStatus(ctx, http.StatusNotAcceptable) return } } From f3fe4dedac9e3c2df1821efb39b9aa4d1d3df5a6 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 15 Apr 2021 16:35:00 +0200 Subject: [PATCH 11/30] Moved checks into router. --- routers/routes/web.go | 2 +- services/lfs/locks.go | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index ddfafbb94d6c8..05ffc1e205eb2 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -1113,7 +1113,7 @@ func RegisterRoutes(m *web.Route) { m.Post("/", lfs.PostLockHandler) m.Post("/verify", lfs.VerifyLockHandler) m.Post("/{lid}/unlock", lfs.UnLockHandler) - }) + }, lfs.CheckAcceptMediaType) m.Any("/*", func(ctx *context.Context) { ctx.NotFound("", nil) }) diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 6d33f291baecf..e4a06328c1c08 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -21,16 +21,6 @@ import ( //checkIsValidRequest check if it a valid request in case of bad request it write the response to ctx. func checkIsValidRequest(ctx *context.Context) bool { - if !setting.LFS.StartServer { - log.Debug("Attempt to access LFS server but LFS server is disabled") - writeStatus(ctx, http.StatusNotFound) - return false - } - if !isValidAccept(ctx.Req) { - log.Info("Attempt access LOCKs without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, http.StatusBadRequest) - return false - } if !ctx.IsSigned { user, _, _, err := parseToken(ctx.Req.Header.Get("Authorization")) if err != nil { From 89607a293db848e155a4b46c79b74aa3b4874f25 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Thu, 15 Apr 2021 19:02:00 +0200 Subject: [PATCH 12/30] Removed invalid v1 api methods. --- routers/routes/web.go | 2 -- services/lfs/server.go | 81 ------------------------------------------ 2 files changed, 83 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 05ffc1e205eb2..b98eb0167e866 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -1105,8 +1105,6 @@ func RegisterRoutes(m *web.Route) { m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) m.Get("/objects/{oid}", lfs.DownloadHandler) m.Put("/objects/{oid}", lfs.UploadHandler) - m.Any("/objects/{oid}", lfs.CheckAcceptMediaType, lfs.LegacyMetaHandler) - m.Post("/objects", lfs.CheckAcceptMediaType, lfs.LegacyPostHandler) m.Post("/verify/{oid}", lfs.CheckAcceptMediaType, lfs.VerifyHandler) m.Group("/locks", func() { m.Get("/", lfs.GetListLockHandler) diff --git a/services/lfs/server.go b/services/lfs/server.go index e02ae79bd5255..bbc1ecd4f9760 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -166,87 +166,6 @@ func DownloadHandler(ctx *context.Context) { logRequest(ctx.Req, statusCode) } -// LegacyMetaHandler retrieves metadata about the object -func LegacyMetaHandler(ctx *context.Context) { - rc, p := unpack(ctx) - - meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, false) - if meta == nil { - // Status already written in getAuthenticatedRepoAndMeta - return - } - - ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) - - if ctx.Req.Method == "GET" { - json := jsoniter.ConfigCompatibleWithStandardLibrary - enc := json.NewEncoder(ctx.Resp) - if err := enc.Encode(buildObjectResponse(rc, meta.Pointer, true, false, 0)); err != nil { - log.Error("Failed to encode representation as json. Error: %v", err) - } - } - - logRequest(ctx.Req, http.StatusOK) -} - -// LegacyPostHandler instructs the client how to upload data -func LegacyPostHandler(ctx *context.Context) { - rc, p := unpack(ctx) - - repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) - if err != nil { - log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) - writeStatus(ctx, http.StatusNotFound) - return - } - - if !authenticate(ctx, repository, rc.Authorization, true) { - requireAuth(ctx) - return - } - - if !p.IsValid() { - log.Info("Invalid LFS OID[%s] attempt to POST in %s/%s", p.Oid, rc.User, rc.Repo) - writeStatus(ctx, http.StatusNotFound) - return - } - - if setting.LFS.MaxFileSize > 0 && p.Size > setting.LFS.MaxFileSize { - log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", p.Oid, p.Size, rc.User, rc.Repo, setting.LFS.MaxFileSize) - writeStatus(ctx, http.StatusRequestEntityTooLarge) - return - } - - meta, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) - if err != nil { - log.Error("Unable to write LFS OID[%s] size %d meta object in %v/%v to database. Error: %v", p.Oid, p.Size, rc.User, rc.Repo, err) - writeStatus(ctx, http.StatusNotFound) - return - } - - ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) - - sentStatus := http.StatusAccepted - contentStore := lfs_module.NewContentStore() - exist, err := contentStore.Exists(p) - if err != nil { - log.Error("Unable to check if LFS OID[%s] exist on %s / %s. Error: %v", p.Oid, rc.User, rc.Repo, err) - writeStatus(ctx, http.StatusInternalServerError) - return - } - if meta.Existing && exist { - sentStatus = http.StatusOK - } - ctx.Resp.WriteHeader(sentStatus) - - json := jsoniter.ConfigCompatibleWithStandardLibrary - enc := json.NewEncoder(ctx.Resp) - if err := enc.Encode(buildObjectResponse(rc, meta.Pointer, meta.Existing, true, 0)); err != nil { - log.Error("Failed to encode representation as json. Error: %v", err) - } - logRequest(ctx.Req, sentStatus) -} - // BatchHandler provides the batch api func BatchHandler(ctx *context.Context) { bv := unpackbatch(ctx) From afd6eba94d82498eaab176006a766c00fb64e28f Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 16:06:08 +0000 Subject: [PATCH 13/30] Unified methods. --- routers/routes/web.go | 2 +- services/lfs/server.go | 145 +++++++++++++++-------------------------- 2 files changed, 54 insertions(+), 93 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index b98eb0167e866..8192b89d3c7a4 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -1105,7 +1105,7 @@ func RegisterRoutes(m *web.Route) { m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) m.Get("/objects/{oid}", lfs.DownloadHandler) m.Put("/objects/{oid}", lfs.UploadHandler) - m.Post("/verify/{oid}", lfs.CheckAcceptMediaType, lfs.VerifyHandler) + m.Post("/verify", lfs.CheckAcceptMediaType, lfs.VerifyHandler) m.Group("/locks", func() { m.Get("/", lfs.GetListLockHandler) m.Post("/", lfs.PostLockHandler) diff --git a/services/lfs/server.go b/services/lfs/server.go index bbc1ecd4f9760..851439d5454f7 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -45,13 +45,13 @@ func (rc *requestContext) ObjectLink(oid string) string { } // VerifyLink builds a URL for verifying the object. -func (rc *requestContext) VerifyLink(oid string) string { - return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify", oid) +func (rc *requestContext) VerifyLink() string { + return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify") } // CheckAcceptMediaType checks if the client accepts the LFS media type. func CheckAcceptMediaType(ctx *context.Context) { - mediaParts := strings.Split(r.Header.Get("Accept"), ";") + mediaParts := strings.Split(ctx.Req.Header.Get("Accept"), ";") if mediaParts[0] != lfs_module.MediaType { log.Info("Calling a LFS method without accepting the correct media type: %s", lfs_module.MediaType) @@ -95,7 +95,6 @@ func DownloadHandler(ctx *context.Context) { meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, false) if meta == nil { - // Status already written in getAuthenticatedRepoAndMeta return } @@ -130,7 +129,6 @@ func DownloadHandler(ctx *context.Context) { contentStore := lfs_module.NewContentStore() content, err := contentStore.Get(meta.Pointer) if err != nil { - // Errors are logged in contentStore.Get writeStatus(ctx, http.StatusNotFound) return } @@ -163,34 +161,38 @@ func DownloadHandler(ctx *context.Context) { if written, err := io.CopyN(ctx.Resp, content, contentLength); err != nil { log.Error("Error whilst copying LFS OID[%s] to the response after %d bytes. Error: %v", meta.Oid, written, err) } - logRequest(ctx.Req, statusCode) } // BatchHandler provides the batch api func BatchHandler(ctx *context.Context) { - bv := unpackbatch(ctx) + var br lfs_module.BatchRequest + if err := decodeJSON(ctx.Req, &br); err != nil { + log.Trace("Unable to decode BATCH request vars: Error: %v", err) + writeStatus(ctx, http.StatusBadRequest) + return + } var isUpload bool - if bv.Operation == "upload" { + if br.Operation == "upload" { isUpload = true - } else if bv.Operation == "download" { + } else if br.Operation == "download" { isUpload = false } else { - log.Info("Attempt to BATCH with invalid operation: %s", bv.Operation) + log.Trace("Attempt to BATCH with invalid operation: %s", br.Operation) writeStatus(ctx, http.StatusBadRequest) return } - reqCtx := getRequestContext(ctx) + rc := getRequestContext(ctx) - repository, err := models.GetRepositoryByOwnerAndName(reqCtx.User, reqCtx.Repo) + repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) if err != nil { - log.Error("Unable to get repository: %s/%s Error: %v", reqCtx.User, reqCtx.Repo, err) + log.Trace("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) writeStatus(ctx, http.StatusNotFound) return } - if !authenticate(ctx, repository, reqCtx.Authorization, isUpload) { + if !authenticate(ctx, repository, rc.Authorization, isUpload) { requireAuth(ctx) return } @@ -199,30 +201,30 @@ func BatchHandler(ctx *context.Context) { var responseObjects []*lfs_module.ObjectResponse - for _, p := range bv.Objects { - if !object.IsValid() { - responseObjects = append(responseObjects, buildObjectResponse(reqCtx, p, false, false, http.StatusUnprocessableEntity)) + for _, p := range br.Objects { + if !p.IsValid() { + responseObjects = append(responseObjects, buildObjectResponse(rc, p, false, false, http.StatusUnprocessableEntity)) continue } exists, err := contentStore.Exists(p) if err != nil { - log.Error("Unable to check if LFS OID[%s] exist on %s/%s. Error: %v", p.Oid, reqCtx.User, reqCtx.Repo, err) + log.Error("Unable to check if LFS OID[%s] exist on %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) writeStatus(ctx, http.StatusInternalServerError) return } meta, metaErr := repository.GetLFSMetaObjectByOid(p.Oid) if metaErr != nil && metaErr != models.ErrLFSObjectNotExist { - log.Error("Unable to get LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, reqCtx.User, reqCtx.Repo, metaErr) + log.Error("Unable to get LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, metaErr) writeStatus(ctx, http.StatusInternalServerError) return } var responseObject *lfs_module.ObjectResponse if isUpload { - if !exists && setting.LFS.MaxFileSize > 0 && object.Size > setting.LFS.MaxFileSize { - log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", p.Oid, p.Size, reqCtx.User, reqCtx.Repo, setting.LFS.MaxFileSize) + if !exists && setting.LFS.MaxFileSize > 0 && p.Size > setting.LFS.MaxFileSize { + log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", p.Oid, p.Size, rc.User, rc.Repo, setting.LFS.MaxFileSize) writeStatus(ctx, http.StatusRequestEntityTooLarge) return } @@ -231,14 +233,14 @@ func BatchHandler(ctx *context.Context) { if meta == nil { _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) if err != nil { - log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, reqCtx.User, reqCtx.Repo, metaErr) + log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, metaErr) writeStatus(ctx, http.StatusInternalServerError) return } } } - responseObject = buildObjectResponse(reqCtx, p, false, !exists, 0) + responseObject = buildObjectResponse(rc, p, false, !exists, 0) } else { errorCode := 0 if !exists || meta == nil { @@ -247,7 +249,7 @@ func BatchHandler(ctx *context.Context) { errorCode = http.StatusUnprocessableEntity } - responseObject = buildObjectResponse(reqCtx, p, true, false, errorCode) + responseObject = buildObjectResponse(rc, p, true, false, errorCode) } responseObjects = append(responseObjects, responseObject) } @@ -261,7 +263,6 @@ func BatchHandler(ctx *context.Context) { if err := enc.Encode(respobj); err != nil { log.Error("Failed to encode representation as json. Error: %v", err) } - logRequest(ctx.Req, http.StatusOK) } // UploadHandler receives data from the client and puts it into the content store @@ -270,32 +271,33 @@ func UploadHandler(ctx *context.Context) { meta, repository := getAuthenticatedRepoAndMeta(ctx, rc, p, true) if meta == nil { - // Status already written in getAuthenticatedRepoAndMeta return } contentStore := lfs_module.NewContentStore() defer ctx.Req.Body.Close() if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil { - // Put will log the error itself - ctx.Resp.WriteHeader(http.StatusInternalServerError) if err == lfs_module.ErrSizeMismatch || err == lfs_module.ErrHashMismatch { - fmt.Fprintf(ctx.Resp, `{"message":"%s"}`, err) + writeStatusMessage(ctx, http.StatusInternalServerError, err) } else { - fmt.Fprintf(ctx.Resp, `{"message":"Internal Server Error"}`) + writeStatus(ctx, http.StatusInternalServerError) } if _, err = repository.RemoveLFSMetaObjectByOid(p.Oid); err != nil { - log.Error("Whilst removing metaobject for LFS OID[%s] due to preceding error there was another Error: %v", p.Oid, err) + log.Error("Error whilst removing metaobject for LFS OID[%s]: %v", p.Oid, err) } return } - - logRequest(ctx.Req, http.StatusOK) } // VerifyHandler verify oid and its size from the content store func VerifyHandler(ctx *context.Context) { - rc, p := unpack(ctx) + var p lfs_module.Pointer + if err := decodeJSON(ctx.Req, &p); err != nil { + writeStatus(ctx, http.StatusUnprocessableEntity) + return + } + + rc := getRequestContext(ctx) meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, true) if meta == nil { @@ -304,20 +306,25 @@ func VerifyHandler(ctx *context.Context) { contentStore := lfs_module.NewContentStore() ok, err := contentStore.Verify(meta.Pointer) - if err != nil { - // Error will be logged in Verify - ctx.Resp.WriteHeader(http.StatusInternalServerError) - fmt.Fprintf(ctx.Resp, `{"message":"Internal Server Error"}`) - return - } status := http.StatusOK - if !ok { + if err != nil { + status = http.StatusInternalServerError + } else if !ok { status = http.StatusUnprocessableEntity } writeStatus(ctx, status) } +func decodeJSON(req *http.Request, v interface{}) error { + json := jsoniter.ConfigCompatibleWithStandardLibrary + + defer req.Body.Close() + + dec := json.NewDecoder(req.Body) + return dec.Decode(v) +} + func getRequestContext(ctx *context.Context) *requestContext { return &requestContext{ User: ctx.Params("username"), @@ -349,7 +356,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa } if upload { rep.Actions["upload"] = &lfs_module.Link{Href: rc.ObjectLink(pointer.Oid), Header: header} - rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(pointer.Oid), Header: verifyHeader} + rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(), Header: verifyHeader} } } return rep @@ -359,62 +366,16 @@ func unpack(ctx *context.Context) (*requestContext, lfs_module.Pointer) { rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.Params("oid")} - if ctx.Req.Method == "POST" { // Maybe also check if +json - var p2 lfs_module.Pointer - bodyReader := ctx.Req.Body - defer bodyReader.Close() - json := jsoniter.ConfigCompatibleWithStandardLibrary - dec := json.NewDecoder(bodyReader) - err := dec.Decode(&p2) - if err != nil { - // The error is logged as a WARN here because this may represent misbehaviour rather than a true error - log.Warn("Unable to decode POST request vars for LFS OID[%s] in %s/%s: Error: %v", p.Oid, rc.User, rc.Repo, err) - return rc, p - } - - p.Oid = p2.Oid - p.Size = p2.Size - } - return rc, p } -// TODO cheap hack, unify with unpack -func unpackbatch(ctx *context.Context) *lfs_module.BatchRequest { - - r := ctx.Req - var bv lfs_module.BatchRequest - - bodyReader := r.Body - defer bodyReader.Close() - json := jsoniter.ConfigCompatibleWithStandardLibrary - dec := json.NewDecoder(bodyReader) - err := dec.Decode(&bv) - if err != nil { - // The error is logged as a WARN here because this may represent misbehaviour rather than a true error - log.Warn("Unable to decode BATCH request vars in %s/%s: Error: %v", ctx.Params("username"), strings.TrimSuffix(ctx.Params("reponame"), ".git"), err) - return &bv - } - - return &bv -} - func writeStatus(ctx *context.Context, status int) { - message := http.StatusText(status) - - mediaParts := strings.Split(ctx.Req.Header.Get("Accept"), ";") - mt := mediaParts[0] - if strings.HasSuffix(mt, "+json") { - message = `{"message":"` + message + `"}` - } - - ctx.Resp.WriteHeader(status) - fmt.Fprint(ctx.Resp, message) - logRequest(ctx.Req, status) + writeStatusMessage(ctx, status, http.StatusText(status)) } -func logRequest(r *http.Request, status int) { - log.Debug("LFS request - Method: %s, URL: %s, Status %d", r.Method, r.URL, status) +func writeStatusMessage(ctx *context.Context, status int, message interface{}) { + ctx.Resp.WriteHeader(status) + fmt.Fprintf(ctx.Resp, `{"message":"%v"}`, message) } // authenticate uses the authorization string to determine whether From aff8fc89f941237bd8a49c75ead01736ecbb0bb1 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 16:56:32 +0000 Subject: [PATCH 14/30] Display better error messages. --- services/lfs/server.go | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 851439d5454f7..e0ddbb6c14318 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -203,7 +203,10 @@ func BatchHandler(ctx *context.Context) { for _, p := range br.Objects { if !p.IsValid() { - responseObjects = append(responseObjects, buildObjectResponse(rc, p, false, false, http.StatusUnprocessableEntity)) + responseObjects = append(responseObjects, buildObjectResponse(rc, p, false, false, &lfs_module.ObjectError{ + Code: http.StatusUnprocessableEntity, + Message: "Oid or size are invalid", + })) continue } @@ -221,6 +224,14 @@ func BatchHandler(ctx *context.Context) { return } + if meta != nil && p.Size != meta.Size { + responseObjects = append(responseObjects, buildObjectResponse(rc, p, false, false, &lfs_module.ObjectError{ + Code: http.StatusUnprocessableEntity, + Message: fmt.Sprintf("Object %s is not %d bytes", p.Oid, p.Size), + })) + continue + } + var responseObject *lfs_module.ObjectResponse if isUpload { if !exists && setting.LFS.MaxFileSize > 0 && p.Size > setting.LFS.MaxFileSize { @@ -240,16 +251,17 @@ func BatchHandler(ctx *context.Context) { } } - responseObject = buildObjectResponse(rc, p, false, !exists, 0) + responseObject = buildObjectResponse(rc, p, false, !exists, nil) } else { - errorCode := 0 + var err *lfs_module.ObjectError if !exists || meta == nil { - errorCode = http.StatusNotFound - } else if meta.Size != p.Size { - errorCode = http.StatusUnprocessableEntity + err = &lfs_module.ObjectError{ + Code: http.StatusNotFound, + Message: http.StatusText(http.StatusNotFound), + } } - responseObject = buildObjectResponse(rc, p, true, false, errorCode) + responseObject = buildObjectResponse(rc, p, true, false, err) } responseObjects = append(responseObjects, responseObject) } @@ -333,13 +345,10 @@ func getRequestContext(ctx *context.Context) *requestContext { } } -func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, download, upload bool, errorCode int) *lfs_module.ObjectResponse { +func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, download, upload bool, err *lfs_module.ObjectError) *lfs_module.ObjectResponse { rep := &lfs_module.ObjectResponse{Pointer: pointer} - if errorCode > 0 { - rep.Error = &lfs_module.ObjectError{ - Code: errorCode, - Message: http.StatusText(errorCode), - } + if err != nil { + rep.Error = err } else { rep.Actions = make(map[string]*lfs_module.Link) From 24279e11a6bb7c51896b82b4ff75679187b7a7dc Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 17:00:14 +0000 Subject: [PATCH 15/30] Added size parameter. Create meta object on upload. --- routers/routes/web.go | 2 +- services/lfs/server.go | 73 ++++++++++++++++++++++++++++++------------ 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 8192b89d3c7a4..9e05df0a21b0a 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -1102,9 +1102,9 @@ func RegisterRoutes(m *web.Route) { m.Group("/{reponame}", func() { m.Group("/info/lfs", func() { m.Post("/objects/batch", lfs.CheckAcceptMediaType, lfs.BatchHandler) + m.Put("/objects/{oid}/{size}", lfs.UploadHandler) m.Get("/objects/{oid}/{filename}", lfs.DownloadHandler) m.Get("/objects/{oid}", lfs.DownloadHandler) - m.Put("/objects/{oid}", lfs.UploadHandler) m.Post("/verify", lfs.CheckAcceptMediaType, lfs.VerifyHandler) m.Group("/locks", func() { m.Get("/", lfs.GetListLockHandler) diff --git a/services/lfs/server.go b/services/lfs/server.go index e0ddbb6c14318..58f2e7c4e0eb1 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -39,13 +39,18 @@ type Claims struct { jwt.StandardClaims } -// ObjectLink builds a URL linking to the object. -func (rc *requestContext) ObjectLink(oid string) string { - return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/objects", oid) +// DownloadLink builds a URL to download the object. +func (rc *requestContext) DownloadLink(p lfs_module.Pointer) string { + return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/objects", p.Oid) +} + +// UploadLink builds a URL to upload the object. +func (rc *requestContext) UploadLink(p lfs_module.Pointer) string { + return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/objects", p.Oid, strconv.FormatInt(p.Size, 10)) } // VerifyLink builds a URL for verifying the object. -func (rc *requestContext) VerifyLink() string { +func (rc *requestContext) VerifyLink(p lfs_module.Pointer) string { return setting.AppURL + path.Join(rc.User, rc.Repo+".git", "info/lfs/verify") } @@ -63,19 +68,12 @@ func CheckAcceptMediaType(ctx *context.Context) { func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) (*models.LFSMetaObject, *models.Repository) { if !p.IsValid() { log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) - writeStatus(ctx, http.StatusNotFound) - return nil, nil - } - - repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) - if err != nil { - log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) - writeStatus(ctx, http.StatusNotFound) + writeStatus(ctx, http.StatusUnprocessableEntity) return nil, nil } - if !authenticate(ctx, repository, rc.Authorization, requireWrite) { - requireAuth(ctx) + repository := getAuthenticatedRepository(ctx, rc, requireWrite) + if repository == nil { return nil, nil } @@ -89,6 +87,22 @@ func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs return meta, repository } +func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requireWrite bool) *models.Repository { + repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) + if err != nil { + log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) + writeStatus(ctx, http.StatusNotFound) + return nil + } + + if !authenticate(ctx, repository, rc.Authorization, requireWrite) { + requireAuth(ctx) + return nil + } + + return repository +} + // DownloadHandler gets the content from the content store func DownloadHandler(ctx *context.Context) { rc, p := unpack(ctx) @@ -279,10 +293,29 @@ func BatchHandler(ctx *context.Context) { // UploadHandler receives data from the client and puts it into the content store func UploadHandler(ctx *context.Context) { - rc, p := unpack(ctx) + rc := getRequestContext(ctx) - meta, repository := getAuthenticatedRepoAndMeta(ctx, rc, p, true) - if meta == nil { + p := lfs_module.Pointer{Oid: ctx.Params("oid")} + var err error + if p.Size, err = strconv.ParseInt(ctx.Params("size"), 10, 64); err != nil { + writeStatusMessage(ctx, http.StatusUnprocessableEntity, err) + } + + if !p.IsValid() { + log.Trace("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) + writeStatus(ctx, http.StatusUnprocessableEntity) + return + } + + repository := getAuthenticatedRepository(ctx, rc, true) + if repository == nil { + return + } + + meta, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) + if err != nil { + log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) + writeStatus(ctx, http.StatusInternalServerError) return } @@ -361,11 +394,11 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa } if download { - rep.Actions["download"] = &lfs_module.Link{Href: rc.ObjectLink(pointer.Oid), Header: header} + rep.Actions["download"] = &lfs_module.Link{Href: rc.DownloadLink(pointer), Header: header} } if upload { - rep.Actions["upload"] = &lfs_module.Link{Href: rc.ObjectLink(pointer.Oid), Header: header} - rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(), Header: verifyHeader} + rep.Actions["upload"] = &lfs_module.Link{Href: rc.UploadLink(pointer), Header: header} + rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(pointer), Header: verifyHeader} } } return rep From c45f43c21f131624168a55a6937d797d2970cd09 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 17:08:44 +0000 Subject: [PATCH 16/30] Use object error on invalid size. --- services/lfs/server.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 58f2e7c4e0eb1..8396418fc5ae6 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -199,15 +199,8 @@ func BatchHandler(ctx *context.Context) { rc := getRequestContext(ctx) - repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) - if err != nil { - log.Trace("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) - writeStatus(ctx, http.StatusNotFound) - return - } - - if !authenticate(ctx, repository, rc.Authorization, isUpload) { - requireAuth(ctx) + repository := getAuthenticatedRepository(ctx, rc, isUpload) + if repository == nil { return } @@ -248,10 +241,12 @@ func BatchHandler(ctx *context.Context) { var responseObject *lfs_module.ObjectResponse if isUpload { + var err *lfs_module.ObjectError if !exists && setting.LFS.MaxFileSize > 0 && p.Size > setting.LFS.MaxFileSize { - log.Info("Denied LFS OID[%s] upload of size %d to %s/%s because of LFS_MAX_FILE_SIZE=%d", p.Oid, p.Size, rc.User, rc.Repo, setting.LFS.MaxFileSize) - writeStatus(ctx, http.StatusRequestEntityTooLarge) - return + err = &lfs_module.ObjectError{ + Code: http.StatusUnprocessableEntity, + Message: fmt.Sprintf("Size must be less than or equal to %d", setting.LFS.MaxFileSize), + } } if exists { @@ -265,7 +260,7 @@ func BatchHandler(ctx *context.Context) { } } - responseObject = buildObjectResponse(rc, p, false, !exists, nil) + responseObject = buildObjectResponse(rc, p, false, !exists, err) } else { var err *lfs_module.ObjectError if !exists || meta == nil { From 709fcfa005316e994500b663caaad0b52c1fa3ce Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 17:13:11 +0000 Subject: [PATCH 17/30] Skip upload if object exists. --- services/lfs/server.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 8396418fc5ae6..8bd5e390b303d 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -219,14 +219,14 @@ func BatchHandler(ctx *context.Context) { exists, err := contentStore.Exists(p) if err != nil { - log.Error("Unable to check if LFS OID[%s] exist on %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) + log.Error("Unable to check if LFS OID[%s] exist. Error: %v", p.Oid, rc.User, rc.Repo, err) writeStatus(ctx, http.StatusInternalServerError) return } - meta, metaErr := repository.GetLFSMetaObjectByOid(p.Oid) - if metaErr != nil && metaErr != models.ErrLFSObjectNotExist { - log.Error("Unable to get LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, metaErr) + meta, err := repository.GetLFSMetaObjectByOid(p.Oid) + if err != nil && err != models.ErrLFSObjectNotExist { + log.Error("Unable to get LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) writeStatus(ctx, http.StatusInternalServerError) return } @@ -315,6 +315,18 @@ func UploadHandler(ctx *context.Context) { } contentStore := lfs_module.NewContentStore() + + exists, err := contentStore.Exists(p) + if err != nil { + log.Error("Unable to check if LFS OID[%s] exist. Error: %v", p.Oid, err) + writeStatus(ctx, http.StatusInternalServerError) + return + } + if m.Existing || exisits { + ctx.Resp.WriteHeader(http.StatusOK) + return + } + defer ctx.Req.Body.Close() if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil { if err == lfs_module.ErrSizeMismatch || err == lfs_module.ErrHashMismatch { From d11bd555f4eaf585b842f9333cf22da7d434b152 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 17:26:22 +0000 Subject: [PATCH 18/30] Moved methods. --- services/lfs/locks.go | 2 +- services/lfs/server.go | 94 ++++++++++++++++++++---------------------- 2 files changed, 45 insertions(+), 51 deletions(-) diff --git a/services/lfs/locks.go b/services/lfs/locks.go index e4a06328c1c08..2ae79f2dc62fa 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -65,7 +65,7 @@ func GetListLockHandler(ctx *context.Context) { } ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) - rv, _ := unpack(ctx) + rv := getRequestContext(ctx) repository, err := models.GetRepositoryByOwnerAndName(rv.User, rv.Repo) if err != nil { diff --git a/services/lfs/server.go b/services/lfs/server.go index 8bd5e390b303d..ae74df4567e12 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -65,49 +65,12 @@ func CheckAcceptMediaType(ctx *context.Context) { } } -func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) (*models.LFSMetaObject, *models.Repository) { - if !p.IsValid() { - log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) - writeStatus(ctx, http.StatusUnprocessableEntity) - return nil, nil - } - - repository := getAuthenticatedRepository(ctx, rc, requireWrite) - if repository == nil { - return nil, nil - } - - meta, err := repository.GetLFSMetaObjectByOid(p.Oid) - if err != nil { - log.Error("Unable to get LFS OID[%s] Error: %v", p.Oid, err) - writeStatus(ctx, http.StatusNotFound) - return nil, nil - } - - return meta, repository -} - -func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requireWrite bool) *models.Repository { - repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) - if err != nil { - log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) - writeStatus(ctx, http.StatusNotFound) - return nil - } - - if !authenticate(ctx, repository, rc.Authorization, requireWrite) { - requireAuth(ctx) - return nil - } - - return repository -} - // DownloadHandler gets the content from the content store func DownloadHandler(ctx *context.Context) { - rc, p := unpack(ctx) + rc := getRequestContext(ctx) + p := lfs_module.Pointer{Oid: ctx.Params("oid")} - meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, false) + meta := getAuthenticatedMeta(ctx, rc, p, false) if meta == nil { return } @@ -253,7 +216,7 @@ func BatchHandler(ctx *context.Context) { if meta == nil { _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) if err != nil { - log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, metaErr) + log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) writeStatus(ctx, http.StatusInternalServerError) return } @@ -322,7 +285,7 @@ func UploadHandler(ctx *context.Context) { writeStatus(ctx, http.StatusInternalServerError) return } - if m.Existing || exisits { + if meta.Existing || exists { ctx.Resp.WriteHeader(http.StatusOK) return } @@ -351,7 +314,7 @@ func VerifyHandler(ctx *context.Context) { rc := getRequestContext(ctx) - meta, _ := getAuthenticatedRepoAndMeta(ctx, rc, p, true) + meta := getAuthenticatedMeta(ctx, rc, p, true) if meta == nil { return } @@ -385,6 +348,44 @@ func getRequestContext(ctx *context.Context) *requestContext { } } +func getAuthenticatedMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) *models.LFSMetaObject { + if !p.IsValid() { + log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) + writeStatus(ctx, http.StatusUnprocessableEntity) + return nil + } + + repository := getAuthenticatedRepository(ctx, rc, requireWrite) + if repository == nil { + return nil + } + + meta, err := repository.GetLFSMetaObjectByOid(p.Oid) + if err != nil { + log.Error("Unable to get LFS OID[%s] Error: %v", p.Oid, err) + writeStatus(ctx, http.StatusNotFound) + return nil + } + + return meta +} + +func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requireWrite bool) *models.Repository { + repository, err := models.GetRepositoryByOwnerAndName(rc.User, rc.Repo) + if err != nil { + log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) + writeStatus(ctx, http.StatusNotFound) + return nil + } + + if !authenticate(ctx, repository, rc.Authorization, requireWrite) { + requireAuth(ctx) + return nil + } + + return repository +} + func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, download, upload bool, err *lfs_module.ObjectError) *lfs_module.ObjectResponse { rep := &lfs_module.ObjectResponse{Pointer: pointer} if err != nil { @@ -411,13 +412,6 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa return rep } -func unpack(ctx *context.Context) (*requestContext, lfs_module.Pointer) { - rc := getRequestContext(ctx) - p := lfs_module.Pointer{Oid: ctx.Params("oid")} - - return rc, p -} - func writeStatus(ctx *context.Context, status int) { writeStatusMessage(ctx, status, http.StatusText(status)) } From 497283f9b6c688f95db53b71cc23e733491fec67 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 17:39:01 +0000 Subject: [PATCH 19/30] Suppress fields in response. --- modules/lfs/shared.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/lfs/shared.go b/modules/lfs/shared.go index 70b76d7512d56..37add77444713 100644 --- a/modules/lfs/shared.go +++ b/modules/lfs/shared.go @@ -45,7 +45,7 @@ type BatchResponse struct { // ObjectResponse is object metadata as seen by clients of the LFS server. type ObjectResponse struct { Pointer - Actions map[string]*Link `json:"actions"` + Actions map[string]*Link `json:"actions,omitempty"` Error *ObjectError `json:"error,omitempty"` } @@ -53,7 +53,7 @@ type ObjectResponse struct { type Link struct { Href string `json:"href"` Header map[string]string `json:"header,omitempty"` - ExpiresAt time.Time `json:"expires_at,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty"` } // ObjectError defines the JSON structure returned to the client in case of an error From e0db2b278392d2ab3ea671de4f78c4a6353cab83 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 17:46:28 +0000 Subject: [PATCH 20/30] Changed error on accept. --- services/lfs/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index ae74df4567e12..fbe07b8854489 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -59,8 +59,8 @@ func CheckAcceptMediaType(ctx *context.Context) { mediaParts := strings.Split(ctx.Req.Header.Get("Accept"), ";") if mediaParts[0] != lfs_module.MediaType { - log.Info("Calling a LFS method without accepting the correct media type: %s", lfs_module.MediaType) - writeStatus(ctx, http.StatusNotAcceptable) + log.Trace("Calling a LFS method without accepting the correct media type: %s", lfs_module.MediaType) + writeStatus(ctx, http.StatusUnsupportedMediaType) return } } From 5a932daf2e797d5bfb6245d2c3f8e48c82f64c01 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 18:20:58 +0000 Subject: [PATCH 21/30] Fixed tests. --- integrations/api_repo_lfs_locks_test.go | 8 ++++---- integrations/lfs_getobject_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 69981d1c42000..15681ed16fdef 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -24,13 +24,13 @@ func TestAPILFSLocksNotStarted(t *testing.T) { repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) req := NewRequestf(t, "GET", "/%s/%s.git/info/lfs/locks", user.Name, repo.Name) - MakeRequest(t, req, http.StatusNotFound) + MakeRequest(t, req, http.StatusForbidden) req = NewRequestf(t, "POST", "/%s/%s.git/info/lfs/locks", user.Name, repo.Name) - MakeRequest(t, req, http.StatusNotFound) + MakeRequest(t, req, http.StatusForbidden) req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/locks/verify", user.Name, repo.Name) - MakeRequest(t, req, http.StatusNotFound) + MakeRequest(t, req, http.StatusForbidden) req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/locks/10/unlock", user.Name, repo.Name) - MakeRequest(t, req, http.StatusNotFound) + MakeRequest(t, req, http.StatusForbidden) } func TestAPILFSLocksNotLogin(t *testing.T) { diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go index 789c7572a77e5..3005a1a623d4f 100644 --- a/integrations/lfs_getobject_test.go +++ b/integrations/lfs_getobject_test.go @@ -198,7 +198,7 @@ func TestGetLFSRange(t *testing.T) { {"bytes=0-10", "123456789\n", http.StatusPartialContent}, // end-range bigger than length-1 is ignored {"bytes=0-11", "123456789\n", http.StatusPartialContent}, - {"bytes=11-", "Requested Range Not Satisfiable", http.StatusRequestedRangeNotSatisfiable}, + {"bytes=11-", "{\"message\":\"Requested Range Not Satisfiable\"}", http.StatusRequestedRangeNotSatisfiable}, // incorrect header value cause whole header to be ignored {"bytes=-", "123456789\n", http.StatusOK}, {"foobar", "123456789\n", http.StatusOK}, From 2b0a713a4c1fba953a3125074446c2177aad1837 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 16 Apr 2021 22:13:45 +0000 Subject: [PATCH 22/30] Added tests. --- integrations/api_repo_lfs_locks_test.go | 19 +- integrations/api_repo_lfs_test.go | 456 ++++++++++++++++++++++++ services/lfs/server.go | 8 +- 3 files changed, 471 insertions(+), 12 deletions(-) create mode 100644 integrations/api_repo_lfs_test.go diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 15681ed16fdef..682b7929757d9 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -11,6 +11,7 @@ import ( "time" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -40,7 +41,7 @@ func TestAPILFSLocksNotLogin(t *testing.T) { repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) req := NewRequestf(t, "GET", "/%s/%s.git/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Accept", lfs.MediaType) resp := MakeRequest(t, req, http.StatusUnauthorized) var lfsLockError api.LFSLockError DecodeJSON(t, resp, &lfsLockError) @@ -102,8 +103,8 @@ func TestAPILFSLocksLogged(t *testing.T) { for _, test := range tests { session := loginUser(t, test.user.Name) req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks", test.repo.FullName()), map[string]string{"path": test.path}) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Header.Set("Accept", lfs.MediaType) + req.Header.Set("Content-Type", lfs.MediaType) resp := session.MakeRequest(t, req, test.httpResult) if len(test.addTime) > 0 { var lfsLock api.LFSLockResponse @@ -119,7 +120,7 @@ func TestAPILFSLocksLogged(t *testing.T) { for _, test := range resultsTests { session := loginUser(t, test.user.Name) req := NewRequestf(t, "GET", "/%s.git/info/lfs/locks", test.repo.FullName()) - req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Accept", lfs.MediaType) resp := session.MakeRequest(t, req, http.StatusOK) var lfsLocks api.LFSLockList DecodeJSON(t, resp, &lfsLocks) @@ -131,8 +132,8 @@ func TestAPILFSLocksLogged(t *testing.T) { } req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks/verify", test.repo.FullName()), map[string]string{}) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Header.Set("Accept", lfs.MediaType) + req.Header.Set("Content-Type", lfs.MediaType) resp = session.MakeRequest(t, req, http.StatusOK) var lfsLocksVerify api.LFSLockListVerify DecodeJSON(t, resp, &lfsLocksVerify) @@ -155,8 +156,8 @@ func TestAPILFSLocksLogged(t *testing.T) { for _, test := range deleteTests { session := loginUser(t, test.user.Name) req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks/%s/unlock", test.repo.FullName(), test.lockID), map[string]string{}) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Header.Set("Accept", lfs.MediaType) + req.Header.Set("Content-Type", lfs.MediaType) resp := session.MakeRequest(t, req, http.StatusOK) var lfsLockRep api.LFSLockResponse DecodeJSON(t, resp, &lfsLockRep) @@ -168,7 +169,7 @@ func TestAPILFSLocksLogged(t *testing.T) { for _, test := range resultsTests { session := loginUser(t, test.user.Name) req := NewRequestf(t, "GET", "/%s.git/info/lfs/locks", test.repo.FullName()) - req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Accept", lfs.MediaType) resp := session.MakeRequest(t, req, http.StatusOK) var lfsLocks api.LFSLockList DecodeJSON(t, resp, &lfsLocks) diff --git a/integrations/api_repo_lfs_test.go b/integrations/api_repo_lfs_test.go new file mode 100644 index 0000000000000..d40e5c7caf325 --- /dev/null +++ b/integrations/api_repo_lfs_test.go @@ -0,0 +1,456 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "bytes" + "net/http" + "path" + "strconv" + "strings" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/modules/setting" + + jsoniter "github.com/json-iterator/go" + "github.com/stretchr/testify/assert" +) + +func TestAPILFSNotStarted(t *testing.T) { + defer prepareTestEnv(t)() + + setting.LFS.StartServer = false + + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + + req := NewRequestf(t, "POST", "/%s/%s.git/info/lfs/objects/batch", user.Name, repo.Name) + MakeRequest(t, req, http.StatusForbidden) + req = NewRequestf(t, "PUT", "/%s/%s.git/info/lfs/objects/oid/10", user.Name, repo.Name) + MakeRequest(t, req, http.StatusForbidden) + req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/objects/oid/name", user.Name, repo.Name) + MakeRequest(t, req, http.StatusForbidden) + req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/objects/oid", user.Name, repo.Name) + MakeRequest(t, req, http.StatusForbidden) + req = NewRequestf(t, "POST", "/%s/%s.git/info/lfs/verify", user.Name, repo.Name) + MakeRequest(t, req, http.StatusForbidden) +} + +func TestAPILFSMediaType(t *testing.T) { + defer prepareTestEnv(t)() + + setting.LFS.StartServer = true + + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + + req := NewRequestf(t, "POST", "/%s/%s.git/info/lfs/objects/batch", user.Name, repo.Name) + MakeRequest(t, req, http.StatusUnsupportedMediaType) + req = NewRequestf(t, "POST", "/%s/%s.git/info/lfs/verify", user.Name, repo.Name) + MakeRequest(t, req, http.StatusUnsupportedMediaType) +} + +func TestAPILFSBatch(t *testing.T) { + defer prepareTestEnv(t)() + + setting.LFS.StartServer = true + + repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1") + assert.NoError(t, err) + content := []byte("dummy1") + oid := storeObjectInRepo(t, repo.ID, &content) + defer repo.RemoveLFSMetaObjectByOid(oid) + + session := loginUser(t, "user2") + + newRequest := func(t testing.TB, br *lfs.BatchRequest) *http.Request { + req := NewRequestWithJSON(t, "POST", "/user2/repo1.git/info/lfs/objects/batch", br) + req.Header.Set("Accept", lfs.MediaType) + req.Header.Set("Content-Type", lfs.MediaType) + return req + } + decodeResponse := func(t *testing.T, b *bytes.Buffer) *lfs.BatchResponse { + var br lfs.BatchResponse + + json := jsoniter.ConfigCompatibleWithStandardLibrary + assert.NoError(t, json.Unmarshal(b.Bytes(), &br)) + return &br + } + + t.Run("InvalidJsonRequest", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, nil) + + session.MakeRequest(t, req, http.StatusBadRequest) + }) + + t.Run("InvalidOperation", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "dummy", + }) + + session.MakeRequest(t, req, http.StatusBadRequest) + }) + + t.Run("InvalidPointer", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "download", + Objects: []lfs.Pointer{ + {Oid: "dummy"}, + {Oid: oid, Size: -1}, + }, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 2) + assert.Equal(t, "dummy", br.Objects[0].Oid) + assert.Equal(t, oid, br.Objects[1].Oid) + assert.Equal(t, int64(0), br.Objects[0].Size) + assert.Equal(t, int64(-1), br.Objects[1].Size) + assert.NotNil(t, br.Objects[0].Error) + assert.NotNil(t, br.Objects[1].Error) + assert.Equal(t, http.StatusUnprocessableEntity, br.Objects[0].Error.Code) + assert.Equal(t, http.StatusUnprocessableEntity, br.Objects[1].Error.Code) + assert.Equal(t, "Oid or size are invalid", br.Objects[0].Error.Message) + assert.Equal(t, "Oid or size are invalid", br.Objects[1].Error.Message) + }) + + t.Run("PointerSizeMissmatch", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "download", + Objects: []lfs.Pointer{ + {Oid: oid, Size: 1}, + }, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.NotNil(t, br.Objects[0].Error) + assert.Equal(t, http.StatusUnprocessableEntity, br.Objects[0].Error.Code) + assert.Equal(t, "Object "+oid+" is not 1 bytes", br.Objects[0].Error.Message) + }) + + t.Run("Download", func(t *testing.T) { + defer PrintCurrentTest(t)() + + t.Run("PointerNotInStore", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "download", + Objects: []lfs.Pointer{ + {Oid: "fb8f7d8435968c4f82a726a92395be4d16f2f63116caf36c8ad35c60831ab042", Size: 6}, + }, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.NotNil(t, br.Objects[0].Error) + assert.Equal(t, http.StatusNotFound, br.Objects[0].Error.Code) + }) + + t.Run("MetaNotFound", func(t *testing.T) { + defer PrintCurrentTest(t)() + + p := lfs.Pointer{Oid: "05eeb4eb5be71f2dd291ca39157d6d9effd7d1ea19cbdc8a99411fe2a8f26a00", Size: 6} + + contentStore := lfs.NewContentStore() + exist, err := contentStore.Exists(p) + assert.NoError(t, err) + assert.False(t, exist) + err = contentStore.Put(p, bytes.NewReader([]byte("dummy0"))) + assert.NoError(t, err) + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "download", + Objects: []lfs.Pointer{p}, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.NotNil(t, br.Objects[0].Error) + assert.Equal(t, http.StatusNotFound, br.Objects[0].Error.Code) + }) + + t.Run("Success", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "download", + Objects: []lfs.Pointer{ + {Oid: oid, Size: 6}, + }, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.Nil(t, br.Objects[0].Error) + assert.Contains(t, br.Objects[0].Actions, "download") + l := br.Objects[0].Actions["download"] + assert.NotNil(t, l) + assert.NotEmpty(t, l.Href) + }) + }) + + t.Run("Upload", func(t *testing.T) { + defer PrintCurrentTest(t)() + + t.Run("FileTooBig", func(t *testing.T) { + defer PrintCurrentTest(t)() + + oldMaxFileSize := setting.LFS.MaxFileSize + setting.LFS.MaxFileSize = 2 + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "upload", + Objects: []lfs.Pointer{ + {Oid: "fb8f7d8435968c4f82a726a92395be4d16f2f63116caf36c8ad35c60831ab042", Size: 6}, + }, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.NotNil(t, br.Objects[0].Error) + assert.Equal(t, http.StatusUnprocessableEntity, br.Objects[0].Error.Code) + assert.Equal(t, "Size must be less than or equal to 2", br.Objects[0].Error.Message) + + setting.LFS.MaxFileSize = oldMaxFileSize + }) + + t.Run("AddMeta", func(t *testing.T) { + defer PrintCurrentTest(t)() + + p := lfs.Pointer{Oid: "05eeb4eb5be71f2dd291ca39157d6d9effd7d1ea19cbdc8a99411fe2a8f26a00", Size: 6} + + contentStore := lfs.NewContentStore() + exist, err := contentStore.Exists(p) + assert.NoError(t, err) + assert.True(t, exist) + + meta, err := repo.GetLFSMetaObjectByOid(p.Oid) + assert.Nil(t, meta) + assert.Equal(t, models.ErrLFSObjectNotExist, err) + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "upload", + Objects: []lfs.Pointer{p}, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.Nil(t, br.Objects[0].Error) + assert.Empty(t, br.Objects[0].Actions) + + meta, err = repo.GetLFSMetaObjectByOid(p.Oid) + assert.NoError(t, err) + assert.NotNil(t, meta) + }) + + t.Run("AlreadyExists", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "upload", + Objects: []lfs.Pointer{ + {Oid: oid, Size: 6}, + }, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.Nil(t, br.Objects[0].Error) + assert.Empty(t, br.Objects[0].Actions) + }) + + t.Run("NewFile", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.BatchRequest{ + Operation: "upload", + Objects: []lfs.Pointer{ + {Oid: "d6f175817f886ec6fbbc1515326465fa96c3bfd54a4ea06cfd6dbbd8340e0153", Size: 1}, + }, + }) + + resp := session.MakeRequest(t, req, http.StatusOK) + br := decodeResponse(t, resp.Body) + assert.Len(t, br.Objects, 1) + assert.Nil(t, br.Objects[0].Error) + assert.Contains(t, br.Objects[0].Actions, "upload") + ul := br.Objects[0].Actions["upload"] + assert.NotNil(t, ul) + assert.NotEmpty(t, ul.Href) + assert.Contains(t, br.Objects[0].Actions, "verify") + vl := br.Objects[0].Actions["verify"] + assert.NotNil(t, vl) + assert.NotEmpty(t, vl.Href) + }) + }) +} + +func TestAPILFSUpload(t *testing.T) { + defer prepareTestEnv(t)() + + setting.LFS.StartServer = true + + repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1") + assert.NoError(t, err) + content := []byte("dummy3") + oid := storeObjectInRepo(t, repo.ID, &content) + defer repo.RemoveLFSMetaObjectByOid(oid) + + session := loginUser(t, "user2") + + newRequest := func(t testing.TB, p lfs.Pointer, content string) *http.Request { + req := NewRequestWithBody(t, "PUT", path.Join("/user2/repo1.git/info/lfs/objects/", p.Oid, strconv.FormatInt(p.Size, 10)), strings.NewReader(content)) + return req + } + + t.Run("InvalidPointer", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, lfs.Pointer{Oid: "dummy"}, "") + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) + + t.Run("AlreadyExistsInStore", func(t *testing.T) { + defer PrintCurrentTest(t)() + + p := lfs.Pointer{Oid: "83de2e488b89a0aa1c97496b888120a28b0c1e15463a4adb8405578c540f36d4", Size: 6} + + contentStore := lfs.NewContentStore() + exist, err := contentStore.Exists(p) + assert.NoError(t, err) + assert.False(t, exist) + err = contentStore.Put(p, bytes.NewReader([]byte("dummy5"))) + assert.NoError(t, err) + + meta, err := repo.GetLFSMetaObjectByOid(p.Oid) + assert.Nil(t, meta) + assert.Equal(t, models.ErrLFSObjectNotExist, err) + + req := newRequest(t, p, "") + + session.MakeRequest(t, req, http.StatusOK) + + meta, err = repo.GetLFSMetaObjectByOid(p.Oid) + assert.NoError(t, err) + assert.NotNil(t, meta) + }) + + t.Run("MetaAlreadyExists", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, lfs.Pointer{Oid: oid, Size: 6}, "") + + session.MakeRequest(t, req, http.StatusOK) + }) + + t.Run("HashMissmatch", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, lfs.Pointer{Oid: "2581dd7bbc1fe44726de4b7dd806a087a978b9c5aec0a60481259e34be09b06a", Size: 1}, "a") + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) + + t.Run("SizeMissmatch", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, lfs.Pointer{Oid: "ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb", Size: 2}, "a") + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) + + t.Run("Success", func(t *testing.T) { + defer PrintCurrentTest(t)() + + p := lfs.Pointer{Oid: "6ccce4863b70f258d691f59609d31b4502e1ba5199942d3bc5d35d17a4ce771d", Size: 5} + + req := newRequest(t, p, "gitea") + + session.MakeRequest(t, req, http.StatusOK) + + contentStore := lfs.NewContentStore() + exist, err := contentStore.Exists(p) + assert.NoError(t, err) + assert.True(t, exist) + + meta, err := repo.GetLFSMetaObjectByOid(p.Oid) + assert.NoError(t, err) + assert.NotNil(t, meta) + }) +} + +func TestAPILFSVerify(t *testing.T) { + defer prepareTestEnv(t)() + + setting.LFS.StartServer = true + + repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1") + assert.NoError(t, err) + content := []byte("dummy3") + oid := storeObjectInRepo(t, repo.ID, &content) + defer repo.RemoveLFSMetaObjectByOid(oid) + + session := loginUser(t, "user2") + + newRequest := func(t testing.TB, p *lfs.Pointer) *http.Request { + req := NewRequestWithJSON(t, "POST", "/user2/repo1.git/info/lfs/verify", p) + req.Header.Set("Accept", lfs.MediaType) + req.Header.Set("Content-Type", lfs.MediaType) + return req + } + + t.Run("InvalidJsonRequest", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, nil) + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) + + t.Run("InvalidPointer", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.Pointer{}) + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) + + t.Run("PointerNotExisting", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.Pointer{Oid: "fb8f7d8435968c4f82a726a92395be4d16f2f63116caf36c8ad35c60831ab042", Size: 6}) + + session.MakeRequest(t, req, http.StatusNotFound) + }) + + t.Run("Success", func(t *testing.T) { + defer PrintCurrentTest(t)() + + req := newRequest(t, &lfs.Pointer{Oid: oid, Size: 6}) + + session.MakeRequest(t, req, http.StatusOK) + }) +} diff --git a/services/lfs/server.go b/services/lfs/server.go index fbe07b8854489..c49f2a4c5ea94 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -293,7 +293,7 @@ func UploadHandler(ctx *context.Context) { defer ctx.Req.Body.Close() if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil { if err == lfs_module.ErrSizeMismatch || err == lfs_module.ErrHashMismatch { - writeStatusMessage(ctx, http.StatusInternalServerError, err) + writeStatusMessage(ctx, http.StatusUnprocessableEntity, err) } else { writeStatus(ctx, http.StatusInternalServerError) } @@ -302,6 +302,8 @@ func UploadHandler(ctx *context.Context) { } return } + + writeStatus(ctx, http.StatusOK) } // VerifyHandler verify oid and its size from the content store @@ -326,7 +328,7 @@ func VerifyHandler(ctx *context.Context) { if err != nil { status = http.StatusInternalServerError } else if !ok { - status = http.StatusUnprocessableEntity + status = http.StatusNotFound } writeStatus(ctx, status) } @@ -351,7 +353,7 @@ func getRequestContext(ctx *context.Context) *requestContext { func getAuthenticatedMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) *models.LFSMetaObject { if !p.IsValid() { log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) - writeStatus(ctx, http.StatusUnprocessableEntity) + writeStatusMessage(ctx, http.StatusUnprocessableEntity, "Oid or size are invalid") return nil } From 9bc1cf6c47905ffa71507162332c7d7e92373ba4 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 18 Apr 2021 17:55:44 +0000 Subject: [PATCH 23/30] Use ErrorResponse object. --- modules/lfs/shared.go | 7 +++++++ services/lfs/server.go | 26 +++++++++++++++----------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/modules/lfs/shared.go b/modules/lfs/shared.go index 37add77444713..9abbf85fbdc78 100644 --- a/modules/lfs/shared.go +++ b/modules/lfs/shared.go @@ -67,3 +67,10 @@ type PointerBlob struct { Hash string Pointer } + +// ErrorResponse describes the error to the client. +type ErrorResponse struct { + Message string + DocumentationURL string `json:"documentation_url,omitempty"` + RequestID string `json:"request_id,omitempty"` +} diff --git a/services/lfs/server.go b/services/lfs/server.go index c49f2a4c5ea94..f39f41acd8038 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -238,12 +238,11 @@ func BatchHandler(ctx *context.Context) { responseObjects = append(responseObjects, responseObject) } - ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) - respobj := &lfs_module.BatchResponse{Objects: responseObjects} - json := jsoniter.ConfigCompatibleWithStandardLibrary - enc := json.NewEncoder(ctx.Resp) + ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) + + enc := jsoniter.NewEncoder(ctx.Resp) if err := enc.Encode(respobj); err != nil { log.Error("Failed to encode representation as json. Error: %v", err) } @@ -256,7 +255,7 @@ func UploadHandler(ctx *context.Context) { p := lfs_module.Pointer{Oid: ctx.Params("oid")} var err error if p.Size, err = strconv.ParseInt(ctx.Params("size"), 10, 64); err != nil { - writeStatusMessage(ctx, http.StatusUnprocessableEntity, err) + writeStatusMessage(ctx, http.StatusUnprocessableEntity, err.Error()) } if !p.IsValid() { @@ -293,7 +292,7 @@ func UploadHandler(ctx *context.Context) { defer ctx.Req.Body.Close() if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil { if err == lfs_module.ErrSizeMismatch || err == lfs_module.ErrHashMismatch { - writeStatusMessage(ctx, http.StatusUnprocessableEntity, err) + writeStatusMessage(ctx, http.StatusUnprocessableEntity, err.Error()) } else { writeStatus(ctx, http.StatusInternalServerError) } @@ -334,11 +333,9 @@ func VerifyHandler(ctx *context.Context) { } func decodeJSON(req *http.Request, v interface{}) error { - json := jsoniter.ConfigCompatibleWithStandardLibrary - defer req.Body.Close() - dec := json.NewDecoder(req.Body) + dec := jsoniter.NewDecoder(req.Body) return dec.Decode(v) } @@ -418,9 +415,16 @@ func writeStatus(ctx *context.Context, status int) { writeStatusMessage(ctx, status, http.StatusText(status)) } -func writeStatusMessage(ctx *context.Context, status int, message interface{}) { +func writeStatusMessage(ctx *context.Context, status int, message string) { + ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) ctx.Resp.WriteHeader(status) - fmt.Fprintf(ctx.Resp, `{"message":"%v"}`, message) + + er := lfs_module.ErrorResponse{Message: message} + + enc := jsoniter.NewEncoder(ctx.Resp) + if err := enc.Encode(er); err != nil { + log.Error("Failed to encode error response as json. Error: %v", err) + } } // authenticate uses the authorization string to determine whether From 3093f7651912f70ed149e470f382f8701100dc01 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 18 Apr 2021 23:26:27 +0000 Subject: [PATCH 24/30] Test against message property. --- integrations/lfs_getobject_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go index 3005a1a623d4f..89bcd1a8d597c 100644 --- a/integrations/lfs_getobject_test.go +++ b/integrations/lfs_getobject_test.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers/routes" + jsoniter "github.com/json-iterator/go" gzipp "github.com/klauspost/compress/gzip" "github.com/stretchr/testify/assert" ) @@ -198,7 +199,7 @@ func TestGetLFSRange(t *testing.T) { {"bytes=0-10", "123456789\n", http.StatusPartialContent}, // end-range bigger than length-1 is ignored {"bytes=0-11", "123456789\n", http.StatusPartialContent}, - {"bytes=11-", "{\"message\":\"Requested Range Not Satisfiable\"}", http.StatusRequestedRangeNotSatisfiable}, + {"bytes=11-", "Requested Range Not Satisfiable", http.StatusRequestedRangeNotSatisfiable}, // incorrect header value cause whole header to be ignored {"bytes=-", "123456789\n", http.StatusOK}, {"foobar", "123456789\n", http.StatusOK}, @@ -210,7 +211,14 @@ func TestGetLFSRange(t *testing.T) { "Range": []string{tt.in}, } resp := storeAndGetLfs(t, &content, &h, tt.status) - assert.Equal(t, tt.out, resp.Body.String()) + if tt.status == http.StatusPartialContent || tt.status == http.StatusOK { + assert.Equal(t, tt.out, resp.Body.String()) + } else { + var er lfs.ErrorResponse + err := jsoniter.Unmarshal(resp.Body.Bytes(), &er) + assert.NoError(t, err) + assert.Equal(t, tt.out, er.Message) + } }) } } From 1c3ac292b7fefd14afb927d620785b8de505b447 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 27 Apr 2021 14:16:54 +0000 Subject: [PATCH 25/30] Add support for the old invalid lfs client. --- services/lfs/server.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index f39f41acd8038..191d689ba7bac 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -393,11 +393,9 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa rep.Actions = make(map[string]*lfs_module.Link) header := make(map[string]string) - verifyHeader := make(map[string]string) if len(rc.Authorization) > 0 { header["Authorization"] = rc.Authorization - verifyHeader["Authorization"] = rc.Authorization } if download { @@ -405,6 +403,15 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa } if upload { rep.Actions["upload"] = &lfs_module.Link{Href: rc.UploadLink(pointer), Header: header} + + verifyHeader := make(map[string]string) + for key, value := range header { + verifyHeader[key] = value + } + + // This is only needed to workaround https://github.com/git-lfs/git-lfs/issues/3662 + verifyHeader["Accept"] = lfs_module.MediaType + rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(pointer), Header: verifyHeader} } } From 1bcbff877779ba91841de2bcd171ecdc10936a97 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 27 Apr 2021 21:17:00 +0200 Subject: [PATCH 26/30] Fixed the check because MinIO wraps the error. --- services/lfs/server.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 191d689ba7bac..910a08322fb38 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. @@ -6,6 +6,7 @@ package lfs import ( "encoding/base64" + "errors" "fmt" "io" "net/http" @@ -291,7 +292,7 @@ func UploadHandler(ctx *context.Context) { defer ctx.Req.Body.Close() if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil { - if err == lfs_module.ErrSizeMismatch || err == lfs_module.ErrHashMismatch { + if errors.Is(err, lfs_module.ErrSizeMismatch) || errors.Is(err, lfs_module.ErrHashMismatch) { writeStatusMessage(ctx, http.StatusUnprocessableEntity, err.Error()) } else { writeStatus(ctx, http.StatusInternalServerError) From 9eaa2e4bedc26c84f27bd8976fedb272060f1cf8 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 27 Apr 2021 21:21:00 +0200 Subject: [PATCH 27/30] Use individual repositories. --- integrations/api_repo_lfs_test.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/integrations/api_repo_lfs_test.go b/integrations/api_repo_lfs_test.go index d40e5c7caf325..11648a55a51c9 100644 --- a/integrations/api_repo_lfs_test.go +++ b/integrations/api_repo_lfs_test.go @@ -54,13 +54,23 @@ func TestAPILFSMediaType(t *testing.T) { MakeRequest(t, req, http.StatusUnsupportedMediaType) } +func createLFSTestRepository(t *testing.T, name string) *models.Repository { + ctx := NewAPITestContext(t, "user2", "lfs-"+name+"-repo") + t.Run("CreateRepo", doAPICreateRepository(ctx, false)) + + repo, err := models.GetRepositoryByOwnerAndName("user2", "lfs-"+name+"-repo") + assert.NoError(t, err) + + return repo +} + func TestAPILFSBatch(t *testing.T) { defer prepareTestEnv(t)() setting.LFS.StartServer = true - repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1") - assert.NoError(t, err) + repo := createLFSTestRepository(t, "batch") + content := []byte("dummy1") oid := storeObjectInRepo(t, repo.ID, &content) defer repo.RemoveLFSMetaObjectByOid(oid) @@ -68,7 +78,7 @@ func TestAPILFSBatch(t *testing.T) { session := loginUser(t, "user2") newRequest := func(t testing.TB, br *lfs.BatchRequest) *http.Request { - req := NewRequestWithJSON(t, "POST", "/user2/repo1.git/info/lfs/objects/batch", br) + req := NewRequestWithJSON(t, "POST", "/user2/lfs-batch-repo.git/info/lfs/objects/batch", br) req.Header.Set("Accept", lfs.MediaType) req.Header.Set("Content-Type", lfs.MediaType) return req @@ -312,8 +322,8 @@ func TestAPILFSUpload(t *testing.T) { setting.LFS.StartServer = true - repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1") - assert.NoError(t, err) + repo := createLFSTestRepository(t, "upload") + content := []byte("dummy3") oid := storeObjectInRepo(t, repo.ID, &content) defer repo.RemoveLFSMetaObjectByOid(oid) @@ -321,7 +331,7 @@ func TestAPILFSUpload(t *testing.T) { session := loginUser(t, "user2") newRequest := func(t testing.TB, p lfs.Pointer, content string) *http.Request { - req := NewRequestWithBody(t, "PUT", path.Join("/user2/repo1.git/info/lfs/objects/", p.Oid, strconv.FormatInt(p.Size, 10)), strings.NewReader(content)) + req := NewRequestWithBody(t, "PUT", path.Join("/user2/lfs-upload-repo.git/info/lfs/objects/", p.Oid, strconv.FormatInt(p.Size, 10)), strings.NewReader(content)) return req } @@ -407,8 +417,8 @@ func TestAPILFSVerify(t *testing.T) { setting.LFS.StartServer = true - repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1") - assert.NoError(t, err) + repo := createLFSTestRepository(t, "verify") + content := []byte("dummy3") oid := storeObjectInRepo(t, repo.ID, &content) defer repo.RemoveLFSMetaObjectByOid(oid) @@ -416,7 +426,7 @@ func TestAPILFSVerify(t *testing.T) { session := loginUser(t, "user2") newRequest := func(t testing.TB, p *lfs.Pointer) *http.Request { - req := NewRequestWithJSON(t, "POST", "/user2/repo1.git/info/lfs/verify", p) + req := NewRequestWithJSON(t, "POST", "/user2/lfs-verify-repo.git/info/lfs/verify", p) req.Header.Set("Accept", lfs.MediaType) req.Header.Set("Content-Type", lfs.MediaType) return req From a3d323b02a79754588e54230efe11b04bd46915c Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 27 Apr 2021 21:24:00 +0200 Subject: [PATCH 28/30] Removed unnecessary postgresql workaround. --- integrations/lfs_getobject_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go index 89bcd1a8d597c..b7423a2dbe55f 100644 --- a/integrations/lfs_getobject_test.go +++ b/integrations/lfs_getobject_test.go @@ -22,21 +22,11 @@ import ( "github.com/stretchr/testify/assert" ) -var lfsID = int64(20000) - func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string { pointer, err := lfs.GeneratePointer(bytes.NewReader(*content)) assert.NoError(t, err) - var lfsMetaObject *models.LFSMetaObject - - if setting.Database.UsePostgreSQL { - lfsMetaObject = &models.LFSMetaObject{ID: lfsID, Pointer: pointer, RepositoryID: repositoryID} - } else { - lfsMetaObject = &models.LFSMetaObject{Pointer: pointer, RepositoryID: repositoryID} - } - lfsID++ - lfsMetaObject, err = models.NewLFSMetaObject(lfsMetaObject) + _, err = models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: pointer, RepositoryID: repositoryID}) assert.NoError(t, err) contentStore := lfs.NewContentStore() exist, err := contentStore.Exists(pointer) From b0e6b8e2dbd4bc28392ecd2cc6862a2de63b5b5f Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 5 Jun 2021 13:56:10 +0000 Subject: [PATCH 29/30] Use StatusNotFound instead of StatusForbidden. --- routers/routes/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 97617236a00ba..fbc41d547d163 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -288,7 +288,7 @@ func RegisterRoutes(m *web.Route) { lfsServerEnabled := func(ctx *context.Context) { if !setting.LFS.StartServer { - ctx.Error(http.StatusForbidden) + ctx.Error(http.StatusNotFound) return } } From 9f29ed54011cbf3a85b80880af9b1d6eb2217521 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 5 Jun 2021 15:06:04 +0000 Subject: [PATCH 30/30] Fixed status codes in tests. --- integrations/api_repo_lfs_locks_test.go | 8 ++++---- integrations/api_repo_lfs_test.go | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 37a7714146fc8..03549c11f4c20 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -25,13 +25,13 @@ func TestAPILFSLocksNotStarted(t *testing.T) { repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) req := NewRequestf(t, "GET", "/%s/%s.git/info/lfs/locks", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) req = NewRequestf(t, "POST", "/%s/%s.git/info/lfs/locks", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/locks/verify", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/locks/10/unlock", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) } func TestAPILFSLocksNotLogin(t *testing.T) { diff --git a/integrations/api_repo_lfs_test.go b/integrations/api_repo_lfs_test.go index 11648a55a51c9..d0328fd12110c 100644 --- a/integrations/api_repo_lfs_test.go +++ b/integrations/api_repo_lfs_test.go @@ -29,15 +29,15 @@ func TestAPILFSNotStarted(t *testing.T) { repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) req := NewRequestf(t, "POST", "/%s/%s.git/info/lfs/objects/batch", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) req = NewRequestf(t, "PUT", "/%s/%s.git/info/lfs/objects/oid/10", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/objects/oid/name", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) req = NewRequestf(t, "GET", "/%s/%s.git/info/lfs/objects/oid", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) req = NewRequestf(t, "POST", "/%s/%s.git/info/lfs/verify", user.Name, repo.Name) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) } func TestAPILFSMediaType(t *testing.T) {