From 16a7d343d78807e39df124756e5d43a69a2203a3 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 27 Nov 2024 20:50:27 -0600 Subject: [PATCH 1/5] Validate OAuth Redirect URIs (#32643) This fixes a TODO in the code to validate the RedirectURIs when adding or editing an OAuth application in user settings. This also includes a refactor of the user settings tests to only create the DB once per top-level test to avoid reloading fixtures. --- modules/util/truncate.go | 2 + modules/validation/binding.go | 37 ++++- modules/validation/binding_test.go | 1 + modules/validation/validurllist_test.go | 157 ++++++++++++++++++++++ routers/web/user/setting/oauth2_common.go | 17 ++- services/forms/user_form.go | 2 +- tests/integration/user_settings_test.go | 117 ++++++++++++---- 7 files changed, 302 insertions(+), 31 deletions(-) create mode 100644 modules/validation/validurllist_test.go diff --git a/modules/util/truncate.go b/modules/util/truncate.go index 77b116eeff27f..f2edbdc67367b 100644 --- a/modules/util/truncate.go +++ b/modules/util/truncate.go @@ -41,6 +41,8 @@ func SplitStringAtByteN(input string, n int) (left, right string) { // SplitTrimSpace splits the string at given separator and trims leading and trailing space func SplitTrimSpace(input, sep string) []string { + // Trim initial leading & trailing space + input = strings.TrimSpace(input) // replace CRLF with LF input = strings.ReplaceAll(input, "\r\n", "\n") diff --git a/modules/validation/binding.go b/modules/validation/binding.go index cb0a5063e509b..75190e31e185b 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/util" "gitea.com/go-chi/binding" "github.com/gobwas/glob" @@ -31,6 +32,7 @@ const ( // AddBindingRules adds additional binding rules func AddBindingRules() { addGitRefNameBindingRule() + addValidURLListBindingRule() addValidURLBindingRule() addValidSiteURLBindingRule() addGlobPatternRule() @@ -44,7 +46,7 @@ func addGitRefNameBindingRule() { // Git refname validation rule binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "GitRefName") + return rule == "GitRefName" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { str := fmt.Sprintf("%v", val) @@ -58,11 +60,38 @@ func addGitRefNameBindingRule() { }) } +func addValidURLListBindingRule() { + // URL validation rule + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return rule == "ValidUrlList" + }, + IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { + str := fmt.Sprintf("%v", val) + if len(str) == 0 { + errs.Add([]string{name}, binding.ERR_URL, "Url") + return false, errs + } + + ok := true + urls := util.SplitTrimSpace(str, "\n") + for _, u := range urls { + if !IsValidURL(u) { + ok = false + errs.Add([]string{name}, binding.ERR_URL, u) + } + } + + return ok, errs + }, + }) +} + func addValidURLBindingRule() { // URL validation rule binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "ValidUrl") + return rule == "ValidUrl" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { str := fmt.Sprintf("%v", val) @@ -80,7 +109,7 @@ func addValidSiteURLBindingRule() { // URL validation rule binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "ValidSiteUrl") + return rule == "ValidSiteUrl" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { str := fmt.Sprintf("%v", val) @@ -171,7 +200,7 @@ func addUsernamePatternRule() { func addValidGroupTeamMapRule() { binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "ValidGroupTeamMap") + return rule == "ValidGroupTeamMap" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { _, err := auth.UnmarshalGroupTeamMapping(fmt.Sprintf("%v", val)) diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index 01ff4e34356a0..28d0f57b5c331 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -27,6 +27,7 @@ type ( TestForm struct { BranchName string `form:"BranchName" binding:"GitRefName"` URL string `form:"ValidUrl" binding:"ValidUrl"` + URLs string `form:"ValidUrls" binding:"ValidUrlList"` GlobPattern string `form:"GlobPattern" binding:"GlobPattern"` RegexPattern string `form:"RegexPattern" binding:"RegexPattern"` } diff --git a/modules/validation/validurllist_test.go b/modules/validation/validurllist_test.go new file mode 100644 index 0000000000000..c6f862a962b00 --- /dev/null +++ b/modules/validation/validurllist_test.go @@ -0,0 +1,157 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package validation + +import ( + "testing" + + "gitea.com/go-chi/binding" +) + +// This is a copy of all the URL tests cases, plus additional ones to +// account for multiple URLs +var urlListValidationTestCases = []validationTestCase{ + { + description: "Empty URL", + data: TestForm{ + URLs: "", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL without port", + data: TestForm{ + URLs: "http://test.lan/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with port", + data: TestForm{ + URLs: "http://test.lan:3000/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with IPv6 address without port", + data: TestForm{ + URLs: "http://[::1]/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with IPv6 address with port", + data: TestForm{ + URLs: "http://[::1]:3000/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Invalid URL", + data: TestForm{ + URLs: "http//test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http//test.lan/", + }, + }, + }, + { + description: "Invalid schema", + data: TestForm{ + URLs: "ftp://test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "ftp://test.lan/", + }, + }, + }, + { + description: "Invalid port", + data: TestForm{ + URLs: "http://test.lan:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://test.lan:3x4/", + }, + }, + }, + { + description: "Invalid port with IPv6 address", + data: TestForm{ + URLs: "http://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, + }, + { + description: "Multi URLs", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://test.local/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Multi URLs with newline", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://test.local/\n", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "List with invalid entry", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, + }, + { + description: "List with two invalid entries", + data: TestForm{ + URLs: "ftp://test.lan:3000/\nhttp://[::1]:3x4/\n", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "ftp://test.lan:3000/", + }, + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, + }, +} + +func Test_ValidURLListValidation(t *testing.T) { + AddBindingRules() + + for _, testCase := range urlListValidationTestCases { + t.Run(testCase.description, func(t *testing.T) { + performValidationTest(t, testCase) + }) + } +} diff --git a/routers/web/user/setting/oauth2_common.go b/routers/web/user/setting/oauth2_common.go index 6029d7bedb0a6..e93e9e19548b4 100644 --- a/routers/web/user/setting/oauth2_common.go +++ b/routers/web/user/setting/oauth2_common.go @@ -47,7 +47,6 @@ func (oa *OAuth2CommonHandlers) AddApp(ctx *context.Context) { return } - // TODO validate redirect URI app, err := auth.CreateOAuth2Application(ctx, auth.CreateOAuth2ApplicationOptions{ Name: form.Name, RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"), @@ -96,11 +95,25 @@ func (oa *OAuth2CommonHandlers) EditSave(ctx *context.Context) { form := web.GetForm(ctx).(*forms.EditOAuth2ApplicationForm) if ctx.HasError() { + app, err := auth.GetOAuth2ApplicationByID(ctx, ctx.PathParamInt64("id")) + if err != nil { + if auth.IsErrOAuthApplicationNotFound(err) { + ctx.NotFound("Application not found", err) + return + } + ctx.ServerError("GetOAuth2ApplicationByID", err) + return + } + if app.UID != oa.OwnerID { + ctx.NotFound("Application not found", nil) + return + } + ctx.Data["App"] = app + oa.renderEditPage(ctx) return } - // TODO validate redirect URI var err error if ctx.Data["App"], err = auth.UpdateOAuth2Application(ctx, auth.UpdateOAuth2ApplicationOptions{ ID: ctx.PathParamInt64("id"), diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 5b7a43642ab2f..ed79936add68e 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -366,7 +366,7 @@ func (f *NewAccessTokenForm) GetScope() (auth_model.AccessTokenScope, error) { // EditOAuth2ApplicationForm form for editing oauth2 applications type EditOAuth2ApplicationForm struct { Name string `binding:"Required;MaxSize(255)" form:"application_name"` - RedirectURIs string `binding:"Required" form:"redirect_uris"` + RedirectURIs string `binding:"Required;ValidUrlList" form:"redirect_uris"` ConfidentialClient bool `form:"confidential_client"` SkipSecondaryAuthorization bool `form:"skip_secondary_authorization"` } diff --git a/tests/integration/user_settings_test.go b/tests/integration/user_settings_test.go index 2103c92d58372..d8402eb25f180 100644 --- a/tests/integration/user_settings_test.go +++ b/tests/integration/user_settings_test.go @@ -10,6 +10,8 @@ import ( "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" ) // Validate that each navbar setting is correct. This checks that the @@ -51,8 +53,10 @@ func WithDisabledFeatures(t *testing.T, features ...string) { } func TestUserSettingsAccount(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("all features enabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() session := loginUser(t, "user2") req := NewRequest(t, "GET", "/user/settings/account") @@ -68,7 +72,7 @@ func TestUserSettingsAccount(t *testing.T) { }) t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -85,7 +89,7 @@ func TestUserSettingsAccount(t *testing.T) { }) t.Run("deletion disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureDeletion) @@ -102,7 +106,7 @@ func TestUserSettingsAccount(t *testing.T) { }) t.Run("deletion, credentials and email notifications are disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() mail := setting.Service.EnableNotifyMail setting.Service.EnableNotifyMail = false @@ -119,8 +123,10 @@ func TestUserSettingsAccount(t *testing.T) { } func TestUserSettingsUpdatePassword(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("enabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() session := loginUser(t, "user2") @@ -138,7 +144,7 @@ func TestUserSettingsUpdatePassword(t *testing.T) { }) t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -156,8 +162,10 @@ func TestUserSettingsUpdatePassword(t *testing.T) { } func TestUserSettingsUpdateEmail(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -175,8 +183,10 @@ func TestUserSettingsUpdateEmail(t *testing.T) { } func TestUserSettingsDeleteEmail(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -194,8 +204,10 @@ func TestUserSettingsDeleteEmail(t *testing.T) { } func TestUserSettingsDelete(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("deletion disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureDeletion) @@ -224,9 +236,10 @@ func TestUserSettingsAppearance(t *testing.T) { } func TestUserSettingsSecurity(t *testing.T) { - t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrepareTestEnv(t)() + t.Run("credentials disabled", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) session := loginUser(t, "user2") @@ -240,8 +253,7 @@ func TestUserSettingsSecurity(t *testing.T) { }) t.Run("mfa disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() - + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageMFA) session := loginUser(t, "user2") @@ -255,8 +267,7 @@ func TestUserSettingsSecurity(t *testing.T) { }) t.Run("credentials and mfa disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() - + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials, setting.UserFeatureManageMFA) session := loginUser(t, "user2") @@ -268,17 +279,75 @@ func TestUserSettingsSecurity(t *testing.T) { func TestUserSettingsApplications(t *testing.T) { defer tests.PrepareTestEnv(t)() - session := loginUser(t, "user2") - req := NewRequest(t, "GET", "/user/settings/applications") - resp := session.MakeRequest(t, req, http.StatusOK) - doc := NewHTMLParser(t, resp.Body) + t.Run("Applications", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - assertNavbar(t, doc) + session := loginUser(t, "user2") + req := NewRequest(t, "GET", "/user/settings/applications") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + assertNavbar(t, doc) + }) + + t.Run("OAuth2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + + t.Run("OAuth2ApplicationShow", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user/settings/applications/oauth2/2") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + assertNavbar(t, doc) + }) + + t.Run("OAuthApplicationsEdit", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user/settings/applications/oauth2/2") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + t.Run("Invalid URL", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user/settings/applications/oauth2/2", map[string]string{ + "_csrf": doc.GetCSRF(), + "application_name": "Test native app", + "redirect_uris": "ftp://127.0.0.1", + "confidential_client": "false", + }) + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + msg := doc.Find(".flash-error p").Text() + assert.Equal(t, `form.RedirectURIs"ftp://127.0.0.1" is not a valid URL.`, msg) + }) + + t.Run("OK", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user/settings/applications/oauth2/2", map[string]string{ + "_csrf": doc.GetCSRF(), + "application_name": "Test native app", + "redirect_uris": "http://127.0.0.1", + "confidential_client": "false", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + }) + }) + }) } func TestUserSettingsKeys(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("all enabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() session := loginUser(t, "user2") req := NewRequest(t, "GET", "/user/settings/keys") @@ -292,7 +361,7 @@ func TestUserSettingsKeys(t *testing.T) { }) t.Run("ssh keys disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageSSHKeys) @@ -308,7 +377,7 @@ func TestUserSettingsKeys(t *testing.T) { }) t.Run("gpg keys disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageGPGKeys) @@ -324,7 +393,7 @@ func TestUserSettingsKeys(t *testing.T) { }) t.Run("ssh & gpg keys disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageSSHKeys, setting.UserFeatureManageGPGKeys) From 1b296ed1a422b8f29ea0c0e887f6470b8895116c Mon Sep 17 00:00:00 2001 From: Pedro Nishiyama Date: Thu, 28 Nov 2024 04:18:23 -0300 Subject: [PATCH 2/5] Allow users with write permission to run actions (#32644) --- I have a use case where I need a team to be able to run actions without admin access. --- routers/web/repo/actions/actions.go | 4 ++-- routers/web/web.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index f5fb056494d7f..ad16b9fb4e4d5 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -168,8 +168,8 @@ func List(ctx *context.Context) { actionsConfig := ctx.Repo.Repository.MustGetUnit(ctx, unit.TypeActions).ActionsConfig() ctx.Data["ActionsConfig"] = actionsConfig - if len(workflowID) > 0 && ctx.Repo.IsAdmin() { - ctx.Data["AllowDisableOrEnableWorkflow"] = true + if len(workflowID) > 0 && ctx.Repo.CanWrite(unit.TypeActions) { + ctx.Data["AllowDisableOrEnableWorkflow"] = ctx.Repo.IsAdmin() isWorkflowDisabled := actionsConfig.IsWorkflowDisabled(workflowID) ctx.Data["CurWorkflowDisabled"] = isWorkflowDisabled diff --git a/routers/web/web.go b/routers/web/web.go index a2c14993ac394..5ed046a9838b1 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1406,7 +1406,7 @@ func registerRoutes(m *web.Router) { m.Get("", actions.List) m.Post("/disable", reqRepoAdmin, actions.DisableWorkflowFile) m.Post("/enable", reqRepoAdmin, actions.EnableWorkflowFile) - m.Post("/run", reqRepoAdmin, actions.Run) + m.Post("/run", reqRepoActionsWriter, actions.Run) m.Group("/runs/{run}", func() { m.Combo(""). From 00f8090de4b42305f6e717dc2cdb32c14b4a8fe2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 27 Nov 2024 23:43:38 -0800 Subject: [PATCH 3/5] Don't create action when syncing mirror pull refs (#32659) Fix #27961 --- services/feed/action.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/feed/action.go b/services/feed/action.go index 83daaa1438fd5..a8820aeb777a7 100644 --- a/services/feed/action.go +++ b/services/feed/action.go @@ -390,6 +390,12 @@ func (a *actionNotifier) DeleteRef(ctx context.Context, doer *user_model.User, r } func (a *actionNotifier) SyncPushCommits(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) { + // ignore pull sync message for pull requests refs + // TODO: it's better to have a UI to let users chose + if opts.RefFullName.IsPull() { + return + } + data, err := json.Marshal(commits) if err != nil { log.Error("json.Marshal: %v", err) From a1f56f83bff56f86180e59742efd3748908b82c1 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 28 Nov 2024 13:25:21 +0100 Subject: [PATCH 4/5] Improve diff file tree (#32658) - Unfolded directories now show a "open" icon - Prevent accidential text selection while toggling directories - Increase vertical item padding from 3px to 6px image --- web_src/js/components/DiffFileTreeItem.vue | 8 ++++++-- web_src/js/svg.ts | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/web_src/js/components/DiffFileTreeItem.vue b/web_src/js/components/DiffFileTreeItem.vue index 84431ff372dd5..12cafd8f1b5d5 100644 --- a/web_src/js/components/DiffFileTreeItem.vue +++ b/web_src/js/components/DiffFileTreeItem.vue @@ -51,7 +51,7 @@ function getIconForDiffType(pType) {
- + {{ item.name }}
@@ -87,12 +87,16 @@ a, a:hover { color: var(--color-text-light-3); } +.item-directory { + user-select: none; +} + .item-file, .item-directory { display: flex; align-items: center; gap: 0.25em; - padding: 3px 6px; + padding: 6px; } .item-file:hover, diff --git a/web_src/js/svg.ts b/web_src/js/svg.ts index cbb1af4ba16fc..3a0f2ed53c71c 100644 --- a/web_src/js/svg.ts +++ b/web_src/js/svg.ts @@ -27,6 +27,7 @@ import octiconDownload from '../../public/assets/img/svg/octicon-download.svg'; import octiconEye from '../../public/assets/img/svg/octicon-eye.svg'; import octiconFile from '../../public/assets/img/svg/octicon-file.svg'; import octiconFileDirectoryFill from '../../public/assets/img/svg/octicon-file-directory-fill.svg'; +import octiconFileDirectoryOpenFill from '../../public/assets/img/svg/octicon-file-directory-open-fill.svg'; import octiconFilter from '../../public/assets/img/svg/octicon-filter.svg'; import octiconGear from '../../public/assets/img/svg/octicon-gear.svg'; import octiconGitBranch from '../../public/assets/img/svg/octicon-git-branch.svg'; @@ -101,6 +102,7 @@ const svgs = { 'octicon-eye': octiconEye, 'octicon-file': octiconFile, 'octicon-file-directory-fill': octiconFileDirectoryFill, + 'octicon-file-directory-open-fill': octiconFileDirectoryOpenFill, 'octicon-filter': octiconFilter, 'octicon-gear': octiconGear, 'octicon-git-branch': octiconGitBranch, From 93640993e3cca9d0e0261591ba38b84b0b99ab12 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 29 Nov 2024 16:08:29 +0800 Subject: [PATCH 5/5] Refactor render system (orgmode) (#32671) Close #29100 --- models/renderhelper/repo_file_test.go | 39 +++++++++++++ modules/markup/orgmode/orgmode.go | 80 ++++++++++++-------------- modules/markup/orgmode/orgmode_test.go | 16 +++--- 3 files changed, 85 insertions(+), 50 deletions(-) diff --git a/models/renderhelper/repo_file_test.go b/models/renderhelper/repo_file_test.go index 40027ec76fd39..959648b660b81 100644 --- a/models/renderhelper/repo_file_test.go +++ b/models/renderhelper/repo_file_test.go @@ -12,6 +12,8 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" + _ "code.gitea.io/gitea/modules/markup/orgmode" + "github.com/stretchr/testify/assert" ) @@ -81,3 +83,40 @@ func TestRepoFile(t *testing.T) { `, rendered) }) } + +func TestRepoFileOrgMode(t *testing.T) { + unittest.PrepareTestEnv(t) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + t.Run("Links", func(t *testing.T) { + rctx := NewRenderContextRepoFile(context.Background(), repo1, RepoFileOptions{ + CurrentRefPath: "/commit/1234", + CurrentTreePath: "my-dir", + }).WithRelativePath("my-dir/a.org") + + rendered, err := markup.RenderString(rctx, ` +[[https://google.com/]] +[[ImageLink.svg][The Image Desc]] +`) + assert.NoError(t, err) + assert.Equal(t, `

+https://google.com/ +The Image Desc

+`, rendered) + }) + + t.Run("CodeHighlight", func(t *testing.T) { + rctx := NewRenderContextRepoFile(context.Background(), repo1, RepoFileOptions{}).WithRelativePath("my-dir/a.org") + + rendered, err := markup.RenderString(rctx, ` +#+begin_src c +int a = 1; +#+end_src +`) + assert.NoError(t, err) + assert.Equal(t, `
+
int a = 1;
+
+`, rendered) + }) +} diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index 31257351ae823..c6cc3340000f6 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -6,10 +6,12 @@ package markup import ( "fmt" "html" + "html/template" "io" "strings" "code.gitea.io/gitea/modules/highlight" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" @@ -20,33 +22,36 @@ import ( ) func init() { - markup.RegisterRenderer(Renderer{}) + markup.RegisterRenderer(renderer{}) } // Renderer implements markup.Renderer for orgmode -type Renderer struct{} +type renderer struct{} -var _ markup.PostProcessRenderer = (*Renderer)(nil) +var ( + _ markup.Renderer = (*renderer)(nil) + _ markup.PostProcessRenderer = (*renderer)(nil) +) // Name implements markup.Renderer -func (Renderer) Name() string { +func (renderer) Name() string { return "orgmode" } // NeedPostProcess implements markup.PostProcessRenderer -func (Renderer) NeedPostProcess() bool { return true } +func (renderer) NeedPostProcess() bool { return true } // Extensions implements markup.Renderer -func (Renderer) Extensions() []string { +func (renderer) Extensions() []string { return []string{".org"} } // SanitizerRules implements markup.Renderer -func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { +func (renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{} } -// Render renders orgmode rawbytes to HTML +// Render renders orgmode raw bytes to HTML func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { htmlWriter := org.NewHTMLWriter() htmlWriter.HighlightCodeBlock = func(source, lang string, inline bool, params map[string]string) string { @@ -56,10 +61,7 @@ func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error panic(err) } }() - var w strings.Builder - if _, err := w.WriteString(`
`); err != nil {
-			return ""
-		}
+		w := &strings.Builder{}
 
 		lexer := lexers.Get(lang)
 		if lexer == nil && lang == "" {
@@ -70,26 +72,20 @@ func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error
 			lang = strings.ToLower(lexer.Config().Name)
 		}
 
+		// include language-x class as part of commonmark spec
+		if err := ctx.RenderInternal.FormatWithSafeAttrs(w, `
`, lang); err != nil {
+			return ""
+		}
 		if lexer == nil {
-			// include language-x class as part of commonmark spec
-			if _, err := w.WriteString(``); err != nil {
-				return ""
-			}
 			if _, err := w.WriteString(html.EscapeString(source)); err != nil {
 				return ""
 			}
 		} else {
-			// include language-x class as part of commonmark spec
-			if _, err := w.WriteString(``); err != nil {
-				return ""
-			}
 			lexer = chroma.Coalesce(lexer)
-
 			if _, err := w.WriteString(string(highlight.CodeFromLexer(lexer, source))); err != nil {
 				return ""
 			}
 		}
-
 		if _, err := w.WriteString("
"); err != nil {
 			return ""
 		}
@@ -97,11 +93,7 @@ func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error
 		return w.String()
 	}
 
-	w := &Writer{
-		HTMLWriter: htmlWriter,
-		Ctx:        ctx,
-	}
-
+	w := &orgWriter{rctx: ctx, HTMLWriter: htmlWriter}
 	htmlWriter.ExtendingWriter = w
 
 	res, err := org.New().Silent().Parse(input, "").Write(w)
@@ -122,17 +114,18 @@ func RenderString(ctx *markup.RenderContext, content string) (string, error) {
 }
 
 // Render renders orgmode string to HTML string
-func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
+func (renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
 	return Render(ctx, input, output)
 }
 
-// Writer implements org.Writer
-type Writer struct {
+type orgWriter struct {
 	*org.HTMLWriter
-	Ctx *markup.RenderContext
+	rctx *markup.RenderContext
 }
 
-func (r *Writer) resolveLink(kind, link string) string {
+var _ org.Writer = (*orgWriter)(nil)
+
+func (r *orgWriter) resolveLink(kind, link string) string {
 	link = strings.TrimPrefix(link, "file:")
 	if !strings.HasPrefix(link, "#") && // not a URL fragment
 		!markup.IsFullURLString(link) {
@@ -142,39 +135,42 @@ func (r *Writer) resolveLink(kind, link string) string {
 			kind = org.RegularLink{URL: link}.Kind()
 		}
 		if kind == "image" || kind == "video" {
-			link = r.Ctx.RenderHelper.ResolveLink(link, markup.LinkTypeMedia)
+			link = r.rctx.RenderHelper.ResolveLink(link, markup.LinkTypeMedia)
 		} else {
-			link = r.Ctx.RenderHelper.ResolveLink(link, markup.LinkTypeDefault)
+			link = r.rctx.RenderHelper.ResolveLink(link, markup.LinkTypeDefault)
 		}
 	}
 	return link
 }
 
 // WriteRegularLink renders images, links or videos
-func (r *Writer) WriteRegularLink(l org.RegularLink) {
+func (r *orgWriter) WriteRegularLink(l org.RegularLink) {
 	link := r.resolveLink(l.Kind(), l.URL)
 
+	printHTML := func(html string, a ...any) {
+		_, _ = fmt.Fprint(r, htmlutil.HTMLFormat(html, a...))
+	}
 	// Inspired by https://github.com/niklasfasching/go-org/blob/6eb20dbda93cb88c3503f7508dc78cbbc639378f/org/html_writer.go#L406-L427
 	switch l.Kind() {
 	case "image":
 		if l.Description == nil {
-			_, _ = fmt.Fprintf(r, `%s`, link, link)
+			printHTML(`%s`, link, link)
 		} else {
 			imageSrc := r.resolveLink(l.Kind(), org.String(l.Description...))
-			_, _ = fmt.Fprintf(r, `%s`, link, imageSrc, imageSrc)
+			printHTML(`%s`, link, imageSrc, imageSrc)
 		}
 	case "video":
 		if l.Description == nil {
-			_, _ = fmt.Fprintf(r, ``, link, link)
+			printHTML(``, link, link)
 		} else {
 			videoSrc := r.resolveLink(l.Kind(), org.String(l.Description...))
-			_, _ = fmt.Fprintf(r, ``, link, videoSrc, videoSrc)
+			printHTML(``, link, videoSrc, videoSrc)
 		}
 	default:
-		description := link
+		var description any = link
 		if l.Description != nil {
-			description = r.WriteNodesAsString(l.Description...)
+			description = template.HTML(r.WriteNodesAsString(l.Description...)) // orgmode HTMLWriter outputs HTML content
 		}
-		_, _ = fmt.Fprintf(r, `%s`, link, description)
+		printHTML(`%s`, link, description)
 	}
 }
diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go
index d30df3b1886f5..e3cc05b4f022c 100644
--- a/modules/markup/orgmode/orgmode_test.go
+++ b/modules/markup/orgmode/orgmode_test.go
@@ -58,15 +58,15 @@ func TestRender_Media(t *testing.T) {
 	}
 
 	test("[[file:../../.images/src/02/train.jpg]]",
-		`
.images/src/02/train.jpg
`)
+		`
.images/src/02/train.jpg
`)
 	test("[[file:train.jpg]]",
-		`
relative-path/train.jpg
`)
+		`
relative-path/train.jpg
`)
 
 	// With description.
 	test("[[https://example.com][https://example.com/example.svg]]",
-		`
https://example.com/example.svg
`)
+		`
https://example.com/example.svg
`)
 	test("[[https://example.com][pre https://example.com/example.svg post]]",
-		`
pre https://example.com/example.svg post
`)
+		`
pre https://example.com/example.svg post
`)
 	test("[[https://example.com][https://example.com/example.mp4]]",
 		`
`)
 	test("[[https://example.com][pre https://example.com/example.mp4 post]]",
@@ -74,19 +74,19 @@ func TestRender_Media(t *testing.T) {
 
 	// Without description.
 	test("[[https://example.com/example.svg]]",
-		`
https://example.com/example.svg
`)
+		`
https://example.com/example.svg
`)
 	test("[[https://example.com/example.mp4]]",
 		`
`)
 
 	// test [[LINK][DESCRIPTION]] syntax with "file:" prefix
 	test(`[[https://example.com/][file:https://example.com/foo%20bar.svg]]`,
-		`
https://example.com/foo%20bar.svg
`)
+		`
https://example.com/foo%20bar.svg
`)
 	test(`[[file:https://example.com/foo%20bar.svg][Goto Image]]`,
 		`
Goto Image
`)
 	test(`[[file:https://example.com/link][https://example.com/image.jpg]]`,
-		`
https://example.com/image.jpg
`)
+		`
https://example.com/image.jpg
`)
 	test(`[[file:https://example.com/link][file:https://example.com/image.jpg]]`,
-		`
https://example.com/image.jpg
`)
+		`
https://example.com/image.jpg
`)
 }
 
 func TestRender_Source(t *testing.T) {