diff --git a/README.md b/README.md index b5a213b..241e9b3 100644 --- a/README.md +++ b/README.md @@ -62,9 +62,9 @@ func main() { } ``` You can add optional parameters to constructor function. -* `WithoutUploadPack`: Disabled `git-upload-pack` command. -* `WithoutReceivePack`: Disabled `git-receive-pack` command. -* `WithoutDumbProto`: Disabled `dumb protocol` handling. +* `WithoutUploadPack` : Disable `git-upload-pack` command. +* `WithoutReceivePack`: Disable `git-receive-pack` command. +* `WithoutDumbProto` : Disable `dumb protocol` handling. ```go ght, err := githttptransfer.New( "/data/git", @@ -88,13 +88,12 @@ func main() { ght.Router.Add(githttptransfer.NewRoute( http.MethodGet, regexp.MustCompile("(.*?)/hello$"), - func(ctx githttptransfer.Context) error { + func(ctx githttptransfer.Context) { resp, req := ctx.Response(), ctx.Request() rp, fp := ctx.RepoPath(), ctx.FilePath() fmt.Fprintf(resp.Writer, "Hi there. URI: %s, RepoPath: %s, FilePath: %s", req.URL.RequestURI(), rp, fp) - return nil }, )) diff --git a/addon/archivehandler/archivehandler.go b/addon/archivehandler/archivehandler.go index fe68305..ee59b65 100644 --- a/addon/archivehandler/archivehandler.go +++ b/addon/archivehandler/archivehandler.go @@ -23,7 +23,7 @@ type ArchiveHandler struct { *githttptransfer.GitHTTPTransfer } -func (ght *ArchiveHandler) HandlerFunc(ctx githttptransfer.Context) error { +func (ght *ArchiveHandler) HandlerFunc(ctx githttptransfer.Context) { res, repoPath, filePath := ctx.Response(), ctx.RepoPath(), ctx.FilePath() @@ -38,12 +38,14 @@ func (ght *ArchiveHandler) HandlerFunc(ctx githttptransfer.Context) error { stdout, err := cmd.StdoutPipe() if err != nil { - return err + githttptransfer.RenderInternalServerError(res.Writer) + return } defer stdout.Close() if err := cmd.Start(); err != nil { - return err + githttptransfer.RenderInternalServerError(res.Writer) + return } res.SetContentType("application/octet-stream") @@ -51,10 +53,10 @@ func (ght *ArchiveHandler) HandlerFunc(ctx githttptransfer.Context) error { res.Header().Add("Content-Transfer-Encoding", "binary") if _, err := res.Copy(stdout); err != nil { - return err + githttptransfer.RenderInternalServerError(res.Writer) + return } if err := cmd.Wait(); err != nil { - return err + githttptransfer.RenderInternalServerError(res.Writer) } - return nil } diff --git a/example/main.go b/example/main.go index 2b5c029..18bd80f 100644 --- a/example/main.go +++ b/example/main.go @@ -25,32 +25,28 @@ func main() { return } - ght.Event.On(githttptransfer.PrepareServiceRPCUpload, func(ctx githttptransfer.Context) error { + ght.Event.On(githttptransfer.PrepareServiceRPCUpload, func(ctx githttptransfer.Context) { // prepare run service rpc upload. - return nil }) - ght.Event.On(githttptransfer.PrepareServiceRPCReceive, func(ctx githttptransfer.Context) error { + ght.Event.On(githttptransfer.PrepareServiceRPCReceive, func(ctx githttptransfer.Context) { // prepare run service rpc receive. - return nil }) - ght.Event.On(githttptransfer.AfterMatchRouting, func(ctx githttptransfer.Context) error { + ght.Event.On(githttptransfer.AfterMatchRouting, func(ctx githttptransfer.Context) { // after match routing. - return nil }) // You can add some custom route. ght.Router.Add(githttptransfer.NewRoute( http.MethodGet, regexp.MustCompile("(.*?)/hello$"), - func(ctx githttptransfer.Context) error { + func(ctx githttptransfer.Context) { resp, req := ctx.Response(), ctx.Request() rp, fp := ctx.RepoPath(), ctx.FilePath() fmt.Fprintf(resp.Writer, "Hi there. URI: %s, RepoPath: %s, FilePath: %s", req.URL.RequestURI(), rp, fp) - return nil }, )) diff --git a/githttptransfer/error.go b/githttptransfer/error.go index 460cc6c..06f6a46 100644 --- a/githttptransfer/error.go +++ b/githttptransfer/error.go @@ -20,10 +20,3 @@ func (e *MethodNotAllowedError) Error() string { return fmt.Sprintf("Method Not Allowed: Method %s, Path %s", e.Method, e.Path) } -type NoAccessError struct { - Dir string -} - -func (e *NoAccessError) Error() string { - return "No Access: " + e.Dir -} diff --git a/githttptransfer/githttptransfer.go b/githttptransfer/githttptransfer.go index 43a99cc..b5876bd 100644 --- a/githttptransfer/githttptransfer.go +++ b/githttptransfer/githttptransfer.go @@ -25,33 +25,33 @@ var ( getIdxFile = regexp.MustCompile("(.*?)/objects/pack/pack-[0-9a-f]{40}\\.idx$") ) -type gitHTTPTransferOptions struct { - uploadPack bool +type options struct { + uploadPack bool receivePack bool - dumbProto bool + dumbProto bool } -type GitHTTPTransferOption func(*gitHTTPTransferOptions) +type Option func(*options) -func WithoutUploadPack() GitHTTPTransferOption { - return func(o *gitHTTPTransferOptions) { +func WithoutUploadPack() Option { + return func(o *options) { o.uploadPack = false } } -func WithoutReceivePack() GitHTTPTransferOption { - return func(o *gitHTTPTransferOptions) { +func WithoutReceivePack() Option { + return func(o *options) { o.receivePack = false } } -func WithoutDumbProto() GitHTTPTransferOption { - return func(o *gitHTTPTransferOptions) { +func WithoutDumbProto() Option { + return func(o *options) { o.dumbProto = false } } -func New(gitRootPath, gitBinPath string, opts ...GitHTTPTransferOption) (*GitHTTPTransfer, error) { +func New(gitRootPath, gitBinPath string, opts ...Option) (*GitHTTPTransfer, error) { if gitRootPath == "" { cwd, err := os.Getwd() @@ -61,7 +61,7 @@ func New(gitRootPath, gitBinPath string, opts ...GitHTTPTransferOption) (*GitHTT gitRootPath = cwd } - ghtOpts := &gitHTTPTransferOptions{true, true, true} + ghtOpts := &options{true, true, true} for _, opt := range opts { opt(ghtOpts) @@ -109,28 +109,14 @@ func (ght *GitHTTPTransfer) ServeHTTP(rw http.ResponseWriter, r *http.Request) { ctx := NewContext(rw, r, repoPath, filePath) - if err := ght.Event.emit(AfterMatchRouting, ctx); err != nil { - RenderInternalServerError(ctx.Response().Writer) - return - } + ght.Event.emit(AfterMatchRouting, ctx) if !ght.Git.Exists(ctx.RepoPath()) { RenderNotFound(ctx.Response().Writer) return } - if err := handler(ctx); err != nil { - if os.IsNotExist(err) { - RenderNotFound(ctx.Response().Writer) - return - } - switch err.(type) { - case *NoAccessError: - RenderNoAccess(ctx.Response().Writer) - return - } - RenderInternalServerError(ctx.Response().Writer) - } + handler(ctx) } func (ght *GitHTTPTransfer) matchRouting(method, path string) (repoPath string, filePath string, handler HandlerFunc, err error) { @@ -144,11 +130,11 @@ func (ght *GitHTTPTransfer) matchRouting(method, path string) (repoPath string, } const ( - uploadPack string = "upload-pack" - receivePack string = "receive-pack" + uploadPack = "upload-pack" + receivePack = "receive-pack" ) -type HandlerFunc func(ctx Context) error +type HandlerFunc func(ctx Context) func newEvent() *event { return &event{map[EventKey]HandlerFunc{}} @@ -166,38 +152,34 @@ type event struct { listeners map[EventKey]HandlerFunc } -func (e *event) emit(evt EventKey, ctx Context) error { +func (e *event) emit(evt EventKey, ctx Context) { v, ok := e.listeners[evt] if ok { - return v(ctx) + v(ctx) } - return nil } func (e *event) On(evt EventKey, listener HandlerFunc) { e.listeners[evt] = listener } -func (ght *GitHTTPTransfer) serviceRPCUpload(ctx Context) error { - if err := ght.Event.emit(PrepareServiceRPCUpload, ctx); err != nil { - return err - } - return ght.serviceRPC(ctx, uploadPack) +func (ght *GitHTTPTransfer) serviceRPCUpload(ctx Context) { + ght.Event.emit(PrepareServiceRPCUpload, ctx) + ght.serviceRPC(ctx, uploadPack) } -func (ght *GitHTTPTransfer) serviceRPCReceive(ctx Context) error { - if err := ght.Event.emit(PrepareServiceRPCReceive, ctx); err != nil { - return err - } - return ght.serviceRPC(ctx, receivePack) +func (ght *GitHTTPTransfer) serviceRPCReceive(ctx Context) { + ght.Event.emit(PrepareServiceRPCReceive, ctx) + ght.serviceRPC(ctx, receivePack) } -func (ght *GitHTTPTransfer) serviceRPC(ctx Context, rpc string) error { +func (ght *GitHTTPTransfer) serviceRPC(ctx Context, rpc string) { res, req, repoPath := ctx.Response(), ctx.Request(), ctx.RepoPath() if !ght.Git.HasAccess(req, rpc, true) { - return &NoAccessError{Dir: ght.Git.GetAbsolutePath(repoPath)} + RenderNoAccess(ctx.Response().Writer) + return } var body io.ReadCloser @@ -206,7 +188,8 @@ func (ght *GitHTTPTransfer) serviceRPC(ctx Context, rpc string) error { if req.Header.Get("Content-Encoding") == "gzip" { body, err = gzip.NewReader(req.Body) if err != nil { - return err + RenderInternalServerError(ctx.Response().Writer) + return } } else { body = req.Body @@ -220,17 +203,19 @@ func (ght *GitHTTPTransfer) serviceRPC(ctx Context, rpc string) error { stdin, err := cmd.StdinPipe() if err != nil { - return err + RenderInternalServerError(ctx.Response().Writer) + return } stdout, err := cmd.StdoutPipe() if err != nil { - return err + RenderInternalServerError(ctx.Response().Writer) + return } - err = cmd.Start() // could be merged in one statement - if err != nil { - return err + if err = cmd.Start(); err != nil { + RenderInternalServerError(ctx.Response().Writer) + return } var wg sync.WaitGroup @@ -250,23 +235,28 @@ func (ght *GitHTTPTransfer) serviceRPC(ctx Context, rpc string) error { wg.Wait() - return cmd.Wait() - + if err = cmd.Wait(); err != nil { + RenderInternalServerError(ctx.Response().Writer) + return + } } -func (ght *GitHTTPTransfer) getInfoRefs(ctx Context) error { +func (ght *GitHTTPTransfer) getInfoRefs(ctx Context) { res, req, repoPath := ctx.Response(), ctx.Request(), ctx.RepoPath() serviceName := getServiceType(req) if !ght.Git.HasAccess(req, serviceName, false) { ght.Git.UpdateServerInfo(repoPath) res.HdrNocache() - return ght.sendFile("text/plain; charset=utf-8", ctx) + if err := ght.sendFile("text/plain; charset=utf-8", ctx); err != nil { + RenderNotFound(ctx.Response().Writer) + } } refs, err := ght.Git.GetInfoRefs(repoPath, serviceName) if err != nil { - return err + RenderNotFound(ctx.Response().Writer) + return } res.HdrNocache() @@ -275,33 +265,42 @@ func (ght *GitHTTPTransfer) getInfoRefs(ctx Context) error { res.PktWrite("# service=git-" + serviceName + "\n") res.PktFlush() res.Write(refs) - - return nil } -func (ght *GitHTTPTransfer) getInfoPacks(ctx Context) error { +func (ght *GitHTTPTransfer) getInfoPacks(ctx Context) { ctx.Response().HdrCacheForever() - return ght.sendFile("text/plain; charset=utf-8", ctx) + if err := ght.sendFile("text/plain; charset=utf-8", ctx); err != nil { + RenderNotFound(ctx.Response().Writer) + } } -func (ght *GitHTTPTransfer) getLooseObject(ctx Context) error { +func (ght *GitHTTPTransfer) getLooseObject(ctx Context) { ctx.Response().HdrCacheForever() - return ght.sendFile("application/x-git-loose-object", ctx) + if err := ght.sendFile("application/x-git-loose-object", ctx); err != nil { + RenderNotFound(ctx.Response().Writer) + } } -func (ght *GitHTTPTransfer) getPackFile(ctx Context) error { +func (ght *GitHTTPTransfer) getPackFile(ctx Context) { ctx.Response().HdrCacheForever() - return ght.sendFile("application/x-git-packed-objects", ctx) + if err := ght.sendFile("application/x-git-packed-objects", ctx); err != nil { + RenderNotFound(ctx.Response().Writer) + } + } -func (ght *GitHTTPTransfer) getIdxFile(ctx Context) error { +func (ght *GitHTTPTransfer) getIdxFile(ctx Context) { ctx.Response().HdrCacheForever() - return ght.sendFile("application/x-git-packed-objects-toc", ctx) + if err := ght.sendFile("application/x-git-packed-objects-toc", ctx); err != nil { + RenderNotFound(ctx.Response().Writer) + } } -func (ght *GitHTTPTransfer) getTextFile(ctx Context) error { +func (ght *GitHTTPTransfer) getTextFile(ctx Context) { ctx.Response().HdrNocache() - return ght.sendFile("text/plain", ctx) + if err := ght.sendFile("text/plain", ctx); err != nil { + RenderNotFound(ctx.Response().Writer) + } } func (ght *GitHTTPTransfer) sendFile(contentType string, ctx Context) error { @@ -314,6 +313,5 @@ func (ght *GitHTTPTransfer) sendFile(contentType string, ctx Context) error { res.SetContentLength(fmt.Sprintf("%d", fileInfo.Size())) res.SetLastModified(fileInfo.ModTime().Format(http.TimeFormat)) http.ServeFile(res.Writer, req, fileInfo.AbsolutePath) - return nil } diff --git a/githttptransfer/githttptransfer_end_to_end_test.go b/githttptransfer/githttptransfer_end_to_end_test.go index d3157ae..d5679f5 100644 --- a/githttptransfer/githttptransfer_end_to_end_test.go +++ b/githttptransfer/githttptransfer_end_to_end_test.go @@ -48,17 +48,14 @@ func setupEndToEndTest(t *testing.T) error { } endToEndTestParams.ght = ght - endToEndTestParams.ght.Event.On(PrepareServiceRPCUpload, func(ctx Context) error { + endToEndTestParams.ght.Event.On(PrepareServiceRPCUpload, func(ctx Context) { t.Log("prepare run service rpc upload.") - return nil }) - endToEndTestParams.ght.Event.On(PrepareServiceRPCReceive, func(ctx Context) error { + endToEndTestParams.ght.Event.On(PrepareServiceRPCReceive, func(ctx Context) { t.Log("prepare run service rpc receive.") - return nil }) - endToEndTestParams.ght.Event.On(AfterMatchRouting, func(ctx Context) error { + endToEndTestParams.ght.Event.On(AfterMatchRouting, func(ctx Context) { t.Log("after match routing.") - return nil }) endToEndTestParams.ts = httptest.NewServer(endToEndTestParams.ght) diff --git a/githttptransfer/githttptransfer_test.go b/githttptransfer/githttptransfer_test.go index 7a7a524..d1e8e89 100644 --- a/githttptransfer/githttptransfer_test.go +++ b/githttptransfer/githttptransfer_test.go @@ -15,24 +15,24 @@ func Test_GitHTTPTransfer_GitHTTPTransferOption(t *testing.T) { } tests := []struct { - description string - url string - contentsType string - expectedCode int - gitHTTPTransferOption GitHTTPTransferOption + description string + url string + contentsType string + expectedCode int + gitHTTPTransferOption Option }{ { - description: "it should return 403 if upload-pack is off", - url: "/test.git/git-upload-pack", - contentsType: "application/x-git-upload-pack-request", - expectedCode: 403, + description: "it should return 403 if upload-pack is off", + url: "/test.git/git-upload-pack", + contentsType: "application/x-git-upload-pack-request", + expectedCode: 403, gitHTTPTransferOption: WithoutUploadPack(), }, { - description: "it should return 403 if receive-pack is off", - url: "/test.git/git-receive-pack", - contentsType: "application/x-git-receive-pack-request", - expectedCode: 403, + description: "it should return 403 if receive-pack is off", + url: "/test.git/git-receive-pack", + contentsType: "application/x-git-receive-pack-request", + expectedCode: 403, gitHTTPTransferOption: WithoutReceivePack(), }, } @@ -68,7 +68,6 @@ func Test_GitHTTPTransfer_GitHTTPTransferOption(t *testing.T) { } - func Test_GitHTTPTransfer_MatchRouting_should_not_match(t *testing.T) { t.Log("it should not match if http method is different") var err error @@ -98,74 +97,74 @@ func Test_GitHTTPTransfer_MatchRouting_should_match(t *testing.T) { } tests := []struct { - description string - method string - path string + description string + method string + path string expectedRepoPath string expectedFilePath string }{ { - description: "it should match git-upload-pack", - method: http.MethodPost, - path: "/base/foo/git-upload-pack", + description: "it should match git-upload-pack", + method: http.MethodPost, + path: "/base/foo/git-upload-pack", expectedRepoPath: "/base/foo", expectedFilePath: "git-upload-pack", }, { - description: "it should match get-info-refs", - method: http.MethodGet, - path: "/base/foo/info/refs", + description: "it should match get-info-refs", + method: http.MethodGet, + path: "/base/foo/info/refs", expectedRepoPath: "/base/foo", - expectedFilePath : "info/refs", + expectedFilePath: "info/refs", }, { - description: "it should match get-head", - method: http.MethodGet, - path: "/base/foo/HEAD", + description: "it should match get-head", + method: http.MethodGet, + path: "/base/foo/HEAD", expectedRepoPath: "/base/foo", - expectedFilePath : "HEAD", + expectedFilePath: "HEAD", }, { - description: "it should match get-alternates", - method: http.MethodGet, - path: "/base/foo/objects/info/alternates", + description: "it should match get-alternates", + method: http.MethodGet, + path: "/base/foo/objects/info/alternates", expectedRepoPath: "/base/foo", - expectedFilePath : "objects/info/alternates", + expectedFilePath: "objects/info/alternates", }, { - description: "it should match get-http-alternates", - method: http.MethodGet, - path: "/base/foo/objects/info/http-alternates", + description: "it should match get-http-alternates", + method: http.MethodGet, + path: "/base/foo/objects/info/http-alternates", expectedRepoPath: "/base/foo", - expectedFilePath : "objects/info/http-alternates", + expectedFilePath: "objects/info/http-alternates", }, { - description: "it should match get-info-packs", - method: http.MethodGet, - path: "/base/foo/objects/info/packs", + description: "it should match get-info-packs", + method: http.MethodGet, + path: "/base/foo/objects/info/packs", expectedRepoPath: "/base/foo", - expectedFilePath : "objects/info/packs", + expectedFilePath: "objects/info/packs", }, { - description: "it should match get-loose-object", - method: http.MethodGet, - path: "/base/foo/objects/3b/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaacccccc", + description: "it should match get-loose-object", + method: http.MethodGet, + path: "/base/foo/objects/3b/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaacccccc", expectedRepoPath: "/base/foo", - expectedFilePath : "objects/3b/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaacccccc", + expectedFilePath: "objects/3b/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaacccccc", }, { - description: "it should match get-pack-file", - method: http.MethodGet, - path: "/base/foo/objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.pack", + description: "it should match get-pack-file", + method: http.MethodGet, + path: "/base/foo/objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.pack", expectedRepoPath: "/base/foo", - expectedFilePath : "objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.pack", + expectedFilePath: "objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.pack", }, { - description: "it should match get-idx-file", - method: http.MethodGet, - path: "/base/foo/objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.idx", + description: "it should match get-idx-file", + method: http.MethodGet, + path: "/base/foo/objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.idx", expectedRepoPath: "/base/foo", - expectedFilePath : "objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.idx", + expectedFilePath: "objects/pack/pack-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbb.idx", }, } @@ -186,4 +185,3 @@ func Test_GitHTTPTransfer_MatchRouting_should_match(t *testing.T) { } } } - diff --git a/githttptransfer/router_test.go b/githttptransfer/router_test.go index 7f25bdf..2f9ae4a 100644 --- a/githttptransfer/router_test.go +++ b/githttptransfer/router_test.go @@ -11,15 +11,13 @@ func Test_Router_Append_should_append_route(t *testing.T) { router.Add(&Route{ http.MethodPost, regexp.MustCompile("(.*?)/foo"), - func(ctx Context) error { - return nil - }}) + func(ctx Context) {}, + }) router.Add(&Route{ http.MethodPost, regexp.MustCompile("(.*?)/bar"), - func(ctx Context) error { - return nil - }}) + func(ctx Context) {}, + }) length := len(router.routes) expected := 2 if expected != length { @@ -32,9 +30,8 @@ func Test_Router_Match_should_match_route(t *testing.T) { router.Add(&Route{ http.MethodPost, regexp.MustCompile("(.*?)/foo"), - func(ctx Context) error { - return nil - }}) + func(ctx Context) {}, + }) match, route, err := router.Match(http.MethodPost, "/base/foo") if err != nil { t.Errorf("error is %s", err.Error()) @@ -55,9 +52,8 @@ func Test_Router_Match_should_return_UrlNotFound_error(t *testing.T) { router.Add(&Route{ http.MethodPost, regexp.MustCompile("(.*?)/foo"), - func(ctx Context) error { - return nil - }}) + func(ctx Context) {}, + }) match, route, err := router.Match(http.MethodPost, "/base/hoge") if err == nil { t.Error("error is nil.") @@ -80,9 +76,8 @@ func Test_Router_Match_should_return_MethodNotAllowed_error(t *testing.T) { router.Add(&Route{ http.MethodPost, regexp.MustCompile("(.*?)/foo"), - func(ctx Context) error { - return nil - }}) + func(ctx Context) {}, + }) match, route, err := router.Match(http.MethodGet, "/base/foo") if err == nil { t.Error("error is nil.") diff --git a/githttptransfer/support_test.go b/githttptransfer/support_test.go index 7028e53..2c23daf 100644 --- a/githttptransfer/support_test.go +++ b/githttptransfer/support_test.go @@ -10,27 +10,27 @@ func Test_GetServiceType(t *testing.T) { tests := []struct { description string - method string - url string - expected string + method string + url string + expected string }{ { description: "it should return upload-pack", - method: http.MethodPost, - url: "http://example.com/base/foo/git-upload-pack?service=git-upload-pack", - expected: "upload-pack", + method: http.MethodPost, + url: "http://example.com/base/foo/git-upload-pack?service=git-upload-pack", + expected: "upload-pack", }, { description: "it should return receive-pack", - method: http.MethodPost, - url: "http://example.com/base/foo/git-upload-pack?service=git-receive-pack", - expected: "receive-pack", + method: http.MethodPost, + url: "http://example.com/base/foo/git-upload-pack?service=git-receive-pack", + expected: "receive-pack", }, { description: "it should return empty", - method: http.MethodPost, - url: "http://example.com/base/foo/git-upload-pack?service=foo-receive-pack", - expected: "", + method: http.MethodPost, + url: "http://example.com/base/foo/git-upload-pack?service=foo-receive-pack", + expected: "", }, }