From f4538791f5fc82b173608fcf9c30e36ec01dc9d3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 29 Mar 2023 14:32:26 +0800 Subject: [PATCH] Refactor internal API for git commands, use meaningful messages instead of "Internal Server Error" (#23687) # Why this PR comes At first, I'd like to help users like #23636 (there are a lot) The unclear "Internal Server Error" is quite anonying, scare users, frustrate contributors, nobody knows what happens. So, it's always good to provide meaningful messages to end users (of course, do not leak sensitive information). When I started working on the "response message to end users", I found that the related code has a lot of technical debt. A lot of copy&paste code, unclear fields and usages. So I think it's good to make everything clear. # Tech Backgrounds Gitea has many sub-commands, some are used by admins, some are used by SSH servers or Git Hooks. Many sub-commands use "internal API" to communicate with Gitea web server. Before, Gitea server always use `StatusCode + Json "err" field` to return messages. * The CLI sub-commands: they expect to show all error related messages to site admin * The Serv/Hook sub-commands (for git clients): they could only show safe messages to end users, the error log could only be recorded by "SSHLog" to Gitea web server. In the old design, it assumes that: * If the StatusCode is 500 (in some functions), then the "err" field is error log, shouldn't be exposed to git client. * If the StatusCode is 40x, then the "err" field could be exposed. And some functions always read the "err" no matter what the StatusCode is. The old code is not strict, and it's difficult to distinguish the messages clearly and then output them correctly. # This PR To help to remove duplicate code and make everything clear, this PR introduces `ResponseExtra` and `requestJSONResp`. * `ResponseExtra` is a struct which contains "extra" information of a internal API response, including StatusCode, UserMsg, Error * `requestJSONResp` is a generic function which can be used for all cases to help to simplify the calls. * Remove all `map["err"]`, always use `private.Response{Err}` to construct error messages. * User messages and error messages are separated clearly, the `fail` and `handleCliResponseExtra` will output correct messages. * Replace all `Internal Server Error` messages with meaningful (still safe) messages. This PR saves more than 300 lines, while makes the git client messages more clear. Many gitea-serv/git-hook related essential functions are covered by tests. --------- Co-authored-by: delvh --- build/generate-emoji.go | 4 +- cmd/hook.go | 144 ++++++++--------- cmd/hook_test.go | 10 +- cmd/keys.go | 9 +- cmd/mailer.go | 12 +- cmd/manager.go | 49 ++---- cmd/manager_logging.go | 93 +++++------ cmd/restore_repo.go | 12 +- cmd/serv.go | 99 +++++++----- modules/httplib/httplib.go | 14 +- modules/private/hook.go | 125 +++------------ modules/private/internal.go | 56 +++---- modules/private/key.go | 35 +---- modules/private/mail.go | 34 +--- modules/private/manager.go | 171 ++++----------------- modules/private/request.go | 135 ++++++++++++++++ modules/private/restore_repo.go | 34 +--- modules/private/serv.go | 65 ++------ routers/private/default_branch.go | 14 -- routers/private/hook_post_receive.go | 1 - routers/private/hook_pre_receive.go | 39 +++-- routers/private/hook_proc_receive.go | 5 +- routers/private/hook_verification.go | 14 -- routers/private/internal.go | 2 +- routers/private/internal_repo.go | 26 +--- routers/private/key.go | 1 - routers/private/manager.go | 4 +- routers/private/manager_windows.go | 2 +- routers/private/serv.go | 141 +++++++---------- tests/integration/api_private_serv_test.go | 48 +++--- 30 files changed, 547 insertions(+), 851 deletions(-) create mode 100644 modules/private/request.go diff --git a/build/generate-emoji.go b/build/generate-emoji.go index 60350336c..09bdeb680 100644 --- a/build/generate-emoji.go +++ b/build/generate-emoji.go @@ -60,13 +60,13 @@ func main() { // generate data buf, err := generate() if err != nil { - log.Fatal(err) + log.Fatalf("generate err: %v", err) } // write err = os.WriteFile(*flagOut, buf, 0o644) if err != nil { - log.Fatal(err) + log.Fatalf("WriteFile err: %v", err) } } diff --git a/cmd/hook.go b/cmd/hook.go index a62ffdde5..9605fcb33 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -6,9 +6,9 @@ package cmd import ( "bufio" "bytes" + "context" "fmt" "io" - "net/http" "os" "strconv" "strings" @@ -167,11 +167,11 @@ func runHookPreReceive(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("hooks/pre-receive.log", c.Bool("debug")) + setup(ctx, c.Bool("debug")) if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 { if setting.OnlyAllowPushIfGiteaEnvironmentSet { - return fail(`Rejecting changes as Gitea environment not set. + return fail(ctx, `Rejecting changes as Gitea environment not set. If you are pushing over SSH you must push with a key managed by Gitea or set your environment appropriately.`, "") } @@ -257,14 +257,9 @@ Gitea or set your environment appropriately.`, "") hookOptions.OldCommitIDs = oldCommitIDs hookOptions.NewCommitIDs = newCommitIDs hookOptions.RefFullNames = refFullNames - statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions) - switch statusCode { - case http.StatusOK: - // no-op - case http.StatusInternalServerError: - return fail("Internal Server Error", msg) - default: - return fail(msg, "") + extra := private.HookPreReceive(ctx, username, reponame, hookOptions) + if extra.HasError() { + return fail(ctx, extra.UserMsg, "HookPreReceive(batch) failed: %v", extra.Error) } count = 0 lastline = 0 @@ -285,12 +280,9 @@ Gitea or set your environment appropriately.`, "") fmt.Fprintf(out, " Checking %d references\n", count) - statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions) - switch statusCode { - case http.StatusInternalServerError: - return fail("Internal Server Error", msg) - case http.StatusForbidden: - return fail(msg, "") + extra := private.HookPreReceive(ctx, username, reponame, hookOptions) + if extra.HasError() { + return fail(ctx, extra.UserMsg, "HookPreReceive(last) failed: %v", extra.Error) } } else if lastline > 0 { fmt.Fprintf(out, "\n") @@ -309,7 +301,7 @@ func runHookPostReceive(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("hooks/post-receive.log", c.Bool("debug")) + setup(ctx, c.Bool("debug")) // First of all run update-server-info no matter what if _, _, err := git.NewCommand(ctx, "update-server-info").RunStdString(nil); err != nil { @@ -323,7 +315,7 @@ func runHookPostReceive(c *cli.Context) error { if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 { if setting.OnlyAllowPushIfGiteaEnvironmentSet { - return fail(`Rejecting changes as Gitea environment not set. + return fail(ctx, `Rejecting changes as Gitea environment not set. If you are pushing over SSH you must push with a key managed by Gitea or set your environment appropriately.`, "") } @@ -394,11 +386,11 @@ Gitea or set your environment appropriately.`, "") hookOptions.OldCommitIDs = oldCommitIDs hookOptions.NewCommitIDs = newCommitIDs hookOptions.RefFullNames = refFullNames - resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) - if resp == nil { + resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) + if extra.HasError() { _ = dWriter.Close() hookPrintResults(results) - return fail("Internal Server Error", err) + return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error) } wasEmpty = wasEmpty || resp.RepoWasEmpty results = append(results, resp.Results...) @@ -409,9 +401,9 @@ Gitea or set your environment appropriately.`, "") if count == 0 { if wasEmpty && masterPushed { // We need to tell the repo to reset the default branch to master - err := private.SetDefaultBranch(ctx, repoUser, repoName, "master") - if err != nil { - return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err) + extra := private.SetDefaultBranch(ctx, repoUser, repoName, "master") + if extra.HasError() { + return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error) } } fmt.Fprintf(out, "Processed %d references in total\n", total) @@ -427,11 +419,11 @@ Gitea or set your environment appropriately.`, "") fmt.Fprintf(out, " Processing %d references\n", count) - resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) + resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) if resp == nil { _ = dWriter.Close() hookPrintResults(results) - return fail("Internal Server Error", err) + return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error) } wasEmpty = wasEmpty || resp.RepoWasEmpty results = append(results, resp.Results...) @@ -440,9 +432,9 @@ Gitea or set your environment appropriately.`, "") if wasEmpty && masterPushed { // We need to tell the repo to reset the default branch to master - err := private.SetDefaultBranch(ctx, repoUser, repoName, "master") - if err != nil { - return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err) + extra := private.SetDefaultBranch(ctx, repoUser, repoName, "master") + if extra.HasError() { + return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error) } } _ = dWriter.Close() @@ -485,22 +477,22 @@ func pushOptions() map[string]string { } func runHookProcReceive(c *cli.Context) error { - setup("hooks/proc-receive.log", c.Bool("debug")) + ctx, cancel := installSignals() + defer cancel() + + setup(ctx, c.Bool("debug")) if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 { if setting.OnlyAllowPushIfGiteaEnvironmentSet { - return fail(`Rejecting changes as Gitea environment not set. + return fail(ctx, `Rejecting changes as Gitea environment not set. If you are pushing over SSH you must push with a key managed by Gitea or set your environment appropriately.`, "") } return nil } - ctx, cancel := installSignals() - defer cancel() - if git.CheckGitVersionAtLeast("2.29") != nil { - return fail("Internal Server Error", "git not support proc-receive.") + return fail(ctx, "No proc-receive support", "current git version doesn't support proc-receive.") } reader := bufio.NewReader(os.Stdin) @@ -515,7 +507,7 @@ Gitea or set your environment appropriately.`, "") // H: PKT-LINE(version=1\0push-options...) // H: flush-pkt - rs, err := readPktLine(reader, pktLineTypeData) + rs, err := readPktLine(ctx, reader, pktLineTypeData) if err != nil { return err } @@ -530,19 +522,19 @@ Gitea or set your environment appropriately.`, "") index := bytes.IndexByte(rs.Data, byte(0)) if index >= len(rs.Data) { - return fail("Internal Server Error", "pkt-line: format error "+fmt.Sprint(rs.Data)) + return fail(ctx, "Protocol: format error", "pkt-line: format error "+fmt.Sprint(rs.Data)) } if index < 0 { if len(rs.Data) == 10 && rs.Data[9] == '\n' { index = 9 } else { - return fail("Internal Server Error", "pkt-line: format error "+fmt.Sprint(rs.Data)) + return fail(ctx, "Protocol: format error", "pkt-line: format error "+fmt.Sprint(rs.Data)) } } if string(rs.Data[0:index]) != VersionHead { - return fail("Internal Server Error", "Received unsupported version: %s", string(rs.Data[0:index])) + return fail(ctx, "Protocol: version error", "Received unsupported version: %s", string(rs.Data[0:index])) } requestOptions = strings.Split(string(rs.Data[index+1:]), " ") @@ -555,17 +547,17 @@ Gitea or set your environment appropriately.`, "") } response = append(response, '\n') - _, err = readPktLine(reader, pktLineTypeFlush) + _, err = readPktLine(ctx, reader, pktLineTypeFlush) if err != nil { return err } - err = writeDataPktLine(os.Stdout, response) + err = writeDataPktLine(ctx, os.Stdout, response) if err != nil { return err } - err = writeFlushPktLine(os.Stdout) + err = writeFlushPktLine(ctx, os.Stdout) if err != nil { return err } @@ -588,7 +580,7 @@ Gitea or set your environment appropriately.`, "") for { // note: pktLineTypeUnknow means pktLineTypeFlush and pktLineTypeData all allowed - rs, err = readPktLine(reader, pktLineTypeUnknow) + rs, err = readPktLine(ctx, reader, pktLineTypeUnknow) if err != nil { return err } @@ -609,7 +601,7 @@ Gitea or set your environment appropriately.`, "") if hasPushOptions { for { - rs, err = readPktLine(reader, pktLineTypeUnknow) + rs, err = readPktLine(ctx, reader, pktLineTypeUnknow) if err != nil { return err } @@ -626,9 +618,9 @@ Gitea or set your environment appropriately.`, "") } // 3. run hook - resp, err := private.HookProcReceive(ctx, repoUser, repoName, hookOptions) - if err != nil { - return fail("Internal Server Error", "run proc-receive hook failed :%v", err) + resp, extra := private.HookProcReceive(ctx, repoUser, repoName, hookOptions) + if extra.HasError() { + return fail(ctx, extra.UserMsg, "HookProcReceive failed: %v", extra.Error) } // 4. response result to service @@ -649,7 +641,7 @@ Gitea or set your environment appropriately.`, "") for _, rs := range resp.Results { if len(rs.Err) > 0 { - err = writeDataPktLine(os.Stdout, []byte("ng "+rs.OriginalRef+" "+rs.Err)) + err = writeDataPktLine(ctx, os.Stdout, []byte("ng "+rs.OriginalRef+" "+rs.Err)) if err != nil { return err } @@ -657,43 +649,43 @@ Gitea or set your environment appropriately.`, "") } if rs.IsNotMatched { - err = writeDataPktLine(os.Stdout, []byte("ok "+rs.OriginalRef)) + err = writeDataPktLine(ctx, os.Stdout, []byte("ok "+rs.OriginalRef)) if err != nil { return err } - err = writeDataPktLine(os.Stdout, []byte("option fall-through")) + err = writeDataPktLine(ctx, os.Stdout, []byte("option fall-through")) if err != nil { return err } continue } - err = writeDataPktLine(os.Stdout, []byte("ok "+rs.OriginalRef)) + err = writeDataPktLine(ctx, os.Stdout, []byte("ok "+rs.OriginalRef)) if err != nil { return err } - err = writeDataPktLine(os.Stdout, []byte("option refname "+rs.Ref)) + err = writeDataPktLine(ctx, os.Stdout, []byte("option refname "+rs.Ref)) if err != nil { return err } if rs.OldOID != git.EmptySHA { - err = writeDataPktLine(os.Stdout, []byte("option old-oid "+rs.OldOID)) + err = writeDataPktLine(ctx, os.Stdout, []byte("option old-oid "+rs.OldOID)) if err != nil { return err } } - err = writeDataPktLine(os.Stdout, []byte("option new-oid "+rs.NewOID)) + err = writeDataPktLine(ctx, os.Stdout, []byte("option new-oid "+rs.NewOID)) if err != nil { return err } if rs.IsForcePush { - err = writeDataPktLine(os.Stdout, []byte("option forced-update")) + err = writeDataPktLine(ctx, os.Stdout, []byte("option forced-update")) if err != nil { return err } } } - err = writeFlushPktLine(os.Stdout) + err = writeFlushPktLine(ctx, os.Stdout) return err } @@ -718,7 +710,7 @@ type gitPktLine struct { Data []byte } -func readPktLine(in *bufio.Reader, requestType pktLineType) (*gitPktLine, error) { +func readPktLine(ctx context.Context, in *bufio.Reader, requestType pktLineType) (*gitPktLine, error) { var ( err error r *gitPktLine @@ -729,33 +721,33 @@ func readPktLine(in *bufio.Reader, requestType pktLineType) (*gitPktLine, error) for i := 0; i < 4; i++ { lengthBytes[i], err = in.ReadByte() if err != nil { - return nil, fail("Internal Server Error", "Pkt-Line: read stdin failed : %v", err) + return nil, fail(ctx, "Protocol: stdin error", "Pkt-Line: read stdin failed : %v", err) } } r = new(gitPktLine) r.Length, err = strconv.ParseUint(string(lengthBytes), 16, 32) if err != nil { - return nil, fail("Internal Server Error", "Pkt-Line format is wrong :%v", err) + return nil, fail(ctx, "Protocol: format parse error", "Pkt-Line format is wrong :%v", err) } if r.Length == 0 { if requestType == pktLineTypeData { - return nil, fail("Internal Server Error", "Pkt-Line format is wrong") + return nil, fail(ctx, "Protocol: format data error", "Pkt-Line format is wrong") } r.Type = pktLineTypeFlush return r, nil } if r.Length <= 4 || r.Length > 65520 || requestType == pktLineTypeFlush { - return nil, fail("Internal Server Error", "Pkt-Line format is wrong") + return nil, fail(ctx, "Protocol: format length error", "Pkt-Line format is wrong") } r.Data = make([]byte, r.Length-4) for i := range r.Data { r.Data[i], err = in.ReadByte() if err != nil { - return nil, fail("Internal Server Error", "Pkt-Line: read stdin failed : %v", err) + return nil, fail(ctx, "Protocol: data error", "Pkt-Line: read stdin failed : %v", err) } } @@ -764,19 +756,15 @@ func readPktLine(in *bufio.Reader, requestType pktLineType) (*gitPktLine, error) return r, nil } -func writeFlushPktLine(out io.Writer) error { +func writeFlushPktLine(ctx context.Context, out io.Writer) error { l, err := out.Write([]byte("0000")) - if err != nil { - return fail("Internal Server Error", "Pkt-Line response failed: %v", err) + if err != nil || l != 4 { + return fail(ctx, "Protocol: write error", "Pkt-Line response failed: %v", err) } - if l != 4 { - return fail("Internal Server Error", "Pkt-Line response failed: %v", err) - } - return nil } -func writeDataPktLine(out io.Writer, data []byte) error { +func writeDataPktLine(ctx context.Context, out io.Writer, data []byte) error { hexchar := []byte("0123456789abcdef") hex := func(n uint64) byte { return hexchar[(n)&15] @@ -790,19 +778,13 @@ func writeDataPktLine(out io.Writer, data []byte) error { tmp[3] = hex(length) lr, err := out.Write(tmp) - if err != nil { - return fail("Internal Server Error", "Pkt-Line response failed: %v", err) - } - if lr != 4 { - return fail("Internal Server Error", "Pkt-Line response failed: %v", err) + if err != nil || lr != 4 { + return fail(ctx, "Protocol: write error", "Pkt-Line response failed: %v", err) } lr, err = out.Write(data) - if err != nil { - return fail("Internal Server Error", "Pkt-Line response failed: %v", err) - } - if int(length-4) != lr { - return fail("Internal Server Error", "Pkt-Line response failed: %v", err) + if err != nil || int(length-4) != lr { + return fail(ctx, "Protocol: write error", "Pkt-Line response failed: %v", err) } return nil diff --git a/cmd/hook_test.go b/cmd/hook_test.go index fe1f072a6..91f24ff2b 100644 --- a/cmd/hook_test.go +++ b/cmd/hook_test.go @@ -6,6 +6,7 @@ package cmd import ( "bufio" "bytes" + "context" "strings" "testing" @@ -14,27 +15,28 @@ import ( func TestPktLine(t *testing.T) { // test read + ctx := context.Background() s := strings.NewReader("0000") r := bufio.NewReader(s) - result, err := readPktLine(r, pktLineTypeFlush) + result, err := readPktLine(ctx, r, pktLineTypeFlush) assert.NoError(t, err) assert.Equal(t, pktLineTypeFlush, result.Type) s = strings.NewReader("0006a\n") r = bufio.NewReader(s) - result, err = readPktLine(r, pktLineTypeData) + result, err = readPktLine(ctx, r, pktLineTypeData) assert.NoError(t, err) assert.Equal(t, pktLineTypeData, result.Type) assert.Equal(t, []byte("a\n"), result.Data) // test write w := bytes.NewBuffer([]byte{}) - err = writeFlushPktLine(w) + err = writeFlushPktLine(ctx, w) assert.NoError(t, err) assert.Equal(t, []byte("0000"), w.Bytes()) w.Reset() - err = writeDataPktLine(w, []byte("a\nb")) + err = writeDataPktLine(ctx, w, []byte("a\nb")) assert.NoError(t, err) assert.Equal(t, []byte("0007a\nb"), w.Bytes()) } diff --git a/cmd/keys.go b/cmd/keys.go index 74dc1cc68..deb94fca5 100644 --- a/cmd/keys.go +++ b/cmd/keys.go @@ -64,11 +64,12 @@ func runKeys(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("keys.log", false) + setup(ctx, false) - authorizedString, err := private.AuthorizedPublicKeyByContent(ctx, content) - if err != nil { - return err + authorizedString, extra := private.AuthorizedPublicKeyByContent(ctx, content) + // do not use handleCliResponseExtra or cli.NewExitError, if it exists immediately, it breaks some tests like Test_CmdKeys + if extra.Error != nil { + return extra.Error } fmt.Println(strings.TrimSpace(authorizedString)) return nil diff --git a/cmd/mailer.go b/cmd/mailer.go index d05fee12b..50ba4b474 100644 --- a/cmd/mailer.go +++ b/cmd/mailer.go @@ -5,7 +5,6 @@ package cmd import ( "fmt" - "net/http" "code.gitea.io/gitea/modules/private" "code.gitea.io/gitea/modules/setting" @@ -43,13 +42,10 @@ func runSendMail(c *cli.Context) error { } } - status, message := private.SendEmail(ctx, subject, body, nil) - if status != http.StatusOK { - fmt.Printf("error: %s\n", message) - return nil + respText, extra := private.SendEmail(ctx, subject, body, nil) + if extra.HasError() { + return handleCliResponseExtra(extra) } - - fmt.Printf("Success: %s\n", message) - + _, _ = fmt.Printf("Sent %s email(s) to all users\n", respText) return nil } diff --git a/cmd/manager.go b/cmd/manager.go index cdfe50907..3f1e22319 100644 --- a/cmd/manager.go +++ b/cmd/manager.go @@ -4,8 +4,6 @@ package cmd import ( - "fmt" - "net/http" "os" "time" @@ -103,57 +101,34 @@ func runShutdown(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) - statusCode, msg := private.Shutdown(ctx) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) - return nil + setup(ctx, c.Bool("debug")) + extra := private.Shutdown(ctx) + return handleCliResponseExtra(extra) } func runRestart(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) - statusCode, msg := private.Restart(ctx) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) - return nil + setup(ctx, c.Bool("debug")) + extra := private.Restart(ctx) + return handleCliResponseExtra(extra) } func runFlushQueues(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) - statusCode, msg := private.FlushQueues(ctx, c.Duration("timeout"), c.Bool("non-blocking")) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) - return nil + setup(ctx, c.Bool("debug")) + extra := private.FlushQueues(ctx, c.Duration("timeout"), c.Bool("non-blocking")) + return handleCliResponseExtra(extra) } func runProcesses(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) - statusCode, msg := private.Processes(ctx, os.Stdout, c.Bool("flat"), c.Bool("no-system"), c.Bool("stacktraces"), c.Bool("json"), c.String("cancel")) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - return nil + setup(ctx, c.Bool("debug")) + extra := private.Processes(ctx, os.Stdout, c.Bool("flat"), c.Bool("no-system"), c.Bool("stacktraces"), c.Bool("json"), c.String("cancel")) + return handleCliResponseExtra(extra) } diff --git a/cmd/manager_logging.go b/cmd/manager_logging.go index d49675ce8..914210d37 100644 --- a/cmd/manager_logging.go +++ b/cmd/manager_logging.go @@ -5,7 +5,6 @@ package cmd import ( "fmt" - "net/http" "os" "code.gitea.io/gitea/modules/log" @@ -191,27 +190,25 @@ var ( ) func runRemoveLogger(c *cli.Context) error { - setup("manager", c.Bool("debug")) + ctx, cancel := installSignals() + defer cancel() + + setup(ctx, c.Bool("debug")) group := c.String("group") if len(group) == 0 { group = log.DEFAULT } name := c.Args().First() - ctx, cancel := installSignals() - defer cancel() - statusCode, msg := private.RemoveLogger(ctx, group, name) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) - return nil + extra := private.RemoveLogger(ctx, group, name) + return handleCliResponseExtra(extra) } func runAddSMTPLogger(c *cli.Context) error { - setup("manager", c.Bool("debug")) + ctx, cancel := installSignals() + defer cancel() + + setup(ctx, c.Bool("debug")) vals := map[string]interface{}{} mode := "smtp" if c.IsSet("host") { @@ -242,7 +239,10 @@ func runAddSMTPLogger(c *cli.Context) error { } func runAddConnLogger(c *cli.Context) error { - setup("manager", c.Bool("debug")) + ctx, cancel := installSignals() + defer cancel() + + setup(ctx, c.Bool("debug")) vals := map[string]interface{}{} mode := "conn" vals["net"] = "tcp" @@ -269,7 +269,10 @@ func runAddConnLogger(c *cli.Context) error { } func runAddFileLogger(c *cli.Context) error { - setup("manager", c.Bool("debug")) + ctx, cancel := installSignals() + defer cancel() + + setup(ctx, c.Bool("debug")) vals := map[string]interface{}{} mode := "file" if c.IsSet("filename") { @@ -299,7 +302,10 @@ func runAddFileLogger(c *cli.Context) error { } func runAddConsoleLogger(c *cli.Context) error { - setup("manager", c.Bool("debug")) + ctx, cancel := installSignals() + defer cancel() + + setup(ctx, c.Bool("debug")) vals := map[string]interface{}{} mode := "console" if c.IsSet("stderr") && c.Bool("stderr") { @@ -338,28 +344,17 @@ func commonAddLogger(c *cli.Context, mode string, vals map[string]interface{}) e ctx, cancel := installSignals() defer cancel() - statusCode, msg := private.AddLogger(ctx, group, name, mode, vals) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) - return nil + extra := private.AddLogger(ctx, group, name, mode, vals) + return handleCliResponseExtra(extra) } func runPauseLogging(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) - statusCode, msg := private.PauseLogging(ctx) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) + setup(ctx, c.Bool("debug")) + userMsg := private.PauseLogging(ctx) + _, _ = fmt.Fprintln(os.Stdout, userMsg) return nil } @@ -367,14 +362,9 @@ func runResumeLogging(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) - statusCode, msg := private.ResumeLogging(ctx) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) + setup(ctx, c.Bool("debug")) + userMsg := private.ResumeLogging(ctx) + _, _ = fmt.Fprintln(os.Stdout, userMsg) return nil } @@ -382,28 +372,17 @@ func runReleaseReopenLogging(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) - statusCode, msg := private.ReleaseReopenLogging(ctx) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) + setup(ctx, c.Bool("debug")) + userMsg := private.ReleaseReopenLogging(ctx) + _, _ = fmt.Fprintln(os.Stdout, userMsg) return nil } func runSetLogSQL(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setup("manager", c.Bool("debug")) + setup(ctx, c.Bool("debug")) - statusCode, msg := private.SetLogSQL(ctx, !c.Bool("off")) - switch statusCode { - case http.StatusInternalServerError: - return fail("InternalServerError", msg) - } - - fmt.Fprintln(os.Stdout, msg) - return nil + extra := private.SetLogSQL(ctx, !c.Bool("off")) + return handleCliResponseExtra(extra) } diff --git a/cmd/restore_repo.go b/cmd/restore_repo.go index c7dff4196..887b59bba 100644 --- a/cmd/restore_repo.go +++ b/cmd/restore_repo.go @@ -4,11 +4,8 @@ package cmd import ( - "errors" - "net/http" "strings" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/private" "code.gitea.io/gitea/modules/setting" @@ -60,7 +57,7 @@ func runRestoreRepository(c *cli.Context) error { if s := c.String("units"); s != "" { units = strings.Split(s, ",") } - statusCode, errStr := private.RestoreRepo( + extra := private.RestoreRepo( ctx, c.String("repo_dir"), c.String("owner_name"), @@ -68,10 +65,5 @@ func runRestoreRepository(c *cli.Context) error { units, c.Bool("validation"), ) - if statusCode == http.StatusOK { - return nil - } - - log.Fatal("Failed to restore repository: %v", errStr) - return errors.New(errStr) + return handleCliResponseExtra(extra) } diff --git a/cmd/serv.go b/cmd/serv.go index d7510845a..72eb63707 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -7,7 +7,6 @@ package cmd import ( "context" "fmt" - "net/http" "net/url" "os" "os/exec" @@ -16,6 +15,7 @@ import ( "strconv" "strings" "time" + "unicode" asymkey_model "code.gitea.io/gitea/models/asymkey" git_model "code.gitea.io/gitea/models/git" @@ -55,7 +55,7 @@ var CmdServ = cli.Command{ }, } -func setup(logPath string, debug bool) { +func setup(ctx context.Context, debug bool) { _ = log.DelLogger("console") if debug { _ = log.NewLogger(1000, "console", "console", `{"level":"trace","stacktracelevel":"NONE","stderr":true}`) @@ -72,15 +72,15 @@ func setup(logPath string, debug bool) { // `[repository]` `ROOT` is a relative path and $GITEA_WORK_DIR isn't passed to the SSH connection. if _, err := os.Stat(setting.RepoRootPath); err != nil { if os.IsNotExist(err) { - _ = fail("Incorrect configuration, no repository directory.", "Directory `[repository].ROOT` %q was not found, please check if $GITEA_WORK_DIR is passed to the SSH connection or make `[repository].ROOT` an absolute value.", setting.RepoRootPath) + _ = fail(ctx, "Incorrect configuration, no repository directory.", "Directory `[repository].ROOT` %q was not found, please check if $GITEA_WORK_DIR is passed to the SSH connection or make `[repository].ROOT` an absolute value.", setting.RepoRootPath) } else { - _ = fail("Incorrect configuration, repository directory is inaccessible", "Directory `[repository].ROOT` %q is inaccessible. err: %v", setting.RepoRootPath, err) + _ = fail(ctx, "Incorrect configuration, repository directory is inaccessible", "Directory `[repository].ROOT` %q is inaccessible. err: %v", setting.RepoRootPath, err) } return } if err := git.InitSimple(context.Background()); err != nil { - _ = fail("Failed to init git", "Failed to init git, err: %v", err) + _ = fail(ctx, "Failed to init git", "Failed to init git, err: %v", err) } } @@ -94,32 +94,54 @@ var ( alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`) ) -func fail(userMessage, logMessage string, args ...interface{}) error { +// fail prints message to stdout, it's mainly used for git serv and git hook commands. +// The output will be passed to git client and shown to user. +func fail(ctx context.Context, userMessage, logMsgFmt string, args ...interface{}) error { + if userMessage == "" { + userMessage = "Internal Server Error (no specific error)" + } + // There appears to be a chance to cause a zombie process and failure to read the Exit status // if nothing is outputted on stdout. _, _ = fmt.Fprintln(os.Stdout, "") _, _ = fmt.Fprintln(os.Stderr, "Gitea:", userMessage) - if len(logMessage) > 0 { + if logMsgFmt != "" { + logMsg := fmt.Sprintf(logMsgFmt, args...) if !setting.IsProd { - _, _ = fmt.Fprintf(os.Stderr, logMessage+"\n", args...) + _, _ = fmt.Fprintln(os.Stderr, "Gitea:", logMsg) } - } - ctx, cancel := installSignals() - defer cancel() - - if len(logMessage) > 0 { - _ = private.SSHLog(ctx, true, fmt.Sprintf(logMessage+": ", args...)) + if userMessage != "" { + if unicode.IsPunct(rune(userMessage[len(userMessage)-1])) { + logMsg = userMessage + " " + logMsg + } else { + logMsg = userMessage + ". " + logMsg + } + } + _ = private.SSHLog(ctx, true, logMsg) } return cli.NewExitError("", 1) } +// handleCliResponseExtra handles the extra response from the cli sub-commands +// If there is a user message it will be printed to stdout +// If the command failed it will return an error (the error will be printed by cli framework) +func handleCliResponseExtra(extra private.ResponseExtra) error { + if extra.UserMsg != "" { + _, _ = fmt.Fprintln(os.Stdout, extra.UserMsg) + } + if extra.HasError() { + return cli.NewExitError(extra.Error, 1) + } + return nil +} + func runServ(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() // FIXME: This needs to internationalised - setup("serv.log", c.Bool("debug")) + setup(ctx, c.Bool("debug")) if setting.SSH.Disabled { println("Gitea: SSH has been disabled") @@ -135,18 +157,18 @@ func runServ(c *cli.Context) error { keys := strings.Split(c.Args()[0], "-") if len(keys) != 2 || keys[0] != "key" { - return fail("Key ID format error", "Invalid key argument: %s", c.Args()[0]) + return fail(ctx, "Key ID format error", "Invalid key argument: %s", c.Args()[0]) } keyID, err := strconv.ParseInt(keys[1], 10, 64) if err != nil { - return fail("Key ID format error", "Invalid key argument: %s", c.Args()[1]) + return fail(ctx, "Key ID parsing error", "Invalid key argument: %s", c.Args()[1]) } cmd := os.Getenv("SSH_ORIGINAL_COMMAND") if len(cmd) == 0 { key, user, err := private.ServNoCommand(ctx, keyID) if err != nil { - return fail("Internal error", "Failed to check provided key: %v", err) + return fail(ctx, "Key check failed", "Failed to check provided key: %v", err) } switch key.Type { case asymkey_model.KeyTypeDeploy: @@ -164,7 +186,7 @@ func runServ(c *cli.Context) error { words, err := shellquote.Split(cmd) if err != nil { - return fail("Error parsing arguments", "Failed to parse arguments: %v", err) + return fail(ctx, "Error parsing arguments", "Failed to parse arguments: %v", err) } if len(words) < 2 { @@ -175,7 +197,7 @@ func runServ(c *cli.Context) error { return nil } } - return fail("Too few arguments", "Too few arguments in cmd: %s", cmd) + return fail(ctx, "Too few arguments", "Too few arguments in cmd: %s", cmd) } verb := words[0] @@ -187,7 +209,7 @@ func runServ(c *cli.Context) error { var lfsVerb string if verb == lfsAuthenticateVerb { if !setting.LFS.StartServer { - return fail("Unknown git command", "LFS authentication request over SSH denied, LFS support is disabled") + return fail(ctx, "Unknown git command", "LFS authentication request over SSH denied, LFS support is disabled") } if len(words) > 2 { @@ -200,37 +222,37 @@ func runServ(c *cli.Context) error { rr := strings.SplitN(repoPath, "/", 2) if len(rr) != 2 { - return fail("Invalid repository path", "Invalid repository path: %v", repoPath) + return fail(ctx, "Invalid repository path", "Invalid repository path: %v", repoPath) } username := strings.ToLower(rr[0]) reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git")) if alphaDashDotPattern.MatchString(reponame) { - return fail("Invalid repo name", "Invalid repo name: %s", reponame) + return fail(ctx, "Invalid repo name", "Invalid repo name: %s", reponame) } if c.Bool("enable-pprof") { if err := os.MkdirAll(setting.PprofDataPath, os.ModePerm); err != nil { - return fail("Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err) + return fail(ctx, "Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err) } stopCPUProfiler, err := pprof.DumpCPUProfileForUsername(setting.PprofDataPath, username) if err != nil { - return fail("Internal Server Error", "Unable to start CPU profile: %v", err) + return fail(ctx, "Unable to start CPU profiler", "Unable to start CPU profile: %v", err) } defer func() { stopCPUProfiler() err := pprof.DumpMemProfileForUsername(setting.PprofDataPath, username) if err != nil { - _ = fail("Internal Server Error", "Unable to dump Mem Profile: %v", err) + _ = fail(ctx, "Unable to dump Mem profile", "Unable to dump Mem Profile: %v", err) } }() } requestedMode, has := allowedCommands[verb] if !has { - return fail("Unknown git command", "Unknown git command %s", verb) + return fail(ctx, "Unknown git command", "Unknown git command %s", verb) } if verb == lfsAuthenticateVerb { @@ -239,20 +261,13 @@ func runServ(c *cli.Context) error { } else if lfsVerb == "download" { requestedMode = perm.AccessModeRead } else { - return fail("Unknown LFS verb", "Unknown lfs verb %s", lfsVerb) + return fail(ctx, "Unknown LFS verb", "Unknown lfs verb %s", lfsVerb) } } - results, err := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb) - if err != nil { - if private.IsErrServCommand(err) { - errServCommand := err.(private.ErrServCommand) - if errServCommand.StatusCode != http.StatusInternalServerError { - return fail("Unauthorized", "%s", errServCommand.Error()) - } - return fail("Internal Server Error", "%s", errServCommand.Error()) - } - return fail("Internal Server Error", "%s", err.Error()) + results, extra := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb) + if extra.HasError() { + return fail(ctx, extra.UserMsg, "ServCommand failed: %s", extra.Error) } // LFS token authentication @@ -274,7 +289,7 @@ func runServ(c *cli.Context) error { // Sign and get the complete encoded token as a string using the secret tokenString, err := token.SignedString(setting.LFS.JWTSecretBytes) if err != nil { - return fail("Internal error", "Failed to sign JWT token: %v", err) + return fail(ctx, "Failed to sign JWT Token", "Failed to sign JWT token: %v", err) } tokenAuthentication := &git_model.LFSTokenResponse{ @@ -286,7 +301,7 @@ func runServ(c *cli.Context) error { enc := json.NewEncoder(os.Stdout) err = enc.Encode(tokenAuthentication) if err != nil { - return fail("Internal error", "Failed to encode LFS json response: %v", err) + return fail(ctx, "Failed to encode LFS json response", "Failed to encode LFS json response: %v", err) } return nil } @@ -332,13 +347,13 @@ func runServ(c *cli.Context) error { gitcmd.Env = append(gitcmd.Env, git.CommonCmdServEnvs()...) if err = gitcmd.Run(); err != nil { - return fail("Internal error", "Failed to execute git command: %v", err) + return fail(ctx, "Failed to execute git command", "Failed to execute git command: %v", err) } // Update user key activity. if results.KeyID > 0 { if err = private.UpdatePublicKeyInRepo(ctx, results.KeyID, results.RepoID); err != nil { - return fail("Internal error", "UpdatePublicKeyInRepo: %v", err) + return fail(ctx, "Failed to update public key", "UpdatePublicKeyInRepo: %v", err) } } diff --git a/modules/httplib/httplib.go b/modules/httplib/httplib.go index a1984400d..e904d77e1 100644 --- a/modules/httplib/httplib.go +++ b/modules/httplib/httplib.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "crypto/tls" + "fmt" "io" "net" "net/http" @@ -68,6 +69,11 @@ func (r *Request) SetTimeout(connectTimeout, readWriteTimeout time.Duration) *Re return r } +func (r *Request) SetReadWriteTimeout(readWriteTimeout time.Duration) *Request { + r.setting.ReadWriteTimeout = readWriteTimeout + return r +} + // SetTLSClientConfig sets tls connection configurations if visiting https url. func (r *Request) SetTLSClientConfig(config *tls.Config) *Request { r.setting.TLSClientConfig = config @@ -138,11 +144,11 @@ func (r *Request) getResponse() (*http.Response, error) { r.Body(paramBody) } - url, err := url.Parse(r.url) + var err error + r.req.URL, err = url.Parse(r.url) if err != nil { return nil, err } - r.req.URL = url trans := r.setting.Transport if trans == nil { @@ -194,3 +200,7 @@ func TimeoutDialer(cTimeout time.Duration) func(ctx context.Context, net, addr s return conn, nil } } + +func (r *Request) GoString() string { + return fmt.Sprintf("%s %s", r.req.Method, r.url) +} diff --git a/modules/private/hook.go b/modules/private/hook.go index 9533eaae5..0563e4d80 100644 --- a/modules/private/hook.go +++ b/modules/private/hook.go @@ -5,14 +5,11 @@ package private import ( "context" - "errors" "fmt" - "net/http" "net/url" "strconv" "time" - "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" ) @@ -99,126 +96,46 @@ type HookProcReceiveRefResult struct { } // HookPreReceive check whether the provided commits are allowed -func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (int, string) { - reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s", - url.PathEscape(ownerName), - url.PathEscape(repoName), - ) - req := newInternalRequest(ctx, reqURL, "POST") - req = req.Header("Content-Type", "application/json") - jsonBytes, _ := json.Marshal(opts) - req.Body(jsonBytes) - req.SetTimeout(60*time.Second, time.Duration(60+len(opts.OldCommitIDs))*time.Second) - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "" +func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) ResponseExtra { + reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName)) + req := newInternalRequest(ctx, reqURL, "POST", opts) + req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second) + _, extra := requestJSONResp(req, &responseText{}) + return extra } // HookPostReceive updates services and users -func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookPostReceiveResult, string) { - reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/post-receive/%s/%s", - url.PathEscape(ownerName), - url.PathEscape(repoName), - ) - - req := newInternalRequest(ctx, reqURL, "POST") - req = req.Header("Content-Type", "application/json") - req.SetTimeout(60*time.Second, time.Duration(60+len(opts.OldCommitIDs))*time.Second) - jsonBytes, _ := json.Marshal(opts) - req.Body(jsonBytes) - resp, err := req.Response() - if err != nil { - return nil, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, decodeJSONError(resp).Err - } - res := &HookPostReceiveResult{} - _ = json.NewDecoder(resp.Body).Decode(res) - - return res, "" +func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookPostReceiveResult, ResponseExtra) { + reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/post-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName)) + req := newInternalRequest(ctx, reqURL, "POST", opts) + req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second) + return requestJSONResp(req, &HookPostReceiveResult{}) } // HookProcReceive proc-receive hook -func HookProcReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookProcReceiveResult, error) { - reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/proc-receive/%s/%s", - url.PathEscape(ownerName), - url.PathEscape(repoName), - ) +func HookProcReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookProcReceiveResult, ResponseExtra) { + reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/proc-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName)) - req := newInternalRequest(ctx, reqURL, "POST") - req = req.Header("Content-Type", "application/json") - req.SetTimeout(60*time.Second, time.Duration(60+len(opts.OldCommitIDs))*time.Second) - jsonBytes, _ := json.Marshal(opts) - req.Body(jsonBytes) - resp, err := req.Response() - if err != nil { - return nil, fmt.Errorf("Unable to contact gitea: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, errors.New(decodeJSONError(resp).Err) - } - res := &HookProcReceiveResult{} - _ = json.NewDecoder(resp.Body).Decode(res) - - return res, nil + req := newInternalRequest(ctx, reqURL, "POST", opts) + req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second) + return requestJSONResp(req, &HookProcReceiveResult{}) } // SetDefaultBranch will set the default branch to the provided branch for the provided repository -func SetDefaultBranch(ctx context.Context, ownerName, repoName, branch string) error { +func SetDefaultBranch(ctx context.Context, ownerName, repoName, branch string) ResponseExtra { reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/set-default-branch/%s/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName), url.PathEscape(branch), ) req := newInternalRequest(ctx, reqURL, "POST") - req = req.Header("Content-Type", "application/json") - - req.SetTimeout(60*time.Second, 60*time.Second) - resp, err := req.Response() - if err != nil { - return fmt.Errorf("Unable to contact gitea: %w", err) - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("Error returned from gitea: %v", decodeJSONError(resp).Err) - } - return nil + return requestJSONUserMsg(req, "") } // SSHLog sends ssh error log response func SSHLog(ctx context.Context, isErr bool, msg string) error { reqURL := setting.LocalURL + "api/internal/ssh/log" - req := newInternalRequest(ctx, reqURL, "POST") - req = req.Header("Content-Type", "application/json") - - jsonBytes, _ := json.Marshal(&SSHLogOption{ - IsError: isErr, - Message: msg, - }) - req.Body(jsonBytes) - - req.SetTimeout(60*time.Second, 60*time.Second) - resp, err := req.Response() - if err != nil { - return fmt.Errorf("unable to contact gitea: %w", err) - } - - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("Error returned from gitea: %v", decodeJSONError(resp).Err) - } - return nil + req := newInternalRequest(ctx, reqURL, "POST", &SSHLogOption{IsError: isErr, Message: msg}) + _, extra := requestJSONResp(req, &responseText{}) + return extra.Error } diff --git a/modules/private/internal.go b/modules/private/internal.go index a8b62fdde..9c330a24a 100644 --- a/modules/private/internal.go +++ b/modules/private/internal.go @@ -11,6 +11,7 @@ import ( "net/http" "os" "strings" + "time" "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/json" @@ -19,29 +20,10 @@ import ( "code.gitea.io/gitea/modules/setting" ) -func newRequest(ctx context.Context, url, method, sourceIP string) *httplib.Request { - if setting.InternalToken == "" { - log.Fatal(`The INTERNAL_TOKEN setting is missing from the configuration file: %q. -Ensure you are running in the correct environment or set the correct configuration file with -c.`, setting.CustomConf) - } - return httplib.NewRequest(url, method). - SetContext(ctx). - Header("X-Real-IP", sourceIP). - Header("Authorization", fmt.Sprintf("Bearer %s", setting.InternalToken)) -} - -// Response internal request response +// Response is used for internal request response (for user message and error message) type Response struct { - Err string `json:"err"` -} - -func decodeJSONError(resp *http.Response) *Response { - var res Response - err := json.NewDecoder(resp.Body).Decode(&res) - if err != nil { - res.Err = err.Error() - } - return &res + Err string `json:"err,omitempty"` // server-side error log message, it won't be exposed to end users + UserMsg string `json:"user_msg,omitempty"` // meaningful error message for end users, it will be shown in git client's output. } func getClientIP() string { @@ -52,11 +34,21 @@ func getClientIP() string { return strings.Fields(sshConnEnv)[0] } -func newInternalRequest(ctx context.Context, url, method string) *httplib.Request { - req := newRequest(ctx, url, method, getClientIP()).SetTLSClientConfig(&tls.Config{ - InsecureSkipVerify: true, - ServerName: setting.Domain, - }) +func newInternalRequest(ctx context.Context, url, method string, body ...any) *httplib.Request { + if setting.InternalToken == "" { + log.Fatal(`The INTERNAL_TOKEN setting is missing from the configuration file: %q. +Ensure you are running in the correct environment or set the correct configuration file with -c.`, setting.CustomConf) + } + + req := httplib.NewRequest(url, method). + SetContext(ctx). + Header("X-Real-IP", getClientIP()). + Header("Authorization", fmt.Sprintf("Bearer %s", setting.InternalToken)). + SetTLSClientConfig(&tls.Config{ + InsecureSkipVerify: true, + ServerName: setting.Domain, + }) + if setting.Protocol == setting.HTTPUnix { req.SetTransport(&http.Transport{ DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { @@ -90,5 +82,15 @@ func newInternalRequest(ctx context.Context, url, method string) *httplib.Reques }, }) } + + if len(body) == 1 { + req.Header("Content-Type", "application/json") + jsonBytes, _ := json.Marshal(body[0]) + req.Body(jsonBytes) + } else if len(body) > 1 { + log.Fatal("Too many arguments for newInternalRequest") + } + + req.SetTimeout(10*time.Second, 60*time.Second) return req } diff --git a/modules/private/key.go b/modules/private/key.go index f09d6de2b..6f7cd8779 100644 --- a/modules/private/key.go +++ b/modules/private/key.go @@ -6,8 +6,6 @@ package private import ( "context" "fmt" - "io" - "net/http" "code.gitea.io/gitea/modules/setting" ) @@ -16,39 +14,18 @@ import ( func UpdatePublicKeyInRepo(ctx context.Context, keyID, repoID int64) error { // Ask for running deliver hook and test pull request tasks. reqURL := setting.LocalURL + fmt.Sprintf("api/internal/ssh/%d/update/%d", keyID, repoID) - resp, err := newInternalRequest(ctx, reqURL, "POST").Response() - if err != nil { - return err - } - - defer resp.Body.Close() - - // All 2XX status codes are accepted and others will return an error - if resp.StatusCode/100 != 2 { - return fmt.Errorf("Failed to update public key: %s", decodeJSONError(resp).Err) - } - return nil + req := newInternalRequest(ctx, reqURL, "POST") + _, extra := requestJSONResp(req, &responseText{}) + return extra.Error } // AuthorizedPublicKeyByContent searches content as prefix (leak e-mail part) // and returns public key found. -func AuthorizedPublicKeyByContent(ctx context.Context, content string) (string, error) { +func AuthorizedPublicKeyByContent(ctx context.Context, content string) (string, ResponseExtra) { // Ask for running deliver hook and test pull request tasks. reqURL := setting.LocalURL + "api/internal/ssh/authorized_keys" req := newInternalRequest(ctx, reqURL, "POST") req.Param("content", content) - resp, err := req.Response() - if err != nil { - return "", err - } - - defer resp.Body.Close() - - // All 2XX status codes are accepted and others will return an error - if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("Failed to update public key: %s", decodeJSONError(resp).Err) - } - bs, err := io.ReadAll(resp.Body) - - return string(bs), err + resp, extra := requestJSONResp(req, &responseText{}) + return resp.Text, extra } diff --git a/modules/private/mail.go b/modules/private/mail.go index 6eb7c2acd..82216b346 100644 --- a/modules/private/mail.go +++ b/modules/private/mail.go @@ -5,11 +5,7 @@ package private import ( "context" - "fmt" - "io" - "net/http" - "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" ) @@ -21,38 +17,18 @@ type Email struct { } // SendEmail calls the internal SendEmail function -// // It accepts a list of usernames. // If DB contains these users it will send the email to them. -// -// If to list == nil its supposed to send an email to every -// user present in DB -func SendEmail(ctx context.Context, subject, message string, to []string) (int, string) { +// If to list == nil, it's supposed to send emails to every user present in DB +func SendEmail(ctx context.Context, subject, message string, to []string) (string, ResponseExtra) { reqURL := setting.LocalURL + "api/internal/mail/send" - req := newInternalRequest(ctx, reqURL, "POST") - req = req.Header("Content-Type", "application/json") - jsonBytes, _ := json.Marshal(Email{ + req := newInternalRequest(ctx, reqURL, "POST", Email{ Subject: subject, Message: message, To: to, }) - req.Body(jsonBytes) - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Response body error: %v", err.Error()) - } - - users := fmt.Sprintf("%d", len(to)) - if len(to) == 0 { - users = "all" - } - - return http.StatusOK, fmt.Sprintf("Sent %s email(s) to %s users", body, users) + resp, extra := requestJSONResp(req, &responseText{}) + return resp.Text, extra } diff --git a/modules/private/manager.go b/modules/private/manager.go index bbf470cd7..5853db34e 100644 --- a/modules/private/manager.go +++ b/modules/private/manager.go @@ -12,44 +12,21 @@ import ( "strconv" "time" - "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" ) // Shutdown calls the internal shutdown function -func Shutdown(ctx context.Context) (int, string) { +func Shutdown(ctx context.Context) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/shutdown" - req := newInternalRequest(ctx, reqURL, "POST") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Shutting down" + return requestJSONUserMsg(req, "Shutting down") } // Restart calls the internal restart function -func Restart(ctx context.Context) (int, string) { +func Restart(ctx context.Context) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/restart" - req := newInternalRequest(ctx, reqURL, "POST") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Restarting" + return requestJSONUserMsg(req, "Restarting") } // FlushOptions represents the options for the flush call @@ -59,102 +36,41 @@ type FlushOptions struct { } // FlushQueues calls the internal flush-queues function -func FlushQueues(ctx context.Context, timeout time.Duration, nonBlocking bool) (int, string) { +func FlushQueues(ctx context.Context, timeout time.Duration, nonBlocking bool) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/flush-queues" - - req := newInternalRequest(ctx, reqURL, "POST") + req := newInternalRequest(ctx, reqURL, "POST", FlushOptions{Timeout: timeout, NonBlocking: nonBlocking}) if timeout > 0 { - req.SetTimeout(timeout+10*time.Second, timeout+10*time.Second) + req.SetReadWriteTimeout(timeout + 10*time.Second) } - req = req.Header("Content-Type", "application/json") - jsonBytes, _ := json.Marshal(FlushOptions{ - Timeout: timeout, - NonBlocking: nonBlocking, - }) - req.Body(jsonBytes) - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Flushed" + return requestJSONUserMsg(req, "Flushed") } // PauseLogging pauses logging -func PauseLogging(ctx context.Context) (int, string) { +func PauseLogging(ctx context.Context) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/pause-logging" - req := newInternalRequest(ctx, reqURL, "POST") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Logging Paused" + return requestJSONUserMsg(req, "Logging Paused") } // ResumeLogging resumes logging -func ResumeLogging(ctx context.Context) (int, string) { +func ResumeLogging(ctx context.Context) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/resume-logging" - req := newInternalRequest(ctx, reqURL, "POST") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Logging Restarted" + return requestJSONUserMsg(req, "Logging Restarted") } // ReleaseReopenLogging releases and reopens logging files -func ReleaseReopenLogging(ctx context.Context) (int, string) { +func ReleaseReopenLogging(ctx context.Context) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/release-and-reopen-logging" - req := newInternalRequest(ctx, reqURL, "POST") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Logging Restarted" + return requestJSONUserMsg(req, "Logging Restarted") } // SetLogSQL sets database logging -func SetLogSQL(ctx context.Context, on bool) (int, string) { +func SetLogSQL(ctx context.Context, on bool) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/set-log-sql?on=" + strconv.FormatBool(on) - req := newInternalRequest(ctx, reqURL, "POST") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Log SQL setting set" + return requestJSONUserMsg(req, "Log SQL setting set") } // LoggerOptions represents the options for the add logger call @@ -166,67 +82,32 @@ type LoggerOptions struct { } // AddLogger adds a logger -func AddLogger(ctx context.Context, group, name, mode string, config map[string]interface{}) (int, string) { +func AddLogger(ctx context.Context, group, name, mode string, config map[string]interface{}) ResponseExtra { reqURL := setting.LocalURL + "api/internal/manager/add-logger" - - req := newInternalRequest(ctx, reqURL, "POST") - req = req.Header("Content-Type", "application/json") - jsonBytes, _ := json.Marshal(LoggerOptions{ + req := newInternalRequest(ctx, reqURL, "POST", LoggerOptions{ Group: group, Name: name, Mode: mode, Config: config, }) - req.Body(jsonBytes) - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Added" + return requestJSONUserMsg(req, "Added") } // RemoveLogger removes a logger -func RemoveLogger(ctx context.Context, group, name string) (int, string) { +func RemoveLogger(ctx context.Context, group, name string) ResponseExtra { reqURL := setting.LocalURL + fmt.Sprintf("api/internal/manager/remove-logger/%s/%s", url.PathEscape(group), url.PathEscape(name)) - req := newInternalRequest(ctx, reqURL, "POST") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - return http.StatusOK, "Removed" + return requestJSONUserMsg(req, "Removed") } // Processes return the current processes from this gitea instance -func Processes(ctx context.Context, out io.Writer, flat, noSystem, stacktraces, json bool, cancel string) (int, string) { +func Processes(ctx context.Context, out io.Writer, flat, noSystem, stacktraces, json bool, cancel string) ResponseExtra { reqURL := setting.LocalURL + fmt.Sprintf("api/internal/manager/processes?flat=%t&no-system=%t&stacktraces=%t&json=%t&cancel-pid=%s", flat, noSystem, stacktraces, json, url.QueryEscape(cancel)) req := newInternalRequest(ctx, reqURL, "GET") - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v", err.Error()) + callback := func(resp *http.Response, extra *ResponseExtra) { + _, extra.Error = io.Copy(out, resp.Body) } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return resp.StatusCode, decodeJSONError(resp).Err - } - - _, err = io.Copy(out, resp.Body) - if err != nil { - return http.StatusInternalServerError, err.Error() - } - return http.StatusOK, "" + _, extra := requestJSONResp(req, &callback) + return extra } diff --git a/modules/private/request.go b/modules/private/request.go new file mode 100644 index 000000000..3eb8c92c1 --- /dev/null +++ b/modules/private/request.go @@ -0,0 +1,135 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package private + +import ( + "fmt" + "io" + "net/http" + "unicode" + + "code.gitea.io/gitea/modules/httplib" + "code.gitea.io/gitea/modules/json" +) + +// responseText is used to get the response as text, instead of parsing it as JSON. +type responseText struct { + Text string +} + +// ResponseExtra contains extra information about the response, especially for error responses. +type ResponseExtra struct { + StatusCode int + UserMsg string + Error error +} + +type responseCallback func(resp *http.Response, extra *ResponseExtra) + +func (re *ResponseExtra) HasError() bool { + return re.Error != nil +} + +type responseError struct { + statusCode int + errorString string +} + +func (re responseError) Error() string { + if re.errorString == "" { + return fmt.Sprintf("internal API error response, status=%d", re.statusCode) + } + return fmt.Sprintf("internal API error response, status=%d, err=%s", re.statusCode, re.errorString) +} + +// requestJSONUserMsg sends a request to the gitea server and then parses the response. +// If the status code is not 2xx, or any error occurs, the ResponseExtra.Error field is guaranteed to be non-nil, +// and the ResponseExtra.UserMsg field will be set to a message for the end user. +// +// * If the "res" is a struct pointer, the response will be parsed as JSON +// * If the "res" is responseText pointer, the response will be stored as text in it +// * If the "res" is responseCallback pointer, the callback function should set the ResponseExtra fields accordingly +func requestJSONResp[T any](req *httplib.Request, res *T) (ret *T, extra ResponseExtra) { + resp, err := req.Response() + if err != nil { + extra.UserMsg = "Internal Server Connection Error" + extra.Error = fmt.Errorf("unable to contact gitea %q: %w", req.GoString(), err) + return nil, extra + } + defer resp.Body.Close() + + extra.StatusCode = resp.StatusCode + + // if the status code is not 2xx, try to parse the error response + if resp.StatusCode/100 != 2 { + var respErr Response + if err := json.NewDecoder(resp.Body).Decode(&respErr); err != nil { + extra.UserMsg = "Internal Server Error Decoding Failed" + extra.Error = fmt.Errorf("unable to decode error response %q: %w", req.GoString(), err) + return nil, extra + } + extra.UserMsg = respErr.UserMsg + if extra.UserMsg == "" { + extra.UserMsg = "Internal Server Error (no message for end users)" + } + extra.Error = responseError{statusCode: resp.StatusCode, errorString: respErr.Err} + return res, extra + } + + // now, the StatusCode must be 2xx + var v any = res + if respText, ok := v.(*responseText); ok { + // get the whole response as a text string + bs, err := io.ReadAll(resp.Body) + if err != nil { + extra.UserMsg = "Internal Server Response Reading Failed" + extra.Error = fmt.Errorf("unable to read response %q: %w", req.GoString(), err) + return nil, extra + } + respText.Text = string(bs) + return res, extra + } else if callback, ok := v.(*responseCallback); ok { + // pass the response to callback, and let the callback update the ResponseExtra + extra.StatusCode = resp.StatusCode + (*callback)(resp, &extra) + return nil, extra + } else if err := json.NewDecoder(resp.Body).Decode(res); err != nil { + // decode the response into the given struct + extra.UserMsg = "Internal Server Response Decoding Failed" + extra.Error = fmt.Errorf("unable to decode response %q: %w", req.GoString(), err) + return nil, extra + } + + if respMsg, ok := v.(*Response); ok { + // if the "res" is Response structure, try to get the UserMsg from it and update the ResponseExtra + extra.UserMsg = respMsg.UserMsg + if respMsg.Err != "" { + // usually this shouldn't happen, because the StatusCode is 2xx, there should be no error. + // but we still handle the "err" response, in case some people return error messages by status code 200. + extra.Error = responseError{statusCode: resp.StatusCode, errorString: respMsg.Err} + } + } + + return res, extra +} + +// requestJSONUserMsg sends a request to the gitea server and then parses the response as private.Response +// If the request succeeds, the successMsg will be used as part of ResponseExtra.UserMsg. +func requestJSONUserMsg(req *httplib.Request, successMsg string) ResponseExtra { + resp, extra := requestJSONResp(req, &Response{}) + if extra.HasError() { + return extra + } + if resp.UserMsg == "" { + extra.UserMsg = successMsg // if UserMsg is empty, then use successMsg as userMsg + } else if successMsg != "" { + // else, now UserMsg is not empty, if successMsg is not empty, then append successMsg to UserMsg + if unicode.IsPunct(rune(extra.UserMsg[len(extra.UserMsg)-1])) { + extra.UserMsg = extra.UserMsg + " " + successMsg + } else { + extra.UserMsg = extra.UserMsg + ". " + successMsg + } + } + return extra +} diff --git a/modules/private/restore_repo.go b/modules/private/restore_repo.go index f40d914a7..34d0f5d48 100644 --- a/modules/private/restore_repo.go +++ b/modules/private/restore_repo.go @@ -6,11 +6,8 @@ package private import ( "context" "fmt" - "io" - "net/http" "time" - "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" ) @@ -24,39 +21,16 @@ type RestoreParams struct { } // RestoreRepo calls the internal RestoreRepo function -func RestoreRepo(ctx context.Context, repoDir, ownerName, repoName string, units []string, validation bool) (int, string) { +func RestoreRepo(ctx context.Context, repoDir, ownerName, repoName string, units []string, validation bool) ResponseExtra { reqURL := setting.LocalURL + "api/internal/restore_repo" - req := newInternalRequest(ctx, reqURL, "POST") - req.SetTimeout(3*time.Second, 0) // since the request will spend much time, don't timeout - req = req.Header("Content-Type", "application/json") - jsonBytes, _ := json.Marshal(RestoreParams{ + req := newInternalRequest(ctx, reqURL, "POST", RestoreParams{ RepoDir: repoDir, OwnerName: ownerName, RepoName: repoName, Units: units, Validation: validation, }) - req.Body(jsonBytes) - resp, err := req.Response() - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Unable to contact gitea: %v, could you confirm it's running?", err.Error()) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - ret := struct { - Err string `json:"err"` - }{} - body, err := io.ReadAll(resp.Body) - if err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Response body error: %v", err.Error()) - } - if err := json.Unmarshal(body, &ret); err != nil { - return http.StatusInternalServerError, fmt.Sprintf("Response body Unmarshal error: %v", err.Error()) - } - return http.StatusInternalServerError, ret.Err - } - - return http.StatusOK, fmt.Sprintf("Restore repo %s/%s successfully", ownerName, repoName) + req.SetTimeout(3*time.Second, 0) // since the request will spend much time, don't timeout + return requestJSONUserMsg(req, fmt.Sprintf("Restore repo %s/%s successfully", ownerName, repoName)) } diff --git a/modules/private/serv.go b/modules/private/serv.go index c176e1ddf..480a44695 100644 --- a/modules/private/serv.go +++ b/modules/private/serv.go @@ -6,13 +6,11 @@ package private import ( "context" "fmt" - "net/http" "net/url" asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/perm" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" ) @@ -24,20 +22,11 @@ type KeyAndOwner struct { // ServNoCommand returns information about the provided key func ServNoCommand(ctx context.Context, keyID int64) (*asymkey_model.PublicKey, *user_model.User, error) { - reqURL := setting.LocalURL + fmt.Sprintf("api/internal/serv/none/%d", - keyID) - resp, err := newInternalRequest(ctx, reqURL, "GET").Response() - if err != nil { - return nil, nil, err - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return nil, nil, fmt.Errorf("%s", decodeJSONError(resp).Err) - } - - var keyAndOwner KeyAndOwner - if err := json.NewDecoder(resp.Body).Decode(&keyAndOwner); err != nil { - return nil, nil, err + reqURL := setting.LocalURL + fmt.Sprintf("api/internal/serv/none/%d", keyID) + req := newInternalRequest(ctx, reqURL, "GET") + keyAndOwner, extra := requestJSONResp(req, &KeyAndOwner{}) + if extra.HasError() { + return nil, nil, extra.Error } return keyAndOwner.Key, keyAndOwner.Owner, nil } @@ -56,53 +45,19 @@ type ServCommandResults struct { RepoID int64 } -// ErrServCommand is an error returned from ServCommmand. -type ErrServCommand struct { - Results ServCommandResults - Err string - StatusCode int -} - -func (err ErrServCommand) Error() string { - return err.Err -} - -// IsErrServCommand checks if an error is a ErrServCommand. -func IsErrServCommand(err error) bool { - _, ok := err.(ErrServCommand) - return ok -} - // ServCommand preps for a serv call -func ServCommand(ctx context.Context, keyID int64, ownerName, repoName string, mode perm.AccessMode, verbs ...string) (*ServCommandResults, error) { +func ServCommand(ctx context.Context, keyID int64, ownerName, repoName string, mode perm.AccessMode, verbs ...string) (*ServCommandResults, ResponseExtra) { reqURL := setting.LocalURL + fmt.Sprintf("api/internal/serv/command/%d/%s/%s?mode=%d", keyID, url.PathEscape(ownerName), url.PathEscape(repoName), - mode) + mode, + ) for _, verb := range verbs { if verb != "" { reqURL += fmt.Sprintf("&verb=%s", url.QueryEscape(verb)) } } - - resp, err := newInternalRequest(ctx, reqURL, "GET").Response() - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - var errServCommand ErrServCommand - if err := json.NewDecoder(resp.Body).Decode(&errServCommand); err != nil { - return nil, err - } - errServCommand.StatusCode = resp.StatusCode - return nil, errServCommand - } - var results ServCommandResults - if err := json.NewDecoder(resp.Body).Decode(&results); err != nil { - return nil, err - } - return &results, nil + req := newInternalRequest(ctx, reqURL, "GET") + return requestJSONResp(req, &ServCommandResults{}) } diff --git a/routers/private/default_branch.go b/routers/private/default_branch.go index 268ebbe44..b15d6ba33 100644 --- a/routers/private/default_branch.go +++ b/routers/private/default_branch.go @@ -1,7 +1,6 @@ // Copyright 2021 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( @@ -14,19 +13,6 @@ import ( "code.gitea.io/gitea/modules/private" ) -// ________ _____ .__ __ -// \______ \ _____/ ____\____ __ __| |_/ |_ -// | | \_/ __ \ __\\__ \ | | \ |\ __\ -// | ` \ ___/| | / __ \| | / |_| | -// /_______ /\___ >__| (____ /____/|____/__| -// \/ \/ \/ -// __________ .__ -// \______ \____________ ____ ____ | |__ -// | | _/\_ __ \__ \ / \_/ ___\| | \ -// | | \ | | \// __ \| | \ \___| Y \ -// |______ / |__| (____ /___| /\___ >___| / -// \/ \/ \/ \/ \/ - // SetDefaultBranch updates the default branch func SetDefaultBranch(ctx *gitea_context.PrivateContext) { ownerName := ctx.Params(":owner") diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index 75de47bdc..cfe20be10 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -1,7 +1,6 @@ // Copyright 2021 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index c711fc947..63b4a8622 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -1,7 +1,6 @@ // Copyright 2019 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( @@ -69,8 +68,8 @@ func (ctx *preReceiveContext) AssertCanWriteCode() bool { if ctx.Written() { return false } - ctx.JSON(http.StatusForbidden, map[string]interface{}{ - "err": "User permission denied for writing.", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "User permission denied for writing.", }) return false } @@ -95,8 +94,8 @@ func (ctx *preReceiveContext) AssertCreatePullRequest() bool { if ctx.Written() { return false } - ctx.JSON(http.StatusForbidden, map[string]interface{}{ - "err": "User permission denied for creating pull-request.", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "User permission denied for creating pull-request.", }) return false } @@ -151,7 +150,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN if branchName == repo.DefaultBranch && newCommitID == git.EmptySHA { log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName), + UserMsg: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName), }) return } @@ -179,7 +178,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN if newCommitID == git.EmptySHA { log.Warn("Forbidden: Branch: %s in %-v is protected from deletion", branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from deletion", branchName), + UserMsg: fmt.Sprintf("branch %s is protected from deletion", branchName), }) return } @@ -196,7 +195,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN } else if len(output) > 0 { log.Warn("Forbidden: Branch: %s in %-v is protected from force push", branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from force push", branchName), + UserMsg: fmt.Sprintf("branch %s is protected from force push", branchName), }) return @@ -217,7 +216,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN unverifiedCommit := err.(*errUnverifiedCommit).sha log.Warn("Forbidden: Branch: %s in %-v is protected from unverified commit %s", branchName, repo, unverifiedCommit) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, unverifiedCommit), + UserMsg: fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, unverifiedCommit), }) return } @@ -272,7 +271,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN if changedProtectedfiles { log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath), + UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath), }) return } @@ -297,7 +296,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN // Or we're simply not able to push to this protected branch log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v", ctx.opts.UserID, branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), + UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), }) return } @@ -333,7 +332,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN if !allowedMerge { log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v and is not allowed to merge pr #%d", ctx.opts.UserID, branchName, repo, pr.Index) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), + UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s", branchName), }) return } @@ -347,7 +346,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN if changedProtectedfiles { log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath), + UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath), }) return } @@ -357,7 +356,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN if models.IsErrDisallowedToMerge(err) { log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error()) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error()), + UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error()), }) return } @@ -400,7 +399,7 @@ func preReceiveTag(ctx *preReceiveContext, oldCommitID, newCommitID, refFullName if !isAllowed { log.Warn("Forbidden: Tag %s in %-v is protected", tagName, ctx.Repo.Repository) ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Tag %s is protected", tagName), + UserMsg: fmt.Sprintf("Tag %s is protected", tagName), }) return } @@ -412,15 +411,15 @@ func preReceivePullRequest(ctx *preReceiveContext, oldCommitID, newCommitID, ref } if ctx.Repo.Repository.IsEmpty { - ctx.JSON(http.StatusForbidden, map[string]interface{}{ - "err": "Can't create pull request for an empty repository.", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "Can't create pull request for an empty repository.", }) return } if ctx.opts.IsWiki { - ctx.JSON(http.StatusForbidden, map[string]interface{}{ - "err": "Pull requests are not supported on the wiki.", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "Pull requests are not supported on the wiki.", }) return } @@ -443,7 +442,7 @@ func preReceivePullRequest(ctx *preReceiveContext, oldCommitID, newCommitID, ref if !baseBranchExist { ctx.JSON(http.StatusForbidden, private.Response{ - Err: fmt.Sprintf("Unexpected ref: %s", refFullName), + UserMsg: fmt.Sprintf("Unexpected ref: %s", refFullName), }) return } diff --git a/routers/private/hook_proc_receive.go b/routers/private/hook_proc_receive.go index 05921e6f5..557712077 100644 --- a/routers/private/hook_proc_receive.go +++ b/routers/private/hook_proc_receive.go @@ -1,7 +1,6 @@ // Copyright 2021 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( @@ -30,8 +29,8 @@ func HookProcReceive(ctx *gitea_context.PrivateContext) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) } else { log.Error(err.Error()) - ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ - "Err": err.Error(), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: err.Error(), }) } diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 8ccde4f3d..caf3874ec 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -1,7 +1,6 @@ // Copyright 2021 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( @@ -16,19 +15,6 @@ import ( "code.gitea.io/gitea/modules/log" ) -// _________ .__ __ -// \_ ___ \ ____ _____ _____ |__|/ |_ -// / \ \/ / _ \ / \ / \| \ __\ -// \ \___( <_> ) Y Y \ Y Y \ || | -// \______ /\____/|__|_| /__|_| /__||__| -// \/ \/ \/ -// ____ ____ .__ _____.__ __ .__ -// \ \ / /___________|__|/ ____\__| ____ _____ _/ |_|__| ____ ____ -// \ Y // __ \_ __ \ \ __\| |/ ___\\__ \\ __\ |/ _ \ / \ -// \ /\ ___/| | \/ || | | \ \___ / __ \| | | ( <_> ) | \ -// \___/ \___ >__| |__||__| |__|\___ >____ /__| |__|\____/|___| / -// \/ \/ \/ \/ -// // This file contains commit verification functions for refs passed across in hooks func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []string) error { diff --git a/routers/private/internal.go b/routers/private/internal.go index 306e4ffb0..4acede337 100644 --- a/routers/private/internal.go +++ b/routers/private/internal.go @@ -1,7 +1,7 @@ // Copyright 2017 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. +// Package private contains all internal routes. The package name "internal" isn't usable because Golang reserves it for disabling cross-package usage. package private import ( diff --git a/routers/private/internal_repo.go b/routers/private/internal_repo.go index bd8db0a18..5e7e82b03 100644 --- a/routers/private/internal_repo.go +++ b/routers/private/internal_repo.go @@ -1,7 +1,6 @@ // Copyright 2021 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( @@ -13,23 +12,10 @@ import ( gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/private" ) -// __________ -// \______ \ ____ ______ ____ -// | _// __ \\____ \ / _ \ -// | | \ ___/| |_> > <_> ) -// |____|_ /\___ > __/ \____/ -// \/ \/|__| -// _____ .__ __ -// / _ \ ______ _____|__| ____ ____ _____ ____ _____/ |_ -// / /_\ \ / ___// ___/ |/ ___\ / \ / \_/ __ \ / \ __\ -// / | \\___ \ \___ \| / /_/ > | \ Y Y \ ___/| | \ | -// \____|__ /____ >____ >__\___ /|___| /__|_| /\___ >___| /__| -// \/ \/ \/ /_____/ \/ \/ \/ \/ - -// This file contains common functions relating to setting the Repository for the -// internal routes +// This file contains common functions relating to setting the Repository for the internal routes // RepoAssignment assigns the repository and gitrepository to the private context func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc { @@ -45,8 +31,8 @@ func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc { gitRepo, err := git.OpenRepository(ctx, repo.RepoPath()) if err != nil { log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ - "Err": fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err), }) return nil } @@ -71,8 +57,8 @@ func loadRepository(ctx *gitea_context.PrivateContext, ownerName, repoName strin repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ownerName, repoName) if err != nil { log.Error("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ - "Err": fmt.Sprintf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err), }) return nil } diff --git a/routers/private/key.go b/routers/private/key.go index b536019dd..a13b4c12a 100644 --- a/routers/private/key.go +++ b/routers/private/key.go @@ -1,7 +1,6 @@ // Copyright 2018 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( diff --git a/routers/private/manager.go b/routers/private/manager.go index a56fe9d12..38ad83326 100644 --- a/routers/private/manager.go +++ b/routers/private/manager.go @@ -31,14 +31,14 @@ func FlushQueues(ctx *context.PrivateContext) { } }() ctx.JSON(http.StatusAccepted, private.Response{ - Err: "Flushing", + UserMsg: "Flushing", }) return } err := queue.GetManager().FlushAll(ctx, opts.Timeout) if err != nil { ctx.JSON(http.StatusRequestTimeout, private.Response{ - Err: fmt.Sprintf("%v", err), + UserMsg: fmt.Sprintf("%v", err), }) } ctx.PlainText(http.StatusOK, "success") diff --git a/routers/private/manager_windows.go b/routers/private/manager_windows.go index b5382c7d9..bd3c3c30d 100644 --- a/routers/private/manager_windows.go +++ b/routers/private/manager_windows.go @@ -16,7 +16,7 @@ import ( // Restart is not implemented for Windows based servers as they can't fork func Restart(ctx *context.PrivateContext) { ctx.JSON(http.StatusNotImplemented, private.Response{ - Err: "windows servers cannot be gracefully restarted - shutdown and restart manually", + UserMsg: "windows servers cannot be gracefully restarted - shutdown and restart manually", }) } diff --git a/routers/private/serv.go b/routers/private/serv.go index 23ac011cf..b1efc5880 100644 --- a/routers/private/serv.go +++ b/routers/private/serv.go @@ -1,7 +1,6 @@ // Copyright 2019 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -// Package private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead. package private import ( @@ -29,7 +28,7 @@ func ServNoCommand(ctx *context.PrivateContext) { keyID := ctx.ParamsInt64(":keyid") if keyID <= 0 { ctx.JSON(http.StatusBadRequest, private.Response{ - Err: fmt.Sprintf("Bad key id: %d", keyID), + UserMsg: fmt.Sprintf("Bad key id: %d", keyID), }) } results := private.KeyAndOwner{} @@ -38,7 +37,7 @@ func ServNoCommand(ctx *context.PrivateContext) { if err != nil { if asymkey_model.IsErrKeyNotExist(err) { ctx.JSON(http.StatusUnauthorized, private.Response{ - Err: fmt.Sprintf("Cannot find key: %d", keyID), + UserMsg: fmt.Sprintf("Cannot find key: %d", keyID), }) return } @@ -55,7 +54,7 @@ func ServNoCommand(ctx *context.PrivateContext) { if err != nil { if user_model.IsErrUserNotExist(err) { ctx.JSON(http.StatusUnauthorized, private.Response{ - Err: fmt.Sprintf("Cannot find owner with id: %d for key: %d", key.OwnerID, keyID), + UserMsg: fmt.Sprintf("Cannot find owner with id: %d for key: %d", key.OwnerID, keyID), }) return } @@ -67,7 +66,7 @@ func ServNoCommand(ctx *context.PrivateContext) { } if !user.IsActive || user.ProhibitLogin { ctx.JSON(http.StatusForbidden, private.Response{ - Err: "Your account is disabled.", + UserMsg: "Your account is disabled.", }) return } @@ -113,23 +112,20 @@ func ServCommand(ctx *context.PrivateContext) { if user_model.IsErrUserNotExist(err) { // User is fetching/cloning a non-existent repository log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr()) - ctx.JSON(http.StatusNotFound, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), + ctx.JSON(http.StatusNotFound, private.Response{ + UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), }) return } log.Error("Unable to get repository owner: %s/%s Error: %v", results.OwnerName, results.RepoName, err) - ctx.JSON(http.StatusForbidden, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err), + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err), }) return } if !owner.IsOrganization() && !owner.IsActive { - ctx.JSON(http.StatusForbidden, private.ErrServCommand{ - Results: results, - Err: "Repository cannot be accessed, you could retry it later", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "Repository cannot be accessed, you could retry it later", }) return } @@ -144,18 +140,16 @@ func ServCommand(ctx *context.PrivateContext) { if verb == "git-upload-pack" { // User is fetching/cloning a non-existent repository log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr()) - ctx.JSON(http.StatusNotFound, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), + ctx.JSON(http.StatusNotFound, private.Response{ + UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), }) return } } } else { log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err) - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err), }) return } @@ -167,26 +161,23 @@ func ServCommand(ctx *context.PrivateContext) { results.RepoID = repo.ID if repo.IsBeingCreated() { - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: "Repository is being created, you could retry after it finished", + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: "Repository is being created, you could retry after it finished", }) return } if repo.IsBroken() { - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: "Repository is in a broken state", + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: "Repository is in a broken state", }) return } // We can shortcut at this point if the repo is a mirror if mode > perm.AccessModeRead && repo.IsMirror { - ctx.JSON(http.StatusForbidden, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Mirror Repository %s/%s is read-only", results.OwnerName, results.RepoName), + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: fmt.Sprintf("Mirror Repository %s/%s is read-only", results.OwnerName, results.RepoName), }) return } @@ -196,16 +187,14 @@ func ServCommand(ctx *context.PrivateContext) { key, err := asymkey_model.GetPublicKeyByID(keyID) if err != nil { if asymkey_model.IsErrKeyNotExist(err) { - ctx.JSON(http.StatusNotFound, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Cannot find key: %d", keyID), + ctx.JSON(http.StatusNotFound, private.Response{ + UserMsg: fmt.Sprintf("Cannot find key: %d", keyID), }) return } log.Error("Unable to get public key: %d Error: %v", keyID, err) - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Unable to get key: %d Error: %v", keyID, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get key: %d Error: %v", keyID, err), }) return } @@ -215,9 +204,8 @@ func ServCommand(ctx *context.PrivateContext) { // If repo doesn't exist, deploy key doesn't make sense if !repoExist && key.Type == asymkey_model.KeyTypeDeploy { - ctx.JSON(http.StatusNotFound, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Cannot find repository %s/%s", results.OwnerName, results.RepoName), + ctx.JSON(http.StatusNotFound, private.Response{ + UserMsg: fmt.Sprintf("Cannot find repository %s/%s", results.OwnerName, results.RepoName), }) return } @@ -232,16 +220,14 @@ func ServCommand(ctx *context.PrivateContext) { deployKey, err = asymkey_model.GetDeployKeyByRepo(ctx, key.ID, repo.ID) if err != nil { if asymkey_model.IsErrDeployKeyNotExist(err) { - ctx.JSON(http.StatusNotFound, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Public (Deploy) Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName), + ctx.JSON(http.StatusNotFound, private.Response{ + UserMsg: fmt.Sprintf("Public (Deploy) Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName), }) return } log.Error("Unable to get deploy for public (deploy) key: %d in %-v Error: %v", key.ID, repo, err) - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Unable to get Deploy Key for Public Key: %d:%s in %s/%s.", key.ID, key.Name, results.OwnerName, results.RepoName), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get Deploy Key for Public Key: %d:%s in %s/%s.", key.ID, key.Name, results.OwnerName, results.RepoName), }) return } @@ -262,23 +248,21 @@ func ServCommand(ctx *context.PrivateContext) { user, err = user_model.GetUserByID(ctx, key.OwnerID) if err != nil { if user_model.IsErrUserNotExist(err) { - ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Public Key: %d:%s owner %d does not exist.", key.ID, key.Name, key.OwnerID), + ctx.JSON(http.StatusUnauthorized, private.Response{ + UserMsg: fmt.Sprintf("Public Key: %d:%s owner %d does not exist.", key.ID, key.Name, key.OwnerID), }) return } log.Error("Unable to get owner: %d for public key: %d:%s Error: %v", key.OwnerID, key.ID, key.Name, err) - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Unable to get Owner: %d for Deploy Key: %d:%s in %s/%s.", key.OwnerID, key.ID, key.Name, ownerName, repoName), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get Owner: %d for Deploy Key: %d:%s in %s/%s.", key.OwnerID, key.ID, key.Name, ownerName, repoName), }) return } if !user.IsActive || user.ProhibitLogin { ctx.JSON(http.StatusForbidden, private.Response{ - Err: "Your account is disabled.", + UserMsg: "Your account is disabled.", }) return } @@ -291,9 +275,8 @@ func ServCommand(ctx *context.PrivateContext) { // Don't allow pushing if the repo is archived if repoExist && mode > perm.AccessModeRead && repo.IsArchived { - ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Repo: %s/%s is archived.", results.OwnerName, results.RepoName), + ctx.JSON(http.StatusUnauthorized, private.Response{ + UserMsg: fmt.Sprintf("Repo: %s/%s is archived.", results.OwnerName, results.RepoName), }) return } @@ -307,9 +290,8 @@ func ServCommand(ctx *context.PrivateContext) { setting.Service.RequireSignInView) { if key.Type == asymkey_model.KeyTypeDeploy { if deployKey.Mode < mode { - ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Deploy Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName), + ctx.JSON(http.StatusUnauthorized, private.Response{ + UserMsg: fmt.Sprintf("Deploy Key: %d:%s is not authorized to %s %s/%s.", key.ID, key.Name, modeString, results.OwnerName, results.RepoName), }) return } @@ -322,9 +304,8 @@ func ServCommand(ctx *context.PrivateContext) { perm, err := access_model.GetUserRepoPermission(ctx, repo, user) if err != nil { log.Error("Unable to get permissions for %-v with key %d in %-v Error: %v", user, key.ID, repo, err) - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Unable to get permissions for user %d:%s with key %d in %s/%s Error: %v", user.ID, user.Name, key.ID, results.OwnerName, results.RepoName, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get permissions for user %d:%s with key %d in %s/%s Error: %v", user.ID, user.Name, key.ID, results.OwnerName, results.RepoName, err), }) return } @@ -333,9 +314,8 @@ func ServCommand(ctx *context.PrivateContext) { if userMode < mode { log.Warn("Failed authentication attempt for %s with key %s (not authorized to %s %s/%s) from %s", user.Name, key.Name, modeString, ownerName, repoName, ctx.RemoteAddr()) - ctx.JSON(http.StatusUnauthorized, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("User: %d:%s with Key: %d:%s is not authorized to %s %s/%s.", user.ID, user.Name, key.ID, key.Name, modeString, ownerName, repoName), + ctx.JSON(http.StatusUnauthorized, private.Response{ + UserMsg: fmt.Sprintf("User: %d:%s with Key: %d:%s is not authorized to %s %s/%s.", user.ID, user.Name, key.ID, key.Name, modeString, ownerName, repoName), }) return } @@ -346,24 +326,21 @@ func ServCommand(ctx *context.PrivateContext) { if !repoExist { owner, err := user_model.GetUserByName(ctx, ownerName) if err != nil { - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Unable to get owner: %s %v", results.OwnerName, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get owner: %s %v", results.OwnerName, err), }) return } if owner.IsOrganization() && !setting.Repository.EnablePushCreateOrg { - ctx.JSON(http.StatusForbidden, private.ErrServCommand{ - Results: results, - Err: "Push to create is not enabled for organizations.", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "Push to create is not enabled for organizations.", }) return } if !owner.IsOrganization() && !setting.Repository.EnablePushCreateUser { - ctx.JSON(http.StatusForbidden, private.ErrServCommand{ - Results: results, - Err: "Push to create is not enabled for users.", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "Push to create is not enabled for users.", }) return } @@ -371,9 +348,8 @@ func ServCommand(ctx *context.PrivateContext) { repo, err = repo_service.PushCreateRepo(ctx, user, owner, results.RepoName) if err != nil { log.Error("pushCreateRepo: %v", err) - ctx.JSON(http.StatusNotFound, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), + ctx.JSON(http.StatusNotFound, private.Response{ + UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), }) return } @@ -384,16 +360,14 @@ func ServCommand(ctx *context.PrivateContext) { // Ensure the wiki is enabled before we allow access to it if _, err := repo.GetUnit(ctx, unit.TypeWiki); err != nil { if repo_model.IsErrUnitTypeNotExist(err) { - ctx.JSON(http.StatusForbidden, private.ErrServCommand{ - Results: results, - Err: "repository wiki is disabled", + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: "repository wiki is disabled", }) return } log.Error("Failed to get the wiki unit in %-v Error: %v", repo, err) - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Failed to get the wiki unit in %s/%s Error: %v", ownerName, repoName, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Failed to get the wiki unit in %s/%s Error: %v", ownerName, repoName, err), }) return } @@ -401,9 +375,8 @@ func ServCommand(ctx *context.PrivateContext) { // Finally if we're trying to touch the wiki we should init it if err = wiki_service.InitWiki(ctx, repo); err != nil { log.Error("Failed to initialize the wiki in %-v Error: %v", repo, err) - ctx.JSON(http.StatusInternalServerError, private.ErrServCommand{ - Results: results, - Err: fmt.Sprintf("Failed to initialize the wiki in %s/%s Error: %v", ownerName, repoName, err), + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Failed to initialize the wiki in %s/%s Error: %v", ownerName, repoName, err), }) return } diff --git a/tests/integration/api_private_serv_test.go b/tests/integration/api_private_serv_test.go index d26935f44..8beec6238 100644 --- a/tests/integration/api_private_serv_test.go +++ b/tests/integration/api_private_serv_test.go @@ -43,8 +43,8 @@ func TestAPIPrivateServ(t *testing.T) { defer cancel() // Can push to a repo we own - results, err := private.ServCommand(ctx, 1, "user2", "repo1", perm.AccessModeWrite, "git-upload-pack", "") - assert.NoError(t, err) + results, extra := private.ServCommand(ctx, 1, "user2", "repo1", perm.AccessModeWrite, "git-upload-pack", "") + assert.NoError(t, extra.Error) assert.False(t, results.IsWiki) assert.Zero(t, results.DeployKeyID) assert.Equal(t, int64(1), results.KeyID) @@ -56,18 +56,18 @@ func TestAPIPrivateServ(t *testing.T) { assert.Equal(t, int64(1), results.RepoID) // Cannot push to a private repo we're not associated with - results, err = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "") - assert.Error(t, err) + results, extra = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "") + assert.Error(t, extra.Error) assert.Empty(t, results) // Cannot pull from a private repo we're not associated with - results, err = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "") - assert.Error(t, err) + results, extra = private.ServCommand(ctx, 1, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "") + assert.Error(t, extra.Error) assert.Empty(t, results) // Can pull from a public repo we're not associated with - results, err = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "") - assert.NoError(t, err) + results, extra = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "") + assert.NoError(t, extra.Error) assert.False(t, results.IsWiki) assert.Zero(t, results.DeployKeyID) assert.Equal(t, int64(1), results.KeyID) @@ -79,8 +79,8 @@ func TestAPIPrivateServ(t *testing.T) { assert.Equal(t, int64(17), results.RepoID) // Cannot push to a public repo we're not associated with - results, err = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeWrite, "git-upload-pack", "") - assert.Error(t, err) + results, extra = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeWrite, "git-upload-pack", "") + assert.Error(t, extra.Error) assert.Empty(t, results) // Add reading deploy key @@ -88,8 +88,8 @@ func TestAPIPrivateServ(t *testing.T) { assert.NoError(t, err) // Can pull from repo we're a deploy key for - results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "") - assert.NoError(t, err) + results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "") + assert.NoError(t, extra.Error) assert.False(t, results.IsWiki) assert.NotZero(t, results.DeployKeyID) assert.Equal(t, deployKey.KeyID, results.KeyID) @@ -101,18 +101,18 @@ func TestAPIPrivateServ(t *testing.T) { assert.Equal(t, int64(19), results.RepoID) // Cannot push to a private repo with reading key - results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "") - assert.Error(t, err) + results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "") + assert.Error(t, extra.Error) assert.Empty(t, results) // Cannot pull from a private repo we're not associated with - results, err = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "") - assert.Error(t, err) + results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "") + assert.Error(t, extra.Error) assert.Empty(t, results) // Cannot pull from a public repo we're not associated with - results, err = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "") - assert.Error(t, err) + results, extra = private.ServCommand(ctx, deployKey.ID, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "") + assert.Error(t, extra.Error) assert.Empty(t, results) // Add writing deploy key @@ -120,13 +120,13 @@ func TestAPIPrivateServ(t *testing.T) { assert.NoError(t, err) // Cannot push to a private repo with reading key - results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "") - assert.Error(t, err) + results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeWrite, "git-upload-pack", "") + assert.Error(t, extra.Error) assert.Empty(t, results) // Can pull from repo we're a writing deploy key for - results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "") - assert.NoError(t, err) + results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "") + assert.NoError(t, extra.Error) assert.False(t, results.IsWiki) assert.NotZero(t, results.DeployKeyID) assert.Equal(t, deployKey.KeyID, results.KeyID) @@ -138,8 +138,8 @@ func TestAPIPrivateServ(t *testing.T) { assert.Equal(t, int64(20), results.RepoID) // Can push to repo we're a writing deploy key for - results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeWrite, "git-upload-pack", "") - assert.NoError(t, err) + results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeWrite, "git-upload-pack", "") + assert.NoError(t, extra.Error) assert.False(t, results.IsWiki) assert.NotZero(t, results.DeployKeyID) assert.Equal(t, deployKey.KeyID, results.KeyID)