From 6f1de0a9e5a58b63d0c64b1182b7686c742a97c1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Nov 2024 08:55:50 +0800 Subject: [PATCH 1/9] Add avif image file support (#32508) Most modern browsers support it now ` Update ALLOWED_TYPES #96 ` https://gitea.com/gitea/docs/pulls/96 --------- Co-authored-by: silverwind --- custom/conf/app.example.ini | 2 +- modules/httplib/serve.go | 4 +-- modules/setting/attachment.go | 26 ++++++++-------- modules/typesniffer/typesniffer.go | 40 ++++++++++++++++++++----- modules/typesniffer/typesniffer_test.go | 30 +++++++++++++++++++ web_src/js/utils.test.ts | 2 +- web_src/js/utils.ts | 2 +- 7 files changed, 81 insertions(+), 25 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 7d5b3961bc..ef5684237d 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1912,7 +1912,7 @@ LEVEL = Info ;ENABLED = true ;; ;; Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types. -;ALLOWED_TYPES = .csv,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.xls,.xlsx,.zip +;ALLOWED_TYPES = .avif,.cpuprofile,.csv,.dmp,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.json,.jsonc,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.webp,.xls,.xlsx,.zip ;; ;; Max size of each file. Defaults to 2048MB ;MAX_SIZE = 2048 diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index 2e3e6a7c42..8fb667876e 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -46,7 +46,7 @@ func ServeSetHeaders(w http.ResponseWriter, opts *ServeHeaderOptions) { w.Header().Add(gzhttp.HeaderNoCompression, "1") } - contentType := typesniffer.ApplicationOctetStream + contentType := typesniffer.MimeTypeApplicationOctetStream if opts.ContentType != "" { if opts.ContentTypeCharset != "" { contentType = opts.ContentType + "; charset=" + strings.ToLower(opts.ContentTypeCharset) @@ -107,7 +107,7 @@ func setServeHeadersByFile(r *http.Request, w http.ResponseWriter, filePath stri } else if isPlain { opts.ContentType = "text/plain" } else { - opts.ContentType = typesniffer.ApplicationOctetStream + opts.ContentType = typesniffer.MimeTypeApplicationOctetStream } } diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index 0fdabb5032..c11b0c478a 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -3,33 +3,33 @@ package setting -// Attachment settings -var Attachment = struct { +type AttachmentSettingType struct { Storage *Storage AllowedTypes string MaxSize int64 MaxFiles int Enabled bool -}{ - Storage: &Storage{}, - AllowedTypes: ".cpuprofile,.csv,.dmp,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.json,.jsonc,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.xls,.xlsx,.zip", - MaxSize: 2048, - MaxFiles: 5, - Enabled: true, } +var Attachment AttachmentSettingType + func loadAttachmentFrom(rootCfg ConfigProvider) (err error) { + Attachment = AttachmentSettingType{ + AllowedTypes: ".avif,.cpuprofile,.csv,.dmp,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.json,.jsonc,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.webp,.xls,.xlsx,.zip", + MaxSize: 2048, + MaxFiles: 5, + Enabled: true, + } sec, _ := rootCfg.GetSection("attachment") if sec == nil { Attachment.Storage, err = getStorage(rootCfg, "attachments", "", nil) return err } - Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(".cpuprofile,.csv,.dmp,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.json,.jsonc,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.xls,.xlsx,.zip") - Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(2048) - Attachment.MaxFiles = sec.Key("MAX_FILES").MustInt(5) - Attachment.Enabled = sec.Key("ENABLED").MustBool(true) - + Attachment.AllowedTypes = sec.Key("ALLOWED_TYPES").MustString(Attachment.AllowedTypes) + Attachment.MaxSize = sec.Key("MAX_SIZE").MustInt64(Attachment.MaxSize) + Attachment.MaxFiles = sec.Key("MAX_FILES").MustInt(Attachment.MaxFiles) + Attachment.Enabled = sec.Key("ENABLED").MustBool(Attachment.Enabled) Attachment.Storage, err = getStorage(rootCfg, "attachments", "", sec) return err } diff --git a/modules/typesniffer/typesniffer.go b/modules/typesniffer/typesniffer.go index 6aec5c285e..8cb3d278ce 100644 --- a/modules/typesniffer/typesniffer.go +++ b/modules/typesniffer/typesniffer.go @@ -5,10 +5,12 @@ package typesniffer import ( "bytes" + "encoding/binary" "fmt" "io" "net/http" "regexp" + "slices" "strings" "code.gitea.io/gitea/modules/util" @@ -18,10 +20,10 @@ import ( const sniffLen = 1024 const ( - // SvgMimeType MIME type of SVG images. - SvgMimeType = "image/svg+xml" - // ApplicationOctetStream MIME type of binary files. - ApplicationOctetStream = "application/octet-stream" + MimeTypeImageSvg = "image/svg+xml" + MimeTypeImageAvif = "image/avif" + + MimeTypeApplicationOctetStream = "application/octet-stream" ) var ( @@ -47,7 +49,7 @@ func (ct SniffedType) IsImage() bool { // IsSvgImage detects if data is an SVG image format func (ct SniffedType) IsSvgImage() bool { - return strings.Contains(ct.contentType, SvgMimeType) + return strings.Contains(ct.contentType, MimeTypeImageSvg) } // IsPDF detects if data is a PDF format @@ -81,6 +83,26 @@ func (ct SniffedType) GetMimeType() string { return strings.SplitN(ct.contentType, ";", 2)[0] } +// https://en.wikipedia.org/wiki/ISO_base_media_file_format#File_type_box +func detectFileTypeBox(data []byte) (brands []string, found bool) { + if len(data) < 12 { + return nil, false + } + boxSize := int(binary.BigEndian.Uint32(data[:4])) + if boxSize < 12 || boxSize > len(data) { + return nil, false + } + tag := string(data[4:8]) + if tag != "ftyp" { + return nil, false + } + brands = append(brands, string(data[8:12])) + for i := 16; i+4 <= boxSize; i += 4 { + brands = append(brands, string(data[i:i+4])) + } + return brands, true +} + // DetectContentType extends http.DetectContentType with more content types. Defaults to text/unknown if input is empty. func DetectContentType(data []byte) SniffedType { if len(data) == 0 { @@ -94,7 +116,6 @@ func DetectContentType(data []byte) SniffedType { } // SVG is unsupported by http.DetectContentType, https://github.com/golang/go/issues/15888 - detectByHTML := strings.Contains(ct, "text/plain") || strings.Contains(ct, "text/html") detectByXML := strings.Contains(ct, "text/xml") if detectByHTML || detectByXML { @@ -102,7 +123,7 @@ func DetectContentType(data []byte) SniffedType { dataProcessed = bytes.TrimSpace(dataProcessed) if detectByHTML && svgTagRegex.Match(dataProcessed) || detectByXML && svgTagInXMLRegex.Match(dataProcessed) { - ct = SvgMimeType + ct = MimeTypeImageSvg } } @@ -116,6 +137,11 @@ func DetectContentType(data []byte) SniffedType { } } + fileTypeBrands, found := detectFileTypeBox(data) + if found && slices.Contains(fileTypeBrands, "avif") { + ct = MimeTypeImageAvif + } + if ct == "application/ogg" { dataHead := data if len(dataHead) > 256 { diff --git a/modules/typesniffer/typesniffer_test.go b/modules/typesniffer/typesniffer_test.go index 731fac11e7..3e5db3308b 100644 --- a/modules/typesniffer/typesniffer_test.go +++ b/modules/typesniffer/typesniffer_test.go @@ -134,3 +134,33 @@ func TestDetectContentTypeOgg(t *testing.T) { assert.NoError(t, err) assert.True(t, st.IsVideo()) } + +func TestDetectFileTypeBox(t *testing.T) { + _, found := detectFileTypeBox([]byte("\x00\x00\xff\xffftypAAAA....")) + assert.False(t, found) + + brands, found := detectFileTypeBox([]byte("\x00\x00\x00\x0cftypAAAA")) + assert.True(t, found) + assert.Equal(t, []string{"AAAA"}, brands) + + brands, found = detectFileTypeBox([]byte("\x00\x00\x00\x10ftypAAAA....BBBB")) + assert.True(t, found) + assert.Equal(t, []string{"AAAA"}, brands) + + brands, found = detectFileTypeBox([]byte("\x00\x00\x00\x14ftypAAAA....BBBB")) + assert.True(t, found) + assert.Equal(t, []string{"AAAA", "BBBB"}, brands) + + _, found = detectFileTypeBox([]byte("\x00\x00\x00\x14ftypAAAA....BBB")) + assert.False(t, found) + + brands, found = detectFileTypeBox([]byte("\x00\x00\x00\x13ftypAAAA....BBB")) + assert.True(t, found) + assert.Equal(t, []string{"AAAA"}, brands) +} + +func TestDetectContentTypeAvif(t *testing.T) { + buf := []byte("\x00\x00\x00\x20ftypavif.......................") + st := DetectContentType(buf) + assert.Equal(t, MimeTypeImageAvif, st.contentType) +} diff --git a/web_src/js/utils.test.ts b/web_src/js/utils.test.ts index 647676bf20..ac9d4fab91 100644 --- a/web_src/js/utils.test.ts +++ b/web_src/js/utils.test.ts @@ -118,7 +118,7 @@ test('encodeURLEncodedBase64, decodeURLEncodedBase64', () => { }); test('file detection', () => { - for (const name of ['a.jpg', '/a.jpeg', '.file.png', '.webp', 'file.svg']) { + for (const name of ['a.avif', 'a.jpg', '/a.jpeg', '.file.png', '.webp', 'file.svg']) { expect(isImageFile({name})).toBeTruthy(); } for (const name of ['', 'a.jpg.x', '/path.png/x', 'webp']) { diff --git a/web_src/js/utils.ts b/web_src/js/utils.ts index 4fed74e20f..bd872f094c 100644 --- a/web_src/js/utils.ts +++ b/web_src/js/utils.ts @@ -165,7 +165,7 @@ export function sleep(ms: number): Promise { } export function isImageFile({name, type}: {name: string, type?: string}): boolean { - return /\.(jpe?g|png|gif|webp|svg|heic)$/i.test(name || '') || type?.startsWith('image/'); + return /\.(avif|jpe?g|png|gif|webp|svg|heic)$/i.test(name || '') || type?.startsWith('image/'); } export function isVideoFile({name, type}: {name: string, type?: string}): boolean { From 21f7db2124cf42c3c74f7728b658f1bc1030acf1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Nov 2024 09:30:26 +0800 Subject: [PATCH 2/9] Fix incorrect project page CSS class (#32510) Otherwise milestone JS would run on this page and cause errors --- templates/repo/projects/new.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/projects/new.tmpl b/templates/repo/projects/new.tmpl index e70f3bca87..67abfe24be 100644 --- a/templates/repo/projects/new.tmpl +++ b/templates/repo/projects/new.tmpl @@ -1,5 +1,5 @@ {{template "base/head" .}} -
+
{{template "repo/header" .}}
{{template "projects/new" .}} From 4121f952d18a4c3a3c08ae645af3458ef08b439d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Nov 2024 18:13:01 -0800 Subject: [PATCH 3/9] Fix oauth2 error handle not return immediately (#32514) --- routers/web/auth/oauth.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 730d68051b..75f94de0ed 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -122,6 +122,8 @@ func SignInOAuthCallback(ctx *context.Context) { } if err, ok := err.(*go_oauth2.RetrieveError); ok { ctx.Flash.Error("OAuth2 RetrieveError: "+err.Error(), true) + ctx.Redirect(setting.AppSubURL + "/user/login") + return } ctx.ServerError("UserSignIn", err) return From e1b269e956e955dd1dfb012f40270d73f8329092 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Nov 2024 20:04:20 -0800 Subject: [PATCH 4/9] Remove transaction for archive download (#32186) Since there is a status column in the database, the transaction is unnecessary when downloading an archive. The transaction is blocking database operations, especially with SQLite. Replace #27563 --- services/repository/archiver/archiver.go | 33 ++++++++----------- services/repository/archiver/archiver_test.go | 12 +++---- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go index 01c58f0ce4..c33369d047 100644 --- a/services/repository/archiver/archiver.go +++ b/services/repository/archiver/archiver.go @@ -68,7 +68,7 @@ func (e RepoRefNotFoundError) Is(err error) bool { } // NewRequest creates an archival request, based on the URI. The -// resulting ArchiveRequest is suitable for being passed to ArchiveRepository() +// resulting ArchiveRequest is suitable for being passed to Await() // if it's determined that the request still needs to be satisfied. func NewRequest(repoID int64, repo *git.Repository, uri string) (*ArchiveRequest, error) { r := &ArchiveRequest{ @@ -151,13 +151,14 @@ func (aReq *ArchiveRequest) Await(ctx context.Context) (*repo_model.RepoArchiver } } +// doArchive satisfies the ArchiveRequest being passed in. Processing +// will occur in a separate goroutine, as this phase may take a while to +// complete. If the archive already exists, doArchive will not do +// anything. In all cases, the caller should be examining the *ArchiveRequest +// being returned for completion, as it may be different than the one they passed +// in. func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver, error) { - txCtx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - ctx, _, finished := process.GetManager().AddContext(txCtx, fmt.Sprintf("ArchiveRequest[%d]: %s", r.RepoID, r.GetArchiveName())) + ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("ArchiveRequest[%d]: %s", r.RepoID, r.GetArchiveName())) defer finished() archiver, err := repo_model.GetRepoArchiver(ctx, r.RepoID, r.Type, r.CommitID) @@ -192,7 +193,7 @@ func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver return nil, err } } - return archiver, committer.Commit() + return archiver, nil } if !errors.Is(err, os.ErrNotExist) { @@ -261,17 +262,7 @@ func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver } } - return archiver, committer.Commit() -} - -// ArchiveRepository satisfies the ArchiveRequest being passed in. Processing -// will occur in a separate goroutine, as this phase may take a while to -// complete. If the archive already exists, ArchiveRepository will not do -// anything. In all cases, the caller should be examining the *ArchiveRequest -// being returned for completion, as it may be different than the one they passed -// in. -func ArchiveRepository(ctx context.Context, request *ArchiveRequest) (*repo_model.RepoArchiver, error) { - return doArchive(ctx, request) + return archiver, nil } var archiverQueue *queue.WorkerPoolQueue[*ArchiveRequest] @@ -281,8 +272,10 @@ func Init(ctx context.Context) error { handler := func(items ...*ArchiveRequest) []*ArchiveRequest { for _, archiveReq := range items { log.Trace("ArchiverData Process: %#v", archiveReq) - if _, err := doArchive(ctx, archiveReq); err != nil { + if archiver, err := doArchive(ctx, archiveReq); err != nil { log.Error("Archive %v failed: %v", archiveReq, err) + } else { + log.Trace("ArchiverData Success: %#v", archiver) } } return nil diff --git a/services/repository/archiver/archiver_test.go b/services/repository/archiver/archiver_test.go index ec6e9dfac3..b3f3ed7bf3 100644 --- a/services/repository/archiver/archiver_test.go +++ b/services/repository/archiver/archiver_test.go @@ -80,13 +80,13 @@ func TestArchive_Basic(t *testing.T) { inFlight[1] = tgzReq inFlight[2] = secondReq - ArchiveRepository(db.DefaultContext, zipReq) - ArchiveRepository(db.DefaultContext, tgzReq) - ArchiveRepository(db.DefaultContext, secondReq) + doArchive(db.DefaultContext, zipReq) + doArchive(db.DefaultContext, tgzReq) + doArchive(db.DefaultContext, secondReq) // Make sure sending an unprocessed request through doesn't affect the queue // count. - ArchiveRepository(db.DefaultContext, zipReq) + doArchive(db.DefaultContext, zipReq) // Sleep two seconds to make sure the queue doesn't change. time.Sleep(2 * time.Second) @@ -101,7 +101,7 @@ func TestArchive_Basic(t *testing.T) { // We still have the other three stalled at completion, waiting to remove // from archiveInProgress. Try to submit this new one before its // predecessor has cleared out of the queue. - ArchiveRepository(db.DefaultContext, zipReq2) + doArchive(db.DefaultContext, zipReq2) // Now we'll submit a request and TimedWaitForCompletion twice, before and // after we release it. We should trigger both the timeout and non-timeout @@ -109,7 +109,7 @@ func TestArchive_Basic(t *testing.T) { timedReq, err := NewRequest(ctx.Repo.Repository.ID, ctx.Repo.GitRepo, secondCommit+".tar.gz") assert.NoError(t, err) assert.NotNil(t, timedReq) - ArchiveRepository(db.DefaultContext, timedReq) + doArchive(db.DefaultContext, timedReq) zipReq2, err = NewRequest(ctx.Repo.Repository.ID, ctx.Repo.GitRepo, firstCommit+".zip") assert.NoError(t, err) From a0c0cb3a2c426e39b91e7301c94ddc3a988c8607 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 15 Nov 2024 12:36:22 +0800 Subject: [PATCH 5/9] Fix `recentupdate` sorting bugs (#32505) Fix #32499 - Add the missing `recentupdate` to `OrderByFlatMap` - Assign default value(`recentupdate`) to `EXPLORE_PAGING_DEFAULT_SORT` --- models/repo/search.go | 1 + modules/setting/ui.go | 1 + 2 files changed, 2 insertions(+) diff --git a/models/repo/search.go b/models/repo/search.go index a73d9fc215..ffb8e26745 100644 --- a/models/repo/search.go +++ b/models/repo/search.go @@ -36,6 +36,7 @@ var OrderByMap = map[string]map[string]db.SearchOrderBy{ var OrderByFlatMap = map[string]db.SearchOrderBy{ "newest": OrderByMap["desc"]["created"], "oldest": OrderByMap["asc"]["created"], + "recentupdate": OrderByMap["desc"]["updated"], "leastupdate": OrderByMap["asc"]["updated"], "reversealphabetically": OrderByMap["desc"]["alpha"], "alphabetically": OrderByMap["asc"]["alpha"], diff --git a/modules/setting/ui.go b/modules/setting/ui.go index a8dc11d097..db0fe9ef79 100644 --- a/modules/setting/ui.go +++ b/modules/setting/ui.go @@ -86,6 +86,7 @@ var UI = struct { Reactions: []string{`+1`, `-1`, `laugh`, `hooray`, `confused`, `heart`, `rocket`, `eyes`}, CustomEmojis: []string{`git`, `gitea`, `codeberg`, `gitlab`, `github`, `gogs`}, CustomEmojisMap: map[string]string{"git": ":git:", "gitea": ":gitea:", "codeberg": ":codeberg:", "gitlab": ":gitlab:", "github": ":github:", "gogs": ":gogs:"}, + ExploreDefaultSort: "recentupdate", PreferredTimestampTense: "mixed", AmbiguousUnicodeDetection: true, From ecbb03dc6d0e0565663dff977a4fc3d40a1e0c1e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Nov 2024 23:45:07 +0800 Subject: [PATCH 6/9] Improve testing and try to fix MySQL hanging (#32515) By some CI fine tunes (`run tests`), SQLite & MSSQL could complete in about 12~13 minutes (before > 14), MySQL could complete in 18 minutes (before: about 23 or even > 30) Major changes: 1. use tmpfs for MySQL storage 1. run `make test-mysql` instead of `make integration-test-coverage` because the code coverage is not really used at the moment. 1. refactor testlogger to make it more reliable and be able to report stuck stacktrace 1. do not requeue failed items when a queue is being flushed (failed items would keep failing and make flush uncompleted) 1. reduce the file sizes for testing 1. use math ChaCha20 random data instead of crypot/rand (for testing purpose only) 1. no need to `DeleteRepository` in `TestLinguist` 1. other related refactoring to make code easier to maintain --- .github/workflows/pull-db-tests.yml | 10 +- models/migrations/base/tests.go | 15 +-- modules/log/color.go | 6 + modules/queue/workergroup.go | 34 +++--- modules/queue/workerqueue.go | 5 +- modules/testlogger/testlogger.go | 105 +++++++++++------- tests/integration/README.md | 14 +-- tests/integration/api_repo_file_get_test.go | 4 +- tests/integration/git_general_test.go | 79 ++++++------- tests/integration/git_misc_test.go | 4 +- tests/integration/integration_test.go | 25 ----- tests/integration/linguist_test.go | 73 ++++++------ .../migration-test/migration_test.go | 51 +++------ tests/integration/repo_branch_test.go | 2 - tests/test_utils.go | 20 +--- 15 files changed, 201 insertions(+), 246 deletions(-) diff --git a/.github/workflows/pull-db-tests.yml b/.github/workflows/pull-db-tests.yml index 22cb784245..0b23de0a66 100644 --- a/.github/workflows/pull-db-tests.yml +++ b/.github/workflows/pull-db-tests.yml @@ -154,12 +154,15 @@ jobs: runs-on: ubuntu-latest services: mysql: - image: mysql:8.0 + # the bitnami mysql image has more options than the official one, it's easier to customize + image: bitnami/mysql:8.0 env: - MYSQL_ALLOW_EMPTY_PASSWORD: true + ALLOW_EMPTY_PASSWORD: true MYSQL_DATABASE: testgitea ports: - "3306:3306" + options: >- + --mount type=tmpfs,destination=/bitnami/mysql/data elasticsearch: image: elasticsearch:7.5.0 env: @@ -188,7 +191,8 @@ jobs: - name: run migration tests run: make test-mysql-migration - name: run tests - run: make integration-test-coverage + # run: make integration-test-coverage (at the moment, no coverage is really handled) + run: make test-mysql env: TAGS: bindata RACE_ENABLED: true diff --git a/models/migrations/base/tests.go b/models/migrations/base/tests.go index 95d8f07934..ddf9a544da 100644 --- a/models/migrations/base/tests.go +++ b/models/migrations/base/tests.go @@ -15,7 +15,6 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/testlogger" @@ -91,12 +90,11 @@ func PrepareTestEnv(t *testing.T, skip int, syncModels ...any) (*xorm.Engine, fu } func MainTest(m *testing.M) { - log.RegisterEventWriter("test", testlogger.NewTestLoggerWriter) + testlogger.Init() giteaRoot := base.SetupGiteaRoot() if giteaRoot == "" { - fmt.Println("Environment variable $GITEA_ROOT not set") - os.Exit(1) + testlogger.Fatalf("Environment variable $GITEA_ROOT not set\n") } giteaBinary := "gitea" if runtime.GOOS == "windows" { @@ -104,8 +102,7 @@ func MainTest(m *testing.M) { } setting.AppPath = filepath.Join(giteaRoot, giteaBinary) if _, err := os.Stat(setting.AppPath); err != nil { - fmt.Printf("Could not find gitea binary at %s\n", setting.AppPath) - os.Exit(1) + testlogger.Fatalf("Could not find gitea binary at %s\n", setting.AppPath) } giteaConf := os.Getenv("GITEA_CONF") @@ -122,8 +119,7 @@ func MainTest(m *testing.M) { tmpDataPath, err := os.MkdirTemp("", "data") if err != nil { - fmt.Printf("Unable to create temporary data path %v\n", err) - os.Exit(1) + testlogger.Fatalf("Unable to create temporary data path %v\n", err) } setting.CustomPath = filepath.Join(setting.AppWorkPath, "custom") @@ -131,8 +127,7 @@ func MainTest(m *testing.M) { unittest.InitSettings() if err = git.InitFull(context.Background()); err != nil { - fmt.Printf("Unable to InitFull: %v\n", err) - os.Exit(1) + testlogger.Fatalf("Unable to InitFull: %v\n", err) } setting.LoadDBSetting() setting.InitLoggersForTest() diff --git a/modules/log/color.go b/modules/log/color.go index dcbba5f6d6..2a37fd0ea9 100644 --- a/modules/log/color.go +++ b/modules/log/color.go @@ -86,6 +86,8 @@ type ColoredValue struct { colors []ColorAttribute } +var _ fmt.Formatter = (*ColoredValue)(nil) + func (c *ColoredValue) Format(f fmt.State, verb rune) { _, _ = f.Write(ColorBytes(c.colors...)) s := fmt.Sprintf(fmt.FormatString(f, verb), c.v) @@ -93,6 +95,10 @@ func (c *ColoredValue) Format(f fmt.State, verb rune) { _, _ = f.Write(resetBytes) } +func (c *ColoredValue) Value() any { + return c.v +} + func NewColoredValue(v any, color ...ColorAttribute) *ColoredValue { return &ColoredValue{v: v, colors: color} } diff --git a/modules/queue/workergroup.go b/modules/queue/workergroup.go index 5859b64c0d..82b0790d5a 100644 --- a/modules/queue/workergroup.go +++ b/modules/queue/workergroup.go @@ -23,7 +23,7 @@ var ( ) func init() { - unhandledItemRequeueDuration.Store(int64(5 * time.Second)) + unhandledItemRequeueDuration.Store(int64(time.Second)) } // workerGroup is a group of workers to work with a WorkerPoolQueue @@ -104,7 +104,12 @@ func (q *WorkerPoolQueue[T]) doWorkerHandle(batch []T) { // if none of the items were handled, it should back-off for a few seconds // in this case the handler (eg: document indexer) may have encountered some errors/failures if len(unhandled) == len(batch) && unhandledItemRequeueDuration.Load() != 0 { + if q.isFlushing.Load() { + return // do not requeue items when flushing, since all items failed, requeue them will continue failing. + } log.Error("Queue %q failed to handle batch of %d items, backoff for a few seconds", q.GetName(), len(batch)) + // TODO: ideally it shouldn't "sleep" here (blocks the worker, then blocks flush). + // It could debounce the requeue operation, and try to requeue the items in the future. select { case <-q.ctxRun.Done(): case <-time.After(time.Duration(unhandledItemRequeueDuration.Load())): @@ -193,6 +198,9 @@ func (q *WorkerPoolQueue[T]) doStartNewWorker(wp *workerGroup[T]) { // doFlush flushes the queue: it tries to read all items from the queue and handles them. // It is for testing purpose only. It's not designed to work for a cluster. func (q *WorkerPoolQueue[T]) doFlush(wg *workerGroup[T], flush flushType) { + q.isFlushing.Store(true) + defer q.isFlushing.Store(false) + log.Debug("Queue %q starts flushing", q.GetName()) defer log.Debug("Queue %q finishes flushing", q.GetName()) @@ -236,6 +244,9 @@ loop: emptyCounter := 0 for { select { + case <-q.ctxRun.Done(): + log.Debug("Queue %q is shutting down", q.GetName()) + return case data, dataOk := <-wg.popItemChan: if !dataOk { return @@ -251,9 +262,6 @@ loop: log.Error("Failed to pop item from queue %q (doFlush): %v", q.GetName(), err) } return - case <-q.ctxRun.Done(): - log.Debug("Queue %q is shutting down", q.GetName()) - return case <-time.After(20 * time.Millisecond): // There is no reliable way to make sure all queue items are consumed by the Flush, there always might be some items stored in some buffers/temp variables. // If we run Gitea in a cluster, we can even not guarantee all items are consumed in a deterministic instance. @@ -331,6 +339,15 @@ func (q *WorkerPoolQueue[T]) doRun() { var batchDispatchC <-chan time.Time = infiniteTimerC for { select { + case flush := <-q.flushChan: + // before flushing, it needs to try to dispatch the batch to worker first, in case there is no worker running + // after the flushing, there is at least one worker running, so "doFlush" could wait for workers to finish + // since we are already in a "flush" operation, so the dispatching function shouldn't read the flush chan. + q.doDispatchBatchToWorker(wg, skipFlushChan) + q.doFlush(wg, flush) + case <-q.ctxRun.Done(): + log.Debug("Queue %q is shutting down", q.GetName()) + return case data, dataOk := <-wg.popItemChan: if !dataOk { return @@ -349,20 +366,11 @@ func (q *WorkerPoolQueue[T]) doRun() { case <-batchDispatchC: batchDispatchC = infiniteTimerC q.doDispatchBatchToWorker(wg, q.flushChan) - case flush := <-q.flushChan: - // before flushing, it needs to try to dispatch the batch to worker first, in case there is no worker running - // after the flushing, there is at least one worker running, so "doFlush" could wait for workers to finish - // since we are already in a "flush" operation, so the dispatching function shouldn't read the flush chan. - q.doDispatchBatchToWorker(wg, skipFlushChan) - q.doFlush(wg, flush) case err := <-wg.popItemErr: if !q.isCtxRunCanceled() { log.Error("Failed to pop item from queue %q (doRun): %v", q.GetName(), err) } return - case <-q.ctxRun.Done(): - log.Debug("Queue %q is shutting down", q.GetName()) - return } } } diff --git a/modules/queue/workerqueue.go b/modules/queue/workerqueue.go index f35ed93239..672e9a4114 100644 --- a/modules/queue/workerqueue.go +++ b/modules/queue/workerqueue.go @@ -32,8 +32,9 @@ type WorkerPoolQueue[T any] struct { baseConfig *BaseConfig baseQueue baseQueue - batchChan chan []T - flushChan chan flushType + batchChan chan []T + flushChan chan flushType + isFlushing atomic.Bool batchLength int workerNum int diff --git a/modules/testlogger/testlogger.go b/modules/testlogger/testlogger.go index 2bc3984ed6..2fbfce6b03 100644 --- a/modules/testlogger/testlogger.go +++ b/modules/testlogger/testlogger.go @@ -19,9 +19,10 @@ import ( ) var ( - prefix string - SlowTest = 10 * time.Second - SlowFlush = 5 * time.Second + prefix string + TestTimeout = 10 * time.Minute + TestSlowRun = 10 * time.Second + TestSlowFlush = 1 * time.Second ) var WriterCloser = &testLoggerWriterCloser{} @@ -89,79 +90,97 @@ func (w *testLoggerWriterCloser) Reset() { w.Unlock() } +// Printf takes a format and args and prints the string to os.Stdout +func Printf(format string, args ...any) { + if !log.CanColorStdout { + for i := 0; i < len(args); i++ { + if c, ok := args[i].(*log.ColoredValue); ok { + args[i] = c.Value() + } + } + } + _, _ = fmt.Fprintf(os.Stdout, format, args...) +} + // PrintCurrentTest prints the current test to os.Stdout func PrintCurrentTest(t testing.TB, skip ...int) func() { t.Helper() - start := time.Now() + runStart := time.Now() actualSkip := util.OptionalArg(skip) + 1 _, filename, line, _ := runtime.Caller(actualSkip) - if log.CanColorStdout { - _, _ = fmt.Fprintf(os.Stdout, "=== %s (%s:%d)\n", fmt.Formatter(log.NewColoredValue(t.Name())), strings.TrimPrefix(filename, prefix), line) - } else { - _, _ = fmt.Fprintf(os.Stdout, "=== %s (%s:%d)\n", t.Name(), strings.TrimPrefix(filename, prefix), line) - } + Printf("=== %s (%s:%d)\n", log.NewColoredValue(t.Name()), strings.TrimPrefix(filename, prefix), line) + WriterCloser.pushT(t) - return func() { - took := time.Since(start) - if took > SlowTest { - if log.CanColorStdout { - _, _ = fmt.Fprintf(os.Stdout, "+++ %s is a slow test (took %v)\n", fmt.Formatter(log.NewColoredValue(t.Name(), log.Bold, log.FgYellow)), fmt.Formatter(log.NewColoredValue(took, log.Bold, log.FgYellow))) - } else { - _, _ = fmt.Fprintf(os.Stdout, "+++ %s is a slow test (took %v)\n", t.Name(), took) + timeoutChecker := time.AfterFunc(TestTimeout, func() { + l := 128 * 1024 + var stack []byte + for { + stack = make([]byte, l) + n := runtime.Stack(stack, true) + if n <= l { + stack = stack[:n] + break } + l = n } - timer := time.AfterFunc(SlowFlush, func() { - if log.CanColorStdout { - _, _ = fmt.Fprintf(os.Stdout, "+++ %s ... still flushing after %v ...\n", fmt.Formatter(log.NewColoredValue(t.Name(), log.Bold, log.FgRed)), SlowFlush) - } else { - _, _ = fmt.Fprintf(os.Stdout, "+++ %s ... still flushing after %v ...\n", t.Name(), SlowFlush) - } + Printf("!!! %s ... timeout: %v ... stacktrace:\n%s\n\n", log.NewColoredValue(t.Name(), log.Bold, log.FgRed), TestTimeout, string(stack)) + }) + return func() { + flushStart := time.Now() + slowFlushChecker := time.AfterFunc(TestSlowFlush, func() { + Printf("+++ %s ... still flushing after %v ...\n", log.NewColoredValue(t.Name(), log.Bold, log.FgRed), TestSlowFlush) }) if err := queue.GetManager().FlushAll(context.Background(), -1); err != nil { t.Errorf("Flushing queues failed with error %v", err) } - timer.Stop() - flushTook := time.Since(start) - took - if flushTook > SlowFlush { - if log.CanColorStdout { - _, _ = fmt.Fprintf(os.Stdout, "+++ %s had a slow clean-up flush (took %v)\n", fmt.Formatter(log.NewColoredValue(t.Name(), log.Bold, log.FgRed)), fmt.Formatter(log.NewColoredValue(flushTook, log.Bold, log.FgRed))) - } else { - _, _ = fmt.Fprintf(os.Stdout, "+++ %s had a slow clean-up flush (took %v)\n", t.Name(), flushTook) - } + slowFlushChecker.Stop() + timeoutChecker.Stop() + + runDuration := time.Since(runStart) + flushDuration := time.Since(flushStart) + if runDuration > TestSlowRun { + Printf("+++ %s is a slow test (run: %v, flush: %v)\n", log.NewColoredValue(t.Name(), log.Bold, log.FgYellow), runDuration, flushDuration) } WriterCloser.popT() } } -// Printf takes a format and args and prints the string to os.Stdout -func Printf(format string, args ...any) { - if log.CanColorStdout { - for i := 0; i < len(args); i++ { - args[i] = log.NewColoredValue(args[i]) - } - } - _, _ = fmt.Fprintf(os.Stdout, "\t"+format, args...) -} - // TestLogEventWriter is a logger which will write to the testing log type TestLogEventWriter struct { *log.EventWriterBaseImpl } -// NewTestLoggerWriter creates a TestLogEventWriter as a log.LoggerProvider -func NewTestLoggerWriter(name string, mode log.WriterMode) log.EventWriter { +// newTestLoggerWriter creates a TestLogEventWriter as a log.LoggerProvider +func newTestLoggerWriter(name string, mode log.WriterMode) log.EventWriter { w := &TestLogEventWriter{} w.EventWriterBaseImpl = log.NewEventWriterBase(name, "test-log-writer", mode) w.OutputWriteCloser = WriterCloser return w } -func init() { +func Init() { const relFilePath = "modules/testlogger/testlogger.go" _, filename, _, _ := runtime.Caller(0) if !strings.HasSuffix(filename, relFilePath) { panic("source code file path doesn't match expected: " + relFilePath) } prefix = strings.TrimSuffix(filename, relFilePath) + + log.RegisterEventWriter("test", newTestLoggerWriter) + + duration, err := time.ParseDuration(os.Getenv("GITEA_TEST_SLOW_RUN")) + if err == nil && duration > 0 { + TestSlowRun = duration + } + + duration, err = time.ParseDuration(os.Getenv("GITEA_TEST_SLOW_FLUSH")) + if err == nil && duration > 0 { + TestSlowFlush = duration + } +} + +func Fatalf(format string, args ...any) { + Printf(format+"\n", args...) + os.Exit(1) } diff --git a/tests/integration/README.md b/tests/integration/README.md index e673bca228..3b44cfaaf0 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -99,18 +99,8 @@ We appreciate that some testing machines may not be very powerful and the default timeouts for declaring a slow test or a slow clean-up flush may not be appropriate. -You can either: - -* Within the test ini file set the following section: - -```ini -[integration-tests] -SLOW_TEST = 10s ; 10s is the default value -SLOW_FLUSH = 5S ; 5s is the default value -``` - -* Set the following environment variables: +You can set the following environment variables: ```bash -GITEA_SLOW_TEST_TIME="10s" GITEA_SLOW_FLUSH_TIME="5s" make test-sqlite +GITEA_TEST_SLOW_RUN="10s" GITEA_TEST_SLOW_FLUSH="1s" make test-sqlite ``` diff --git a/tests/integration/api_repo_file_get_test.go b/tests/integration/api_repo_file_get_test.go index 27bc9e25bf..2f897093ee 100644 --- a/tests/integration/api_repo_file_get_test.go +++ b/tests/integration/api_repo_file_get_test.go @@ -39,11 +39,11 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) - lfs := lfsCommitAndPushTest(t, dstPath, littleSize)[0] + lfs := lfsCommitAndPushTest(t, dstPath, testFileSizeSmall)[0] reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/media/"+lfs) respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) - assert.Equal(t, littleSize, respLFS.Length) + assert.Equal(t, testFileSizeSmall, respLFS.Length) doAPIDeleteRepository(httpContext) }) diff --git a/tests/integration/git_general_test.go b/tests/integration/git_general_test.go index 7fd19e7edd..a47cb75196 100644 --- a/tests/integration/git_general_test.go +++ b/tests/integration/git_general_test.go @@ -4,9 +4,10 @@ package integration import ( - "crypto/rand" "encoding/hex" "fmt" + "io" + mathRand "math/rand/v2" "net/http" "net/url" "os" @@ -34,8 +35,8 @@ import ( ) const ( - littleSize = 1024 // 1K - bigSize = 128 * 1024 * 1024 // 128M + testFileSizeSmall = 10 + testFileSizeLarge = 10 * 1024 * 1024 // 10M ) func TestGitGeneral(t *testing.T) { @@ -73,8 +74,8 @@ func testGitGeneral(t *testing.T, u *url.URL) { t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) - pushedFilesStandard := standardCommitAndPushTest(t, dstPath, littleSize, bigSize) - pushedFilesLFS := lfsCommitAndPushTest(t, dstPath, littleSize, bigSize) + pushedFilesStandard := standardCommitAndPushTest(t, dstPath, testFileSizeSmall, testFileSizeLarge) + pushedFilesLFS := lfsCommitAndPushTest(t, dstPath, testFileSizeSmall, testFileSizeLarge) rawTest(t, &httpContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) mediaTest(t, &httpContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) @@ -114,8 +115,8 @@ func testGitGeneral(t *testing.T, u *url.URL) { t.Run("Clone", doGitClone(dstPath, sshURL)) - pushedFilesStandard := standardCommitAndPushTest(t, dstPath, littleSize, bigSize) - pushedFilesLFS := lfsCommitAndPushTest(t, dstPath, littleSize, bigSize) + pushedFilesStandard := standardCommitAndPushTest(t, dstPath, testFileSizeSmall, testFileSizeLarge) + pushedFilesLFS := lfsCommitAndPushTest(t, dstPath, testFileSizeSmall, testFileSizeLarge) rawTest(t, &sshContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) mediaTest(t, &sshContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1]) @@ -202,14 +203,14 @@ func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS s // Request raw paths req := NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", little)) resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, littleSize, resp.Length) + assert.Equal(t, testFileSizeSmall, resp.Length) if setting.LFS.StartServer { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", littleLFS)) resp := session.MakeRequest(t, req, http.StatusOK) - assert.NotEqual(t, littleSize, resp.Body.Len()) + assert.NotEqual(t, testFileSizeSmall, resp.Body.Len()) assert.LessOrEqual(t, resp.Body.Len(), 1024) - if resp.Body.Len() != littleSize && resp.Body.Len() <= 1024 { + if resp.Body.Len() != testFileSizeSmall && resp.Body.Len() <= 1024 { assert.Contains(t, resp.Body.String(), lfs.MetaFileIdentifier) } } @@ -217,13 +218,13 @@ func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS s if !testing.Short() { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", big)) resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Length) + assert.Equal(t, testFileSizeLarge, resp.Length) if setting.LFS.StartServer { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", bigLFS)) resp := session.MakeRequest(t, req, http.StatusOK) - assert.NotEqual(t, bigSize, resp.Body.Len()) - if resp.Body.Len() != bigSize && resp.Body.Len() <= 1024 { + assert.NotEqual(t, testFileSizeLarge, resp.Body.Len()) + if resp.Body.Len() != testFileSizeLarge && resp.Body.Len() <= 1024 { assert.Contains(t, resp.Body.String(), lfs.MetaFileIdentifier) } } @@ -243,21 +244,21 @@ func mediaTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS // Request media paths req := NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", little)) resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, littleSize, resp.Length) + assert.Equal(t, testFileSizeSmall, resp.Length) req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", littleLFS)) resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, littleSize, resp.Length) + assert.Equal(t, testFileSizeSmall, resp.Length) if !testing.Short() { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", big)) resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Length) + assert.Equal(t, testFileSizeLarge, resp.Length) if setting.LFS.StartServer { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", bigLFS)) resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Length) + assert.Equal(t, testFileSizeLarge, resp.Length) } } }) @@ -287,35 +288,19 @@ func doCommitAndPush(t *testing.T, size int, repoPath, prefix string) string { } func generateCommitWithNewData(size int, repoPath, email, fullName, prefix string) (string, error) { - // Generate random file - bufSize := 4 * 1024 - if bufSize > size { - bufSize = size - } - - buffer := make([]byte, bufSize) - tmpFile, err := os.CreateTemp(repoPath, prefix) if err != nil { return "", err } defer tmpFile.Close() - written := 0 - for written < size { - n := size - written - if n > bufSize { - n = bufSize - } - _, err := rand.Read(buffer[:n]) - if err != nil { - return "", err - } - n, err = tmpFile.Write(buffer[:n]) - if err != nil { - return "", err - } - written += n + + var seed [32]byte + rander := mathRand.NewChaCha8(seed) // for testing only, no need to seed + _, err = io.CopyN(tmpFile, rander, int64(size)) + if err != nil { + return "", err } + _ = tmpFile.Close() // Commit // Now here we should explicitly allow lfs filters to run @@ -355,7 +340,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes // Try to push without permissions, which should fail t.Run("TryPushWithoutPermissions", func(t *testing.T) { - _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") + _, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") assert.NoError(t, err) doGitPushTestRepositoryFail(dstPath, "origin", "protected") }) @@ -367,7 +352,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes // Normal push should work t.Run("NormalPushWithPermissions", func(t *testing.T) { - _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") + _, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") assert.NoError(t, err) doGitPushTestRepository(dstPath, "origin", "protected") }) @@ -376,7 +361,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes t.Run("ForcePushWithoutForcePermissions", func(t *testing.T) { t.Run("CreateDivergentHistory", func(t *testing.T) { git.NewCommand(git.DefaultContext, "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath}) - _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-new") + _, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-new") assert.NoError(t, err) }) doGitPushTestRepositoryFail(dstPath, "-f", "origin", "protected") @@ -411,7 +396,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes assert.NoError(t, err) }) t.Run("GenerateCommit", func(t *testing.T) { - _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") + _, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") assert.NoError(t, err) }) t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "protected:unprotected-2")) @@ -426,7 +411,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes t.Run("ProtectProtectedBranchUnprotectedFilePaths", doProtectBranch(ctx, "protected", "", "", "unprotected-file-*")) t.Run("GenerateCommit", func(t *testing.T) { - _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "unprotected-file-") + _, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "unprotected-file-") assert.NoError(t, err) }) t.Run("PushUnprotectedFilesToProtectedBranch", doGitPushTestRepository(dstPath, "origin", "protected")) @@ -436,7 +421,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes t.Run("CheckoutMaster", doGitCheckoutBranch(dstPath, "master")) t.Run("CreateBranchForced", doGitCreateBranch(dstPath, "toforce")) t.Run("GenerateCommit", func(t *testing.T) { - _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") + _, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") assert.NoError(t, err) }) t.Run("FailToForcePushToProtectedBranch", doGitPushTestRepositoryFail(dstPath, "-f", "origin", "toforce:protected")) @@ -649,7 +634,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { t.Run("CheckoutProtected", doGitCheckoutBranch(dstPath, "protected")) t.Run("PullProtected", doGitPull(dstPath, "origin", "protected")) t.Run("GenerateCommit", func(t *testing.T) { - _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") + _, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") assert.NoError(t, err) }) t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "protected:unprotected3")) diff --git a/tests/integration/git_misc_test.go b/tests/integration/git_misc_test.go index 82ab184bb0..00ed2a766f 100644 --- a/tests/integration/git_misc_test.go +++ b/tests/integration/git_misc_test.go @@ -98,7 +98,7 @@ func TestAgitPullPush(t *testing.T) { doGitCreateBranch(dstPath, "test-agit-push") // commit 1 - _, err = generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") + _, err = generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") assert.NoError(t, err) // push to create an agit pull request @@ -115,7 +115,7 @@ func TestAgitPullPush(t *testing.T) { assert.Equal(t, "test-description", pr.Issue.Content) // commit 2 - _, err = generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-2-") + _, err = generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-2-") assert.NoError(t, err) // push 2 diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index a9db713f17..4338e19617 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -20,7 +20,6 @@ import ( "strings" "sync/atomic" "testing" - "time" "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/unittest" @@ -28,7 +27,6 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/testlogger" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers" @@ -90,27 +88,6 @@ func TestMain(m *testing.M) { tests.InitTest(true) testWebRoutes = routers.NormalRoutes() - // integration test settings... - if setting.CfgProvider != nil { - testingCfg := setting.CfgProvider.Section("integration-tests") - testlogger.SlowTest = testingCfg.Key("SLOW_TEST").MustDuration(testlogger.SlowTest) - testlogger.SlowFlush = testingCfg.Key("SLOW_FLUSH").MustDuration(testlogger.SlowFlush) - } - - if os.Getenv("GITEA_SLOW_TEST_TIME") != "" { - duration, err := time.ParseDuration(os.Getenv("GITEA_SLOW_TEST_TIME")) - if err == nil { - testlogger.SlowTest = duration - } - } - - if os.Getenv("GITEA_SLOW_FLUSH_TIME") != "" { - duration, err := time.ParseDuration(os.Getenv("GITEA_SLOW_FLUSH_TIME")) - if err == nil { - testlogger.SlowFlush = duration - } - } - os.Unsetenv("GIT_AUTHOR_NAME") os.Unsetenv("GIT_AUTHOR_EMAIL") os.Unsetenv("GIT_AUTHOR_DATE") @@ -132,8 +109,6 @@ func TestMain(m *testing.M) { // Instead, "No tests were found", last nonsense log is "According to the configuration, subsequent logs will not be printed to the console" exitCode := m.Run() - testlogger.WriterCloser.Reset() - if err = util.RemoveAll(setting.Indexer.IssuePath); err != nil { fmt.Printf("util.RemoveAll: %v\n", err) os.Exit(1) diff --git a/tests/integration/linguist_test.go b/tests/integration/linguist_test.go index e569de93a8..2d50dc599a 100644 --- a/tests/integration/linguist_test.go +++ b/tests/integration/linguist_test.go @@ -6,6 +6,7 @@ package integration import ( "context" "net/url" + "strconv" "strings" "testing" "time" @@ -19,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/queue" repo_service "code.gitea.io/gitea/services/repository" files_service "code.gitea.io/gitea/services/repository/files" + "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" ) @@ -218,42 +220,43 @@ func TestLinguist(t *testing.T) { } for i, c := range cases { - repo, err := repo_service.CreateRepository(db.DefaultContext, user, user, repo_service.CreateRepoOptions{ - Name: "linguist-test", + t.Run("Case-"+strconv.Itoa(i), func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repo, err := repo_service.CreateRepository(db.DefaultContext, user, user, repo_service.CreateRepoOptions{ + Name: "linguist-test-" + strconv.Itoa(i), + }) + assert.NoError(t, err) + + files := []*files_service.ChangeRepoFile{ + { + TreePath: ".gitattributes", + ContentReader: strings.NewReader(c.GitAttributesContent), + }, + } + files = append(files, c.FilesToAdd...) + for _, f := range files { + f.Operation = "create" + } + + _, err = files_service.ChangeRepoFiles(git.DefaultContext, repo, user, &files_service.ChangeRepoFilesOptions{ + Files: files, + OldBranch: repo.DefaultBranch, + NewBranch: repo.DefaultBranch, + }) + assert.NoError(t, err) + + assert.NoError(t, stats.UpdateRepoIndexer(repo)) + assert.NoError(t, queue.GetManager().FlushAll(context.Background(), 10*time.Second)) + + stats, err := repo_model.GetTopLanguageStats(db.DefaultContext, repo, len(c.FilesToAdd)) + assert.NoError(t, err) + + languages := make([]string, 0, len(stats)) + for _, s := range stats { + languages = append(languages, s.Language) + } + assert.Equal(t, c.ExpectedLanguageOrder, languages, "case %d: unexpected language stats", i) }) - assert.NoError(t, err) - - files := []*files_service.ChangeRepoFile{ - { - TreePath: ".gitattributes", - ContentReader: strings.NewReader(c.GitAttributesContent), - }, - } - files = append(files, c.FilesToAdd...) - for _, f := range files { - f.Operation = "create" - } - - _, err = files_service.ChangeRepoFiles(git.DefaultContext, repo, user, &files_service.ChangeRepoFilesOptions{ - Files: files, - OldBranch: repo.DefaultBranch, - NewBranch: repo.DefaultBranch, - }) - assert.NoError(t, err) - - assert.NoError(t, stats.UpdateRepoIndexer(repo)) - assert.NoError(t, queue.GetManager().FlushAll(context.Background(), 10*time.Second)) - - stats, err := repo_model.GetTopLanguageStats(db.DefaultContext, repo, len(c.FilesToAdd)) - assert.NoError(t, err) - - languages := make([]string, 0, len(stats)) - for _, s := range stats { - languages = append(languages, s.Language) - } - assert.Equal(t, c.ExpectedLanguageOrder, languages, "case %d: unexpected language stats", i) - - assert.NoError(t, repo_service.DeleteRepository(db.DefaultContext, user, repo, false)) } }) } diff --git a/tests/integration/migration-test/migration_test.go b/tests/integration/migration-test/migration_test.go index 9b61bdfe87..627d1f89c4 100644 --- a/tests/integration/migration-test/migration_test.go +++ b/tests/integration/migration-test/migration_test.go @@ -28,33 +28,29 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/testlogger" "code.gitea.io/gitea/modules/util" - "code.gitea.io/gitea/tests" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "xorm.io/xorm" ) var currentEngine *xorm.Engine func initMigrationTest(t *testing.T) func() { - log.RegisterEventWriter("test", testlogger.NewTestLoggerWriter) + testlogger.Init() - deferFn := tests.PrintCurrentTest(t, 2) + deferFn := testlogger.PrintCurrentTest(t, 2) giteaRoot := base.SetupGiteaRoot() if giteaRoot == "" { - tests.Printf("Environment variable $GITEA_ROOT not set\n") - os.Exit(1) + testlogger.Fatalf("Environment variable $GITEA_ROOT not set\n") } setting.AppPath = path.Join(giteaRoot, "gitea") if _, err := os.Stat(setting.AppPath); err != nil { - tests.Printf("Could not find gitea binary at %s\n", setting.AppPath) - os.Exit(1) + testlogger.Fatalf(fmt.Sprintf("Could not find gitea binary at %s\n", setting.AppPath)) } giteaConf := os.Getenv("GITEA_CONF") if giteaConf == "" { - tests.Printf("Environment variable $GITEA_CONF not set\n") - os.Exit(1) + testlogger.Fatalf("Environment variable $GITEA_CONF not set\n") } else if !path.IsAbs(giteaConf) { setting.CustomConf = path.Join(giteaRoot, giteaConf) } else { @@ -123,13 +119,10 @@ func readSQLFromFile(version string) (string, error) { return string(charset.MaybeRemoveBOM(bytes, charset.ConvertOpts{})), nil } -func restoreOldDB(t *testing.T, version string) bool { +func restoreOldDB(t *testing.T, version string) { data, err := readSQLFromFile(version) - assert.NoError(t, err) - if len(data) == 0 { - tests.Printf("No db found to restore for %s version: %s\n", setting.Database.Type, version) - return false - } + require.NoError(t, err) + require.NotEmpty(t, data, "No data found for %s version: %s", setting.Database.Type, version) switch { case setting.Database.Type.IsSQLite3(): @@ -197,15 +190,12 @@ func restoreOldDB(t *testing.T, version string) bool { db, err = sql.Open("postgres", fmt.Sprintf("postgres://%s:%s@%s/%s?sslmode=%s", setting.Database.User, setting.Database.Passwd, setting.Database.Host, setting.Database.Name, setting.Database.SSLMode)) } - if !assert.NoError(t, err) { - return false - } + require.NoError(t, err) defer db.Close() schrows, err := db.Query(fmt.Sprintf("SELECT 1 FROM information_schema.schemata WHERE schema_name = '%s'", setting.Database.Schema)) - if !assert.NoError(t, err) || !assert.NotEmpty(t, schrows) { - return false - } + require.NoError(t, err) + require.NotEmpty(t, schrows) if !schrows.Next() { // Create and setup a DB schema @@ -260,7 +250,6 @@ func restoreOldDB(t *testing.T, version string) bool { } db.Close() } - return true } func wrappedMigrate(x *xorm.Engine) error { @@ -269,11 +258,8 @@ func wrappedMigrate(x *xorm.Engine) error { } func doMigrationTest(t *testing.T, version string) { - defer tests.PrintCurrentTest(t)() - tests.Printf("Performing migration test for %s version: %s\n", setting.Database.Type, version) - if !restoreOldDB(t, version) { - return - } + defer testlogger.PrintCurrentTest(t)() + restoreOldDB(t, version) setting.InitSQLLoggersForCli(log.INFO) @@ -305,14 +291,9 @@ func TestMigrations(t *testing.T) { dialect := setting.Database.Type versions, err := availableVersions() - assert.NoError(t, err) + require.NoError(t, err) + require.NotEmpty(t, versions, "No old database versions available to migration test for %s", dialect) - if len(versions) == 0 { - tests.Printf("No old database versions available to migration test for %s\n", dialect) - return - } - - tests.Printf("Preparing to test %d migrations for %s\n", len(versions), dialect) for _, version := range versions { t.Run(fmt.Sprintf("Migrate-%s-%s", dialect, version), func(t *testing.T) { doMigrationTest(t, version) diff --git a/tests/integration/repo_branch_test.go b/tests/integration/repo_branch_test.go index 6d1cc8afcf..2b4c417334 100644 --- a/tests/integration/repo_branch_test.go +++ b/tests/integration/repo_branch_test.go @@ -209,8 +209,6 @@ func checkRecentlyPushedNewBranches(t *testing.T, session *TestSession, repoPath } func TestRecentlyPushedNewBranches(t *testing.T) { - defer tests.PrepareTestEnv(t)() - onGiteaRun(t, func(t *testing.T, u *url.URL) { user1Session := loginUser(t, "user1") user2Session := loginUser(t, "user2") diff --git a/tests/test_utils.go b/tests/test_utils.go index 8df739689d..deefdd43c5 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -29,17 +29,12 @@ import ( "github.com/stretchr/testify/assert" ) -func exitf(format string, args ...any) { - fmt.Printf(format+"\n", args...) - os.Exit(1) -} - func InitTest(requireGitea bool) { - log.RegisterEventWriter("test", testlogger.NewTestLoggerWriter) + testlogger.Init() giteaRoot := base.SetupGiteaRoot() if giteaRoot == "" { - exitf("Environment variable $GITEA_ROOT not set") + testlogger.Fatalf("Environment variable $GITEA_ROOT not set\n") } // TODO: Speedup tests that rely on the event source ticker, confirm whether there is any bug or failure. @@ -54,7 +49,7 @@ func InitTest(requireGitea bool) { } setting.AppPath = filepath.Join(giteaRoot, giteaBinary) if _, err := os.Stat(setting.AppPath); err != nil { - exitf("Could not find gitea binary at %s", setting.AppPath) + testlogger.Fatalf("Could not find gitea binary at %s\n", setting.AppPath) } } giteaConf := os.Getenv("GITEA_CONF") @@ -66,7 +61,7 @@ func InitTest(requireGitea bool) { _ = os.Setenv("GITEA_CONF", giteaConf) fmt.Printf("Environment variable $GITEA_CONF not set, use default: %s\n", giteaConf) if !setting.EnableSQLite3 { - exitf(`sqlite3 requires: import _ "github.com/mattn/go-sqlite3" or -tags sqlite,sqlite_unlock_notify`) + testlogger.Fatalf(`sqlite3 requires: import _ "github.com/mattn/go-sqlite3" or -tags sqlite,sqlite_unlock_notify` + "\n") } } if !filepath.IsAbs(giteaConf) { @@ -85,7 +80,7 @@ func InitTest(requireGitea bool) { setting.LoadDBSetting() if err := storage.Init(); err != nil { - exitf("Init storage failed: %v", err) + testlogger.Fatalf("Init storage failed: %v\n", err) } switch { @@ -258,8 +253,3 @@ func PrintCurrentTest(t testing.TB, skip ...int) func() { t.Helper() return testlogger.PrintCurrentTest(t, util.OptionalArg(skip)+1) } - -// Printf takes a format and args and prints the string to os.Stdout -func Printf(format string, args ...any) { - testlogger.Printf(format, args...) -} From e546480d0a2bbd9fcf897d7506a1efe3fa44cef3 Mon Sep 17 00:00:00 2001 From: charles <30816317+charles7668@users.noreply.github.com> Date: Sat, 16 Nov 2024 02:34:54 +0800 Subject: [PATCH 7/9] Fix large image overflow in comment page (#31740) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close #31709 52px is calculate by avatar size in templates\repo\issue\view_content\comments.tmpl ```html ``` + ```css .ui.comments .comment > .avatar ~ .content { margin-left: 12px; } ``` ![圖片](https://github.com/user-attachments/assets/bf15f4d4-1574-46f6-9f5e-1fbdbf1a98b0) --------- Co-authored-by: wxiaoguang --- templates/repo/issue/fields/markdown.tmpl | 2 +- web_src/css/modules/comment.css | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/templates/repo/issue/fields/markdown.tmpl b/templates/repo/issue/fields/markdown.tmpl index f6995328dd..da8f5e6bdf 100644 --- a/templates/repo/issue/fields/markdown.tmpl +++ b/templates/repo/issue/fields/markdown.tmpl @@ -1,3 +1,3 @@
-
{{ctx.RenderUtils.MarkdownToHtml .item.Attributes.value}}
+
{{ctx.RenderUtils.MarkdownToHtml .item.Attributes.value}}
diff --git a/web_src/css/modules/comment.css b/web_src/css/modules/comment.css index 68306686ef..9947b15b9a 100644 --- a/web_src/css/modules/comment.css +++ b/web_src/css/modules/comment.css @@ -53,6 +53,7 @@ display: flex; flex-direction: column; flex: 1; + min-width: 0; } .ui.comments .comment > .avatar ~ .content { From 5eebe1dc5fb29a162c51d050396fce7b14e47f4e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 16 Nov 2024 16:41:44 +0800 Subject: [PATCH 8/9] Fix and refactor markdown rendering (#32522) --- models/repo/repo.go | 49 +++++--- models/repo/repo_test.go | 67 ++++++----- modules/markup/html.go | 126 +++++++++------------ modules/markup/html_commit.go | 6 +- modules/markup/html_email.go | 2 +- modules/markup/html_emoji.go | 2 +- modules/markup/html_internal_test.go | 68 +++++++---- modules/markup/html_issue.go | 14 ++- modules/markup/html_link.go | 27 +---- modules/markup/html_node.go | 4 +- modules/markup/html_test.go | 15 ++- modules/markup/markdown/goldmark.go | 5 +- modules/markup/markdown/markdown_test.go | 69 +++++------ modules/markup/markdown/transform_image.go | 2 +- modules/markup/orgmode/orgmode.go | 5 +- modules/markup/orgmode/orgmode_test.go | 2 +- modules/markup/render.go | 29 ++--- modules/markup/render_links.go | 2 +- modules/templates/util_render.go | 23 ++-- modules/templates/util_render_test.go | 20 ++-- routers/common/markup.go | 13 ++- routers/web/feed/convert.go | 2 +- routers/web/feed/profile.go | 4 +- routers/web/org/home.go | 2 +- routers/web/repo/wiki.go | 5 +- routers/web/shared/user/header.go | 2 +- templates/repo/view_file.tmpl | 2 +- 27 files changed, 289 insertions(+), 278 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 4776ff0b9c..7d78cee287 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "html/template" + "maps" "net" "net/url" "path/filepath" @@ -165,10 +166,10 @@ type Repository struct { Status RepositoryStatus `xorm:"NOT NULL DEFAULT 0"` - RenderingMetas map[string]string `xorm:"-"` - DocumentRenderingMetas map[string]string `xorm:"-"` - Units []*RepoUnit `xorm:"-"` - PrimaryLanguage *LanguageStat `xorm:"-"` + commonRenderingMetas map[string]string `xorm:"-"` + + Units []*RepoUnit `xorm:"-"` + PrimaryLanguage *LanguageStat `xorm:"-"` IsFork bool `xorm:"INDEX NOT NULL DEFAULT false"` ForkID int64 `xorm:"INDEX"` @@ -473,9 +474,8 @@ func (repo *Repository) MustOwner(ctx context.Context) *user_model.User { return repo.Owner } -// ComposeMetas composes a map of metas for properly rendering issue links and external issue trackers. -func (repo *Repository) ComposeMetas(ctx context.Context) map[string]string { - if len(repo.RenderingMetas) == 0 { +func (repo *Repository) composeCommonMetas(ctx context.Context) map[string]string { + if len(repo.commonRenderingMetas) == 0 { metas := map[string]string{ "user": repo.OwnerName, "repo": repo.Name, @@ -508,21 +508,34 @@ func (repo *Repository) ComposeMetas(ctx context.Context) map[string]string { metas["org"] = strings.ToLower(repo.OwnerName) } - repo.RenderingMetas = metas + repo.commonRenderingMetas = metas } - return repo.RenderingMetas + return repo.commonRenderingMetas } -// ComposeDocumentMetas composes a map of metas for properly rendering documents +// ComposeMetas composes a map of metas for properly rendering comments or comment-like contents (commit message) +func (repo *Repository) ComposeMetas(ctx context.Context) map[string]string { + metas := maps.Clone(repo.composeCommonMetas(ctx)) + metas["markdownLineBreakStyle"] = "comment" + metas["markupAllowShortIssuePattern"] = "true" + return metas +} + +// ComposeWikiMetas composes a map of metas for properly rendering wikis +func (repo *Repository) ComposeWikiMetas(ctx context.Context) map[string]string { + // does wiki need the "teams" and "org" from common metas? + metas := maps.Clone(repo.composeCommonMetas(ctx)) + metas["markdownLineBreakStyle"] = "document" + metas["markupAllowShortIssuePattern"] = "true" + return metas +} + +// ComposeDocumentMetas composes a map of metas for properly rendering documents (repo files) func (repo *Repository) ComposeDocumentMetas(ctx context.Context) map[string]string { - if len(repo.DocumentRenderingMetas) == 0 { - metas := map[string]string{} - for k, v := range repo.ComposeMetas(ctx) { - metas[k] = v - } - repo.DocumentRenderingMetas = metas - } - return repo.DocumentRenderingMetas + // does document(file) need the "teams" and "org" from common metas? + metas := maps.Clone(repo.composeCommonMetas(ctx)) + metas["markdownLineBreakStyle"] = "document" + return metas } // GetBaseRepo populates repo.BaseRepo for a fork repository and diff --git a/models/repo/repo_test.go b/models/repo/repo_test.go index c13b698abf..6468e0f605 100644 --- a/models/repo/repo_test.go +++ b/models/repo/repo_test.go @@ -1,13 +1,12 @@ // Copyright 2017 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package repo_test +package repo import ( "testing" "code.gitea.io/gitea/models/db" - repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -20,18 +19,18 @@ import ( ) var ( - countRepospts = repo_model.CountRepositoryOptions{OwnerID: 10} - countReposptsPublic = repo_model.CountRepositoryOptions{OwnerID: 10, Private: optional.Some(false)} - countReposptsPrivate = repo_model.CountRepositoryOptions{OwnerID: 10, Private: optional.Some(true)} + countRepospts = CountRepositoryOptions{OwnerID: 10} + countReposptsPublic = CountRepositoryOptions{OwnerID: 10, Private: optional.Some(false)} + countReposptsPrivate = CountRepositoryOptions{OwnerID: 10, Private: optional.Some(true)} ) func TestGetRepositoryCount(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) ctx := db.DefaultContext - count, err1 := repo_model.CountRepositories(ctx, countRepospts) - privateCount, err2 := repo_model.CountRepositories(ctx, countReposptsPrivate) - publicCount, err3 := repo_model.CountRepositories(ctx, countReposptsPublic) + count, err1 := CountRepositories(ctx, countRepospts) + privateCount, err2 := CountRepositories(ctx, countReposptsPrivate) + publicCount, err3 := CountRepositories(ctx, countReposptsPublic) assert.NoError(t, err1) assert.NoError(t, err2) assert.NoError(t, err3) @@ -42,7 +41,7 @@ func TestGetRepositoryCount(t *testing.T) { func TestGetPublicRepositoryCount(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - count, err := repo_model.CountRepositories(db.DefaultContext, countReposptsPublic) + count, err := CountRepositories(db.DefaultContext, countReposptsPublic) assert.NoError(t, err) assert.Equal(t, int64(1), count) } @@ -50,14 +49,14 @@ func TestGetPublicRepositoryCount(t *testing.T) { func TestGetPrivateRepositoryCount(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - count, err := repo_model.CountRepositories(db.DefaultContext, countReposptsPrivate) + count, err := CountRepositories(db.DefaultContext, countReposptsPrivate) assert.NoError(t, err) assert.Equal(t, int64(2), count) } func TestRepoAPIURL(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) + repo := unittest.AssertExistsAndLoadBean(t, &Repository{ID: 10}) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user12/repo10", repo.APIURL()) } @@ -65,22 +64,22 @@ func TestRepoAPIURL(t *testing.T) { func TestWatchRepo(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + repo := unittest.AssertExistsAndLoadBean(t, &Repository{ID: 3}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - assert.NoError(t, repo_model.WatchRepo(db.DefaultContext, user, repo, true)) - unittest.AssertExistsAndLoadBean(t, &repo_model.Watch{RepoID: repo.ID, UserID: user.ID}) - unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repo.ID}) + assert.NoError(t, WatchRepo(db.DefaultContext, user, repo, true)) + unittest.AssertExistsAndLoadBean(t, &Watch{RepoID: repo.ID, UserID: user.ID}) + unittest.CheckConsistencyFor(t, &Repository{ID: repo.ID}) - assert.NoError(t, repo_model.WatchRepo(db.DefaultContext, user, repo, false)) - unittest.AssertNotExistsBean(t, &repo_model.Watch{RepoID: repo.ID, UserID: user.ID}) - unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repo.ID}) + assert.NoError(t, WatchRepo(db.DefaultContext, user, repo, false)) + unittest.AssertNotExistsBean(t, &Watch{RepoID: repo.ID, UserID: user.ID}) + unittest.CheckConsistencyFor(t, &Repository{ID: repo.ID}) } func TestMetas(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - repo := &repo_model.Repository{Name: "testRepo"} + repo := &Repository{Name: "testRepo"} repo.Owner = &user_model.User{Name: "testOwner"} repo.OwnerName = repo.Owner.Name @@ -90,16 +89,16 @@ func TestMetas(t *testing.T) { assert.Equal(t, "testRepo", metas["repo"]) assert.Equal(t, "testOwner", metas["user"]) - externalTracker := repo_model.RepoUnit{ + externalTracker := RepoUnit{ Type: unit.TypeExternalTracker, - Config: &repo_model.ExternalTrackerConfig{ + Config: &ExternalTrackerConfig{ ExternalTrackerFormat: "https://someurl.com/{user}/{repo}/{issue}", }, } testSuccess := func(expectedStyle string) { - repo.Units = []*repo_model.RepoUnit{&externalTracker} - repo.RenderingMetas = nil + repo.Units = []*RepoUnit{&externalTracker} + repo.commonRenderingMetas = nil metas := repo.ComposeMetas(db.DefaultContext) assert.Equal(t, expectedStyle, metas["style"]) assert.Equal(t, "testRepo", metas["repo"]) @@ -118,7 +117,7 @@ func TestMetas(t *testing.T) { externalTracker.ExternalTrackerConfig().ExternalTrackerStyle = markup.IssueNameStyleRegexp testSuccess(markup.IssueNameStyleRegexp) - repo, err := repo_model.GetRepositoryByID(db.DefaultContext, 3) + repo, err := GetRepositoryByID(db.DefaultContext, 3) assert.NoError(t, err) metas = repo.ComposeMetas(db.DefaultContext) @@ -132,7 +131,7 @@ func TestGetRepositoryByURL(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) t.Run("InvalidPath", func(t *testing.T) { - repo, err := repo_model.GetRepositoryByURL(db.DefaultContext, "something") + repo, err := GetRepositoryByURL(db.DefaultContext, "something") assert.Nil(t, repo) assert.Error(t, err) @@ -140,7 +139,7 @@ func TestGetRepositoryByURL(t *testing.T) { t.Run("ValidHttpURL", func(t *testing.T) { test := func(t *testing.T, url string) { - repo, err := repo_model.GetRepositoryByURL(db.DefaultContext, url) + repo, err := GetRepositoryByURL(db.DefaultContext, url) assert.NotNil(t, repo) assert.NoError(t, err) @@ -155,7 +154,7 @@ func TestGetRepositoryByURL(t *testing.T) { t.Run("ValidGitSshURL", func(t *testing.T) { test := func(t *testing.T, url string) { - repo, err := repo_model.GetRepositoryByURL(db.DefaultContext, url) + repo, err := GetRepositoryByURL(db.DefaultContext, url) assert.NotNil(t, repo) assert.NoError(t, err) @@ -173,7 +172,7 @@ func TestGetRepositoryByURL(t *testing.T) { t.Run("ValidImplicitSshURL", func(t *testing.T) { test := func(t *testing.T, url string) { - repo, err := repo_model.GetRepositoryByURL(db.DefaultContext, url) + repo, err := GetRepositoryByURL(db.DefaultContext, url) assert.NotNil(t, repo) assert.NoError(t, err) @@ -200,21 +199,21 @@ func TestComposeSSHCloneURL(t *testing.T) { setting.SSH.Domain = "domain" setting.SSH.Port = 22 setting.Repository.UseCompatSSHURI = false - assert.Equal(t, "git@domain:user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + assert.Equal(t, "git@domain:user/repo.git", ComposeSSHCloneURL("user", "repo")) setting.Repository.UseCompatSSHURI = true - assert.Equal(t, "ssh://git@domain/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + assert.Equal(t, "ssh://git@domain/user/repo.git", ComposeSSHCloneURL("user", "repo")) // test SSH_DOMAIN while use non-standard SSH port setting.SSH.Port = 123 setting.Repository.UseCompatSSHURI = false - assert.Equal(t, "ssh://git@domain:123/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + assert.Equal(t, "ssh://git@domain:123/user/repo.git", ComposeSSHCloneURL("user", "repo")) setting.Repository.UseCompatSSHURI = true - assert.Equal(t, "ssh://git@domain:123/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + assert.Equal(t, "ssh://git@domain:123/user/repo.git", ComposeSSHCloneURL("user", "repo")) // test IPv6 SSH_DOMAIN setting.Repository.UseCompatSSHURI = false setting.SSH.Domain = "::1" setting.SSH.Port = 22 - assert.Equal(t, "git@[::1]:user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + assert.Equal(t, "git@[::1]:user/repo.git", ComposeSSHCloneURL("user", "repo")) setting.SSH.Port = 123 - assert.Equal(t, "ssh://git@[::1]:123/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + assert.Equal(t, "ssh://git@[::1]:123/user/repo.git", ComposeSSHCloneURL("user", "repo")) } diff --git a/modules/markup/html.go b/modules/markup/html.go index 54c65c95d2..16ccd4b406 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -7,11 +7,11 @@ import ( "bytes" "io" "regexp" + "slices" "strings" "sync" "code.gitea.io/gitea/modules/markup/common" - "code.gitea.io/gitea/modules/setting" "golang.org/x/net/html" "golang.org/x/net/html/atom" @@ -25,7 +25,27 @@ const ( IssueNameStyleRegexp = "regexp" ) -var ( +// CSS class for action keywords (e.g. "closes: #1") +const keywordClass = "issue-keyword" + +type globalVarsType struct { + hashCurrentPattern *regexp.Regexp + shortLinkPattern *regexp.Regexp + anyHashPattern *regexp.Regexp + comparePattern *regexp.Regexp + fullURLPattern *regexp.Regexp + emailRegex *regexp.Regexp + blackfridayExtRegex *regexp.Regexp + emojiShortCodeRegex *regexp.Regexp + issueFullPattern *regexp.Regexp + filesChangedFullPattern *regexp.Regexp + + tagCleaner *regexp.Regexp + nulCleaner *strings.Replacer +} + +var globalVars = sync.OnceValue[*globalVarsType](func() *globalVarsType { + v := &globalVarsType{} // NOTE: All below regex matching do not perform any extra validation. // Thus a link is produced even if the linked entity does not exist. // While fast, this is also incorrect and lead to false positives. @@ -36,79 +56,56 @@ var ( // hashCurrentPattern matches string that represents a commit SHA, e.g. d8a994ef243349f321568f9e36d5c3f444b99cae // Although SHA1 hashes are 40 chars long, SHA256 are 64, the regex matches the hash from 7 to 64 chars in length // so that abbreviated hash links can be used as well. This matches git and GitHub usability. - hashCurrentPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-f]{7,64})(?:\s|$|\)|\]|[.,:](\s|$))`) + v.hashCurrentPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-f]{7,64})(?:\s|$|\)|\]|[.,:](\s|$))`) // shortLinkPattern matches short but difficult to parse [[name|link|arg=test]] syntax - shortLinkPattern = regexp.MustCompile(`\[\[(.*?)\]\](\w*)`) + v.shortLinkPattern = regexp.MustCompile(`\[\[(.*?)\]\](\w*)`) // anyHashPattern splits url containing SHA into parts - anyHashPattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{40,64})(/[-+~%./\w]+)?(\?[-+~%.\w&=]+)?(#[-+~%.\w]+)?`) + v.anyHashPattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{40,64})(/[-+~%./\w]+)?(\?[-+~%.\w&=]+)?(#[-+~%.\w]+)?`) // comparePattern matches "http://domain/org/repo/compare/COMMIT1...COMMIT2#hash" - comparePattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{7,64})(\.\.\.?)([0-9a-f]{7,64})?(#[-+~_%.a-zA-Z0-9]+)?`) + v.comparePattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{7,64})(\.\.\.?)([0-9a-f]{7,64})?(#[-+~_%.a-zA-Z0-9]+)?`) // fullURLPattern matches full URL like "mailto:...", "https://..." and "ssh+git://..." - fullURLPattern = regexp.MustCompile(`^[a-z][-+\w]+:`) + v.fullURLPattern = regexp.MustCompile(`^[a-z][-+\w]+:`) // emailRegex is definitely not perfect with edge cases, // it is still accepted by the CommonMark specification, as well as the HTML5 spec: // http://spec.commonmark.org/0.28/#email-address // https://html.spec.whatwg.org/multipage/input.html#e-mail-state-(type%3Demail) - emailRegex = regexp.MustCompile("(?:\\s|^|\\(|\\[)([a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9]{2,}(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)+)(?:\\s|$|\\)|\\]|;|,|\\?|!|\\.(\\s|$))") + v.emailRegex = regexp.MustCompile("(?:\\s|^|\\(|\\[)([a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9]{2,}(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)+)(?:\\s|$|\\)|\\]|;|,|\\?|!|\\.(\\s|$))") // blackfridayExtRegex is for blackfriday extensions create IDs like fn:user-content-footnote - blackfridayExtRegex = regexp.MustCompile(`[^:]*:user-content-`) + v.blackfridayExtRegex = regexp.MustCompile(`[^:]*:user-content-`) // emojiShortCodeRegex find emoji by alias like :smile: - emojiShortCodeRegex = regexp.MustCompile(`:[-+\w]+:`) -) + v.emojiShortCodeRegex = regexp.MustCompile(`:[-+\w]+:`) -// CSS class for action keywords (e.g. "closes: #1") -const keywordClass = "issue-keyword" + // example: https://domain/org/repo/pulls/27#hash + v.issueFullPattern = regexp.MustCompile(`https?://(?:\S+/)[\w_.-]+/[\w_.-]+/(?:issues|pulls)/((?:\w{1,10}-)?[1-9][0-9]*)([\?|#](\S+)?)?\b`) + + // example: https://domain/org/repo/pulls/27/files#hash + v.filesChangedFullPattern = regexp.MustCompile(`https?://(?:\S+/)[\w_.-]+/[\w_.-]+/pulls/((?:\w{1,10}-)?[1-9][0-9]*)/files([\?|#](\S+)?)?\b`) + + v.tagCleaner = regexp.MustCompile(`<((?:/?\w+/\w+)|(?:/[\w ]+/)|(/?[hH][tT][mM][lL]\b)|(/?[hH][eE][aA][dD]\b))`) + v.nulCleaner = strings.NewReplacer("\000", "") + return v +}) // IsFullURLBytes reports whether link fits valid format. func IsFullURLBytes(link []byte) bool { - return fullURLPattern.Match(link) + return globalVars().fullURLPattern.Match(link) } func IsFullURLString(link string) bool { - return fullURLPattern.MatchString(link) + return globalVars().fullURLPattern.MatchString(link) } func IsNonEmptyRelativePath(link string) bool { return link != "" && !IsFullURLString(link) && link[0] != '/' && link[0] != '?' && link[0] != '#' } -// regexp for full links to issues/pulls -var issueFullPattern *regexp.Regexp - -// Once for to prevent races -var issueFullPatternOnce sync.Once - -// regexp for full links to hash comment in pull request files changed tab -var filesChangedFullPattern *regexp.Regexp - -// Once for to prevent races -var filesChangedFullPatternOnce sync.Once - -func getIssueFullPattern() *regexp.Regexp { - issueFullPatternOnce.Do(func() { - // example: https://domain/org/repo/pulls/27#hash - issueFullPattern = regexp.MustCompile(regexp.QuoteMeta(setting.AppURL) + - `[\w_.-]+/[\w_.-]+/(?:issues|pulls)/((?:\w{1,10}-)?[1-9][0-9]*)([\?|#](\S+)?)?\b`) - }) - return issueFullPattern -} - -func getFilesChangedFullPattern() *regexp.Regexp { - filesChangedFullPatternOnce.Do(func() { - // example: https://domain/org/repo/pulls/27/files#hash - filesChangedFullPattern = regexp.MustCompile(regexp.QuoteMeta(setting.AppURL) + - `[\w_.-]+/[\w_.-]+/pulls/((?:\w{1,10}-)?[1-9][0-9]*)/files([\?|#](\S+)?)?\b`) - }) - return filesChangedFullPattern -} - // CustomLinkURLSchemes allows for additional schemes to be detected when parsing links within text func CustomLinkURLSchemes(schemes []string) { schemes = append(schemes, "http", "https") @@ -197,13 +194,6 @@ func RenderCommitMessage( content string, ) (string, error) { procs := commitMessageProcessors - if ctx.DefaultLink != "" { - // we don't have to fear data races, because being - // commitMessageProcessors of fixed len and cap, every time we append - // something to it the slice is realloc+copied, so append always - // generates the slice ex-novo. - procs = append(procs, genDefaultLinkProcessor(ctx.DefaultLink)) - } return renderProcessString(ctx, procs, content) } @@ -231,16 +221,17 @@ var emojiProcessors = []processor{ // which changes every text node into a link to the passed default link. func RenderCommitMessageSubject( ctx *RenderContext, - content string, + defaultLink, content string, ) (string, error) { - procs := commitMessageSubjectProcessors - if ctx.DefaultLink != "" { - // we don't have to fear data races, because being - // commitMessageSubjectProcessors of fixed len and cap, every time we - // append something to it the slice is realloc+copied, so append always - // generates the slice ex-novo. - procs = append(procs, genDefaultLinkProcessor(ctx.DefaultLink)) - } + procs := slices.Clone(commitMessageSubjectProcessors) + procs = append(procs, func(ctx *RenderContext, node *html.Node) { + ch := &html.Node{Parent: node, Type: html.TextNode, Data: node.Data} + node.Type = html.ElementNode + node.Data = "a" + node.DataAtom = atom.A + node.Attr = []html.Attribute{{Key: "href", Val: defaultLink}, {Key: "class", Val: "muted"}} + node.FirstChild, node.LastChild = ch, ch + }) return renderProcessString(ctx, procs, content) } @@ -249,10 +240,8 @@ func RenderIssueTitle( ctx *RenderContext, title string, ) (string, error) { + // do not render other issue/commit links in an issue's title - which in most cases is already a link. return renderProcessString(ctx, []processor{ - issueIndexPatternProcessor, - commitCrossReferencePatternProcessor, - hashCurrentPatternProcessor, emojiShortCodeProcessor, emojiProcessor, }, title) @@ -288,11 +277,6 @@ func RenderEmoji( return renderProcessString(ctx, emojiProcessors, content) } -var ( - tagCleaner = regexp.MustCompile(`<((?:/?\w+/\w+)|(?:/[\w ]+/)|(/?[hH][tT][mM][lL]\b)|(/?[hH][eE][aA][dD]\b))`) - nulCleaner = strings.NewReplacer("\000", "") -) - func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output io.Writer) error { defer ctx.Cancel() // FIXME: don't read all content to memory @@ -306,7 +290,7 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output // prepend "" strings.NewReader(""), // Strip out nuls - they're always invalid - bytes.NewReader(tagCleaner.ReplaceAll([]byte(nulCleaner.Replace(string(rawHTML))), []byte("<$1"))), + bytes.NewReader(globalVars().tagCleaner.ReplaceAll([]byte(globalVars().nulCleaner.Replace(string(rawHTML))), []byte("<$1"))), // close the tags strings.NewReader(""), )) @@ -353,7 +337,7 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Nod // Add user-content- to IDs and "#" links if they don't already have them for idx, attr := range node.Attr { val := strings.TrimPrefix(attr.Val, "#") - notHasPrefix := !(strings.HasPrefix(val, "user-content-") || blackfridayExtRegex.MatchString(val)) + notHasPrefix := !(strings.HasPrefix(val, "user-content-") || globalVars().blackfridayExtRegex.MatchString(val)) if attr.Key == "id" && notHasPrefix { node.Attr[idx].Val = "user-content-" + attr.Val diff --git a/modules/markup/html_commit.go b/modules/markup/html_commit.go index 86d70746d4..0e674c83e1 100644 --- a/modules/markup/html_commit.go +++ b/modules/markup/html_commit.go @@ -54,7 +54,7 @@ func createCodeLink(href, content, class string) *html.Node { } func anyHashPatternExtract(s string) (ret anyHashPatternResult, ok bool) { - m := anyHashPattern.FindStringSubmatchIndex(s) + m := globalVars().anyHashPattern.FindStringSubmatchIndex(s) if m == nil { return ret, false } @@ -120,7 +120,7 @@ func comparePatternProcessor(ctx *RenderContext, node *html.Node) { node = node.NextSibling continue } - m := comparePattern.FindStringSubmatchIndex(node.Data) + m := globalVars().comparePattern.FindStringSubmatchIndex(node.Data) if m == nil || slices.Contains(m[:8], -1) { // ensure that every group (m[0]...m[7]) has a match node = node.NextSibling continue @@ -173,7 +173,7 @@ func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { ctx.ShaExistCache = make(map[string]bool) } for node != nil && node != next && start < len(node.Data) { - m := hashCurrentPattern.FindStringSubmatchIndex(node.Data[start:]) + m := globalVars().hashCurrentPattern.FindStringSubmatchIndex(node.Data[start:]) if m == nil { return } diff --git a/modules/markup/html_email.go b/modules/markup/html_email.go index a062789b35..32d0285eb4 100644 --- a/modules/markup/html_email.go +++ b/modules/markup/html_email.go @@ -9,7 +9,7 @@ import "golang.org/x/net/html" func emailAddressProcessor(ctx *RenderContext, node *html.Node) { next := node.NextSibling for node != nil && node != next { - m := emailRegex.FindStringSubmatchIndex(node.Data) + m := globalVars().emailRegex.FindStringSubmatchIndex(node.Data) if m == nil { return } diff --git a/modules/markup/html_emoji.go b/modules/markup/html_emoji.go index c60d06b823..6eacb2067f 100644 --- a/modules/markup/html_emoji.go +++ b/modules/markup/html_emoji.go @@ -62,7 +62,7 @@ func emojiShortCodeProcessor(ctx *RenderContext, node *html.Node) { start := 0 next := node.NextSibling for node != nil && node != next && start < len(node.Data) { - m := emojiShortCodeRegex.FindStringSubmatchIndex(node.Data[start:]) + m := globalVars().emojiShortCodeRegex.FindStringSubmatchIndex(node.Data[start:]) if m == nil { return } diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 2fb657f56b..cdcc94d563 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -40,17 +40,19 @@ func link(href, class, contents string) string { } var numericMetas = map[string]string{ - "format": "https://someurl.com/{user}/{repo}/{index}", - "user": "someUser", - "repo": "someRepo", - "style": IssueNameStyleNumeric, + "format": "https://someurl.com/{user}/{repo}/{index}", + "user": "someUser", + "repo": "someRepo", + "style": IssueNameStyleNumeric, + "markupAllowShortIssuePattern": "true", } var alphanumericMetas = map[string]string{ - "format": "https://someurl.com/{user}/{repo}/{index}", - "user": "someUser", - "repo": "someRepo", - "style": IssueNameStyleAlphanumeric, + "format": "https://someurl.com/{user}/{repo}/{index}", + "user": "someUser", + "repo": "someRepo", + "style": IssueNameStyleAlphanumeric, + "markupAllowShortIssuePattern": "true", } var regexpMetas = map[string]string{ @@ -62,8 +64,15 @@ var regexpMetas = map[string]string{ // these values should match the TestOrgRepo const above var localMetas = map[string]string{ - "user": "test-owner", - "repo": "test-repo", + "user": "test-owner", + "repo": "test-repo", + "markupAllowShortIssuePattern": "true", +} + +var localWikiMetas = map[string]string{ + "user": "test-owner", + "repo": "test-repo", + "markupContentMode": "wiki", } func TestRender_IssueIndexPattern(t *testing.T) { @@ -124,9 +133,8 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, &RenderContext{ - Ctx: git.DefaultContext, - Metas: localMetas, - ContentMode: RenderContentAsComment, + Ctx: git.DefaultContext, + Metas: localMetas, }) class := "ref-issue" @@ -139,9 +147,8 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNum := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNum, &RenderContext{ - Ctx: git.DefaultContext, - Metas: numericMetas, - ContentMode: RenderContentAsComment, + Ctx: git.DefaultContext, + Metas: numericMetas, }) } @@ -262,7 +269,7 @@ func TestRender_IssueIndexPattern5(t *testing.T) { }) } -func TestRender_IssueIndexPattern_Document(t *testing.T) { +func TestRender_IssueIndexPattern_NoShortPattern(t *testing.T) { setting.AppURL = TestAppURL metas := map[string]string{ "format": "https://someurl.com/{user}/{repo}/{index}", @@ -285,6 +292,22 @@ func TestRender_IssueIndexPattern_Document(t *testing.T) { }) } +func TestRender_RenderIssueTitle(t *testing.T) { + setting.AppURL = TestAppURL + metas := map[string]string{ + "format": "https://someurl.com/{user}/{repo}/{index}", + "user": "someUser", + "repo": "someRepo", + "style": IssueNameStyleNumeric, + } + actual, err := RenderIssueTitle(&RenderContext{ + Ctx: git.DefaultContext, + Metas: metas, + }, "#1") + assert.NoError(t, err) + assert.Equal(t, "#1", actual) +} + func testRenderIssueIndexPattern(t *testing.T, input, expected string, ctx *RenderContext) { ctx.Links.AbsolutePrefix = true if ctx.Links.Base == "" { @@ -318,8 +341,7 @@ func TestRender_AutoLink(t *testing.T) { Links: Links{ Base: TestRepoURL, }, - Metas: localMetas, - ContentMode: RenderContentAsWiki, + Metas: localWikiMetas, }, strings.NewReader(input), &buffer) assert.Equal(t, err, nil) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String())) @@ -391,10 +413,10 @@ func TestRegExp_sha1CurrentPattern(t *testing.T) { } for _, testCase := range trueTestCases { - assert.True(t, hashCurrentPattern.MatchString(testCase)) + assert.True(t, globalVars().hashCurrentPattern.MatchString(testCase)) } for _, testCase := range falseTestCases { - assert.False(t, hashCurrentPattern.MatchString(testCase)) + assert.False(t, globalVars().hashCurrentPattern.MatchString(testCase)) } } @@ -474,9 +496,9 @@ func TestRegExp_shortLinkPattern(t *testing.T) { } for _, testCase := range trueTestCases { - assert.True(t, shortLinkPattern.MatchString(testCase)) + assert.True(t, globalVars().shortLinkPattern.MatchString(testCase)) } for _, testCase := range falseTestCases { - assert.False(t, shortLinkPattern.MatchString(testCase)) + assert.False(t, globalVars().shortLinkPattern.MatchString(testCase)) } } diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go index fa630656ce..2acf154ad2 100644 --- a/modules/markup/html_issue.go +++ b/modules/markup/html_issue.go @@ -7,6 +7,7 @@ import ( "strings" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/regexplru" @@ -23,18 +24,21 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { } next := node.NextSibling for node != nil && node != next { - m := getIssueFullPattern().FindStringSubmatchIndex(node.Data) + m := globalVars().issueFullPattern.FindStringSubmatchIndex(node.Data) if m == nil { return } - mDiffView := getFilesChangedFullPattern().FindStringSubmatchIndex(node.Data) + mDiffView := globalVars().filesChangedFullPattern.FindStringSubmatchIndex(node.Data) // leave it as it is if the link is from "Files Changed" tab in PR Diff View https://domain/org/repo/pulls/27/files if mDiffView != nil { return } link := node.Data[m[0]:m[1]] + if !httplib.IsCurrentGiteaSiteURL(ctx.Ctx, link) { + return + } text := "#" + node.Data[m[2]:m[3]] // if m[4] and m[5] is not -1, then link is to a comment // indicate that in the text by appending (comment) @@ -67,8 +71,10 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { return } - // crossLinkOnly if not comment and not wiki - crossLinkOnly := ctx.ContentMode != RenderContentAsTitle && ctx.ContentMode != RenderContentAsComment && ctx.ContentMode != RenderContentAsWiki + // crossLinkOnly: do not parse "#123", only parse "owner/repo#123" + // if there is no repo in the context, then the "#123" format can't be parsed + // old logic: crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki + crossLinkOnly := ctx.Metas["markupAllowShortIssuePattern"] != "true" var ( found bool diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index 30564da548..b7562d0aa6 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -20,9 +20,9 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu isAnchorFragment := link != "" && link[0] == '#' if !isAnchorFragment && !IsFullURLString(link) { linkBase := ctx.Links.Base - if ctx.ContentMode == RenderContentAsWiki { + if ctx.IsMarkupContentWiki() { // no need to check if the link should be resolved as a wiki link or a wiki raw link - // just use wiki link here and it will be redirected to a wiki raw link if necessary + // just use wiki link here, and it will be redirected to a wiki raw link if necessary linkBase = ctx.Links.WikiLink() } else if ctx.Links.BranchPath != "" || ctx.Links.TreePath != "" { // if there is no BranchPath, then the link will be something like "/owner/repo/src/{the-file-path}" @@ -40,7 +40,7 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu func shortLinkProcessor(ctx *RenderContext, node *html.Node) { next := node.NextSibling for node != nil && node != next { - m := shortLinkPattern.FindStringSubmatchIndex(node.Data) + m := globalVars().shortLinkPattern.FindStringSubmatchIndex(node.Data) if m == nil { return } @@ -147,7 +147,7 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) { } if image { if !absoluteLink { - link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), link) + link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), link) } title := props["title"] if title == "" { @@ -200,25 +200,6 @@ func linkProcessor(ctx *RenderContext, node *html.Node) { } } -func genDefaultLinkProcessor(defaultLink string) processor { - return func(ctx *RenderContext, node *html.Node) { - ch := &html.Node{ - Parent: node, - Type: html.TextNode, - Data: node.Data, - } - - node.Type = html.ElementNode - node.Data = "a" - node.DataAtom = atom.A - node.Attr = []html.Attribute{ - {Key: "href", Val: defaultLink}, - {Key: "class", Val: "default-link muted"}, - } - node.FirstChild, node.LastChild = ch, ch - } -} - // descriptionLinkProcessor creates links for DescriptionHTML func descriptionLinkProcessor(ctx *RenderContext, node *html.Node) { next := node.NextSibling diff --git a/modules/markup/html_node.go b/modules/markup/html_node.go index c499854053..234adba2bf 100644 --- a/modules/markup/html_node.go +++ b/modules/markup/html_node.go @@ -17,7 +17,7 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) { } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val) + attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), attr.Val) // By default, the "" tag should also be clickable, // because frontend use `` to paste the re-scaled image into the markdown, @@ -53,7 +53,7 @@ func visitNodeVideo(ctx *RenderContext, node *html.Node) (next *html.Node) { continue } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val) + attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), attr.Val) } attr.Val = camoHandleLink(attr.Val) node.Attr[i] = attr diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 262d0fc4dd..67ac2758a3 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -27,6 +27,11 @@ var ( "user": testRepoOwnerName, "repo": testRepoName, } + localWikiMetas = map[string]string{ + "user": testRepoOwnerName, + "repo": testRepoName, + "markupContentMode": "wiki", + } ) type mockRepo struct { @@ -413,8 +418,7 @@ func TestRender_ShortLinks(t *testing.T) { Links: markup.Links{ Base: markup.TestRepoURL, }, - Metas: localMetas, - ContentMode: markup.RenderContentAsWiki, + Metas: localWikiMetas, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) @@ -526,10 +530,9 @@ func TestRender_ShortLinks(t *testing.T) { func TestRender_RelativeMedias(t *testing.T) { render := func(input string, isWiki bool, links markup.Links) string { buffer, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: links, - Metas: localMetas, - ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsComment), + Ctx: git.DefaultContext, + Links: links, + Metas: util.Iif(isWiki, localWikiMetas, localMetas), }, input) assert.NoError(t, err) return strings.TrimSpace(string(buffer)) diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index c8488cfb50..c837b21e78 100644 --- a/modules/markup/markdown/goldmark.go +++ b/modules/markup/markdown/goldmark.go @@ -75,11 +75,12 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa // TODO: this was a quite unclear part, old code: `if metas["mode"] != "document" { use comment link break setting }` // many places render non-comment contents with no mode=document, then these contents also use comment's hard line break setting // especially in many tests. + markdownLineBreakStyle := ctx.Metas["markdownLineBreakStyle"] if markup.RenderBehaviorForTesting.ForceHardLineBreak { v.SetHardLineBreak(true) - } else if ctx.ContentMode == markup.RenderContentAsComment { + } else if markdownLineBreakStyle == "comment" { v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInComments) - } else { + } else if markdownLineBreakStyle == "document" { v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInDocuments) } } diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 315eed2e62..780df8727f 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -37,6 +37,12 @@ var localMetas = map[string]string{ "repo": testRepoName, } +var localWikiMetas = map[string]string{ + "user": testRepoOwnerName, + "repo": testRepoName, + "markupContentMode": "wiki", +} + type mockRepo struct { OwnerName string RepoName string @@ -75,7 +81,7 @@ func TestRender_StandardLinks(t *testing.T) { Links: markup.Links{ Base: FullURL, }, - ContentMode: markup.RenderContentAsWiki, + Metas: localWikiMetas, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) @@ -307,9 +313,8 @@ func TestTotal_RenderWiki(t *testing.T) { Links: markup.Links{ Base: FullURL, }, - Repo: newMockRepo(testRepoOwnerName, testRepoName), - Metas: localMetas, - ContentMode: markup.RenderContentAsWiki, + Repo: newMockRepo(testRepoOwnerName, testRepoName), + Metas: localWikiMetas, }, sameCases[i]) assert.NoError(t, err) assert.Equal(t, answers[i], string(line)) @@ -334,7 +339,7 @@ func TestTotal_RenderWiki(t *testing.T) { Links: markup.Links{ Base: FullURL, }, - ContentMode: markup.RenderContentAsWiki, + Metas: localWikiMetas, }, testCases[i]) assert.NoError(t, err) assert.EqualValues(t, testCases[i+1], string(line)) @@ -657,9 +662,9 @@ mail@domain.com remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -684,9 +689,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -713,9 +718,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -742,9 +747,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -771,9 +776,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -800,9 +805,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -830,9 +835,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -860,9 +865,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -890,9 +895,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -920,9 +925,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -951,9 +956,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -982,9 +987,9 @@ space

remote image
local image
remote link
-https://example.com/user/repo/compare/88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb#hash
+88fc37a3c0...12fc37a3c0 (hash)
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare
-https://example.com/user/repo/commit/88fc37a3c0a4dda553bdcfc80c178a58247f42fb
+88fc37a3c0
com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
👍
mail@domain.com
@@ -999,9 +1004,9 @@ space

defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() for i, c := range cases { result, err := markdown.RenderString(&markup.RenderContext{ - Ctx: context.Background(), - Links: c.Links, - ContentMode: util.Iif(c.IsWiki, markup.RenderContentAsWiki, markup.RenderContentAsDefault), + Ctx: context.Background(), + Links: c.Links, + Metas: util.Iif(c.IsWiki, map[string]string{"markupContentMode": "wiki"}, map[string]string{}), }, input) assert.NoError(t, err, "Unexpected error in testcase: %v", i) assert.Equal(t, c.Expected, string(result), "Unexpected result in testcase %v", i) diff --git a/modules/markup/markdown/transform_image.go b/modules/markup/markdown/transform_image.go index 4ed4118854..b2262c1c78 100644 --- a/modules/markup/markdown/transform_image.go +++ b/modules/markup/markdown/transform_image.go @@ -21,7 +21,7 @@ func (g *ASTTransformer) transformImage(ctx *markup.RenderContext, v *ast.Image) // Check if the destination is a real link if len(v.Destination) > 0 && !markup.IsFullURLBytes(v.Destination) { v.Destination = []byte(giteautil.URLJoin( - ctx.Links.ResolveMediaLink(ctx.ContentMode == markup.RenderContentAsWiki), + ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), strings.TrimLeft(string(v.Destination), "/"), )) } diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index 6b9c963157..c587a6ada5 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -144,15 +144,14 @@ func (r *Writer) resolveLink(kind, link string) string { } base := r.Ctx.Links.Base - isWiki := r.Ctx.ContentMode == markup.RenderContentAsWiki - if isWiki { + if r.Ctx.IsMarkupContentWiki() { base = r.Ctx.Links.WikiLink() } else if r.Ctx.Links.HasBranchInfo() { base = r.Ctx.Links.SrcLink() } if kind == "image" || kind == "video" { - base = r.Ctx.Links.ResolveMediaLink(isWiki) + base = r.Ctx.Links.ResolveMediaLink(r.Ctx.IsMarkupContentWiki()) } link = util.URLJoin(base, link) diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go index b882678c7e..a3eefc3db3 100644 --- a/modules/markup/orgmode/orgmode_test.go +++ b/modules/markup/orgmode/orgmode_test.go @@ -27,7 +27,7 @@ func TestRender_StandardLinks(t *testing.T) { Base: "/relative-path", BranchPath: "branch/main", }, - ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsDefault), + Metas: map[string]string{"markupContentMode": util.Iif(isWiki, "wiki", "")}, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) diff --git a/modules/markup/render.go b/modules/markup/render.go index add50f4382..1977dc73f5 100644 --- a/modules/markup/render.go +++ b/modules/markup/render.go @@ -27,15 +27,6 @@ const ( RenderMetaAsTable RenderMetaMode = "table" ) -type RenderContentMode string - -const ( - RenderContentAsDefault RenderContentMode = "" // empty means "default", no special handling, maybe just a simple "document" - RenderContentAsComment RenderContentMode = "comment" - RenderContentAsTitle RenderContentMode = "title" - RenderContentAsWiki RenderContentMode = "wiki" -) - var RenderBehaviorForTesting struct { // Markdown line break rendering has 2 default behaviors: // * Use hard: replace "\n" with "
" for comments, setting.Markdown.EnableHardLineBreakInComments=true @@ -59,12 +50,14 @@ type RenderContext struct { // for file mode, it could be left as empty, and will be detected by file extension in RelativePath MarkupType string - // what the content will be used for: eg: for comment or for wiki? or just render a file? - ContentMode RenderContentMode + Links Links // special link references for rendering, especially when there is a branch/tree path + + // user&repo, format&style®exp (for external issue pattern), teams&org (for mention) + // BranchNameSubURL (for iframe&asciicast) + // markupAllowShortIssuePattern, markupContentMode (wiki) + // markdownLineBreakStyle (comment, document) + Metas map[string]string - Links Links // special link references for rendering, especially when there is a branch/tree path - Metas map[string]string // user&repo, format&style®exp (for external issue pattern), teams&org (for mention), BranchNameSubURL(for iframe&asciicast) - DefaultLink string // TODO: need to figure out GitRepo *git.Repository Repo gitrepo.Repository ShaExistCache map[string]bool @@ -102,6 +95,10 @@ func (ctx *RenderContext) AddCancel(fn func()) { } } +func (ctx *RenderContext) IsMarkupContentWiki() bool { + return ctx.Metas != nil && ctx.Metas["markupContentMode"] == "wiki" +} + // Render renders markup file to HTML with all specific handling stuff. func Render(ctx *RenderContext, input io.Reader, output io.Writer) error { if ctx.MarkupType == "" && ctx.RelativePath != "" { @@ -232,3 +229,7 @@ func Init(ph *ProcessorHelper) { } } } + +func ComposeSimpleDocumentMetas() map[string]string { + return map[string]string{"markdownLineBreakStyle": "document"} +} diff --git a/modules/markup/render_links.go b/modules/markup/render_links.go index 3e1aa7ce3a..c8339d8f8b 100644 --- a/modules/markup/render_links.go +++ b/modules/markup/render_links.go @@ -10,7 +10,7 @@ import ( type Links struct { AbsolutePrefix bool // add absolute URL prefix to auto-resolved links like "#issue", but not for pre-provided links and medias - Base string // base prefix for pre-provided links and medias (images, videos) + Base string // base prefix for pre-provided links and medias (images, videos), usually it is the path to the repo BranchPath string // actually it is the ref path, eg: "branch/features/feat-12", "tag/v1.0" TreePath string // the dir of the file, eg: "doc" if the file "doc/CHANGE.md" is being rendered } diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index 1db1e4a111..8e443446bd 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -62,19 +62,18 @@ func (ut *RenderUtils) RenderCommitMessageLinkSubject(msg, urlDefault string, me } msgLine = strings.TrimRightFunc(msgLine, unicode.IsSpace) if len(msgLine) == 0 { - return template.HTML("") + return "" } // we can safely assume that it will not return any error, since there // shouldn't be any special HTML. renderedMessage, err := markup.RenderCommitMessageSubject(&markup.RenderContext{ - Ctx: ut.ctx, - DefaultLink: urlDefault, - Metas: metas, - }, template.HTMLEscapeString(msgLine)) + Ctx: ut.ctx, + Metas: metas, + }, urlDefault, template.HTMLEscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessageSubject: %v", err) - return template.HTML("") + return "" } return renderCodeBlock(template.HTML(renderedMessage)) } @@ -94,9 +93,8 @@ func (ut *RenderUtils) RenderCommitBody(msg string, metas map[string]string) tem } renderedMessage, err := markup.RenderCommitMessage(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - ContentMode: markup.RenderContentAsComment, + Ctx: ut.ctx, + Metas: metas, }, template.HTMLEscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessage: %v", err) @@ -117,9 +115,8 @@ func renderCodeBlock(htmlEscapedTextToRender template.HTML) template.HTML { // RenderIssueTitle renders issue/pull title with defined post processors func (ut *RenderUtils) RenderIssueTitle(text string, metas map[string]string) template.HTML { renderedText, err := markup.RenderIssueTitle(&markup.RenderContext{ - Ctx: ut.ctx, - ContentMode: markup.RenderContentAsTitle, - Metas: metas, + Ctx: ut.ctx, + Metas: metas, }, template.HTMLEscapeString(text)) if err != nil { log.Error("RenderIssueTitle: %v", err) @@ -212,7 +209,7 @@ func reactionToEmoji(reaction string) template.HTML { func (ut *RenderUtils) MarkdownToHtml(input string) template.HTML { //nolint:revive output, err := markdown.RenderString(&markup.RenderContext{ Ctx: ut.ctx, - Metas: map[string]string{"mode": "document"}, + Metas: markup.ComposeSimpleDocumentMetas(), }, input) if err != nil { log.Error("RenderString: %v", err) diff --git a/modules/templates/util_render_test.go b/modules/templates/util_render_test.go index 2d331b8317..529507e7ea 100644 --- a/modules/templates/util_render_test.go +++ b/modules/templates/util_render_test.go @@ -47,10 +47,11 @@ mail@domain.com } var testMetas = map[string]string{ - "user": "user13", - "repo": "repo11", - "repoPath": "../../tests/gitea-repositories-meta/user13/repo11.git/", - "mode": "comment", + "user": "user13", + "repo": "repo11", + "repoPath": "../../tests/gitea-repositories-meta/user13/repo11.git/", + "markdownLineBreakStyle": "comment", + "markupAllowShortIssuePattern": "true", } func TestMain(m *testing.M) { @@ -75,8 +76,7 @@ func newTestRenderUtils() *RenderUtils { func TestRenderCommitBody(t *testing.T) { defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() type args struct { - msg string - metas map[string]string + msg string } tests := []struct { name string @@ -108,7 +108,7 @@ func TestRenderCommitBody(t *testing.T) { ut := newTestRenderUtils() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equalf(t, tt.want, ut.RenderCommitBody(tt.args.msg, tt.args.metas), "RenderCommitBody(%v, %v)", tt.args.msg, tt.args.metas) + assert.Equalf(t, tt.want, ut.RenderCommitBody(tt.args.msg, nil), "RenderCommitBody(%v, %v)", tt.args.msg, nil) }) } @@ -140,7 +140,7 @@ func TestRenderCommitMessage(t *testing.T) { } func TestRenderCommitMessageLinkSubject(t *testing.T) { - expected := `space @mention-user` + expected := `space @mention-user` assert.EqualValues(t, expected, newTestRenderUtils().RenderCommitMessageLinkSubject(testInput(), "https://example.com/link", testMetas)) } @@ -164,11 +164,11 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit 👍 mail@domain.com @mention-user test -#123 +#123 space ` expected = strings.ReplaceAll(expected, "", " ") - assert.EqualValues(t, expected, string(newTestRenderUtils().RenderIssueTitle(testInput(), testMetas))) + assert.EqualValues(t, expected, string(newTestRenderUtils().RenderIssueTitle(testInput(), nil))) } func TestRenderMarkdownToHtml(t *testing.T) { diff --git a/routers/common/markup.go b/routers/common/markup.go index c8cc1a5ff1..dd6b286109 100644 --- a/routers/common/markup.go +++ b/routers/common/markup.go @@ -47,11 +47,12 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa switch mode { case "gfm": // legacy mode, do nothing case "comment": - renderCtx.ContentMode = markup.RenderContentAsComment + renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "comment"} case "wiki": - renderCtx.ContentMode = markup.RenderContentAsWiki + renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "document", "markupContentMode": "wiki"} case "file": // render the repo file content by its extension + renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "document"} renderCtx.MarkupType = "" renderCtx.RelativePath = filePath renderCtx.InStandalonePage = true @@ -74,10 +75,12 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa if repo != nil && repo.Repository != nil { renderCtx.Repo = repo.Repository - if renderCtx.ContentMode == markup.RenderContentAsComment { - renderCtx.Metas = repo.Repository.ComposeMetas(ctx) - } else { + if mode == "file" { renderCtx.Metas = repo.Repository.ComposeDocumentMetas(ctx) + } else if mode == "wiki" { + renderCtx.Metas = repo.Repository.ComposeWikiMetas(ctx) + } else if mode == "comment" { + renderCtx.Metas = repo.Repository.ComposeMetas(ctx) } } if err := markup.Render(renderCtx, strings.NewReader(text), ctx.Resp); err != nil { diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index a273515c8a..afc2c343a6 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -56,7 +56,7 @@ func renderMarkdown(ctx *context.Context, act *activities_model.Action, content Links: markup.Links{ Base: act.GetRepoLink(ctx), }, - Metas: map[string]string{ + Metas: map[string]string{ // FIXME: not right here, it should use issue to compose the metas "user": act.GetRepoUserName(ctx), "repo": act.GetRepoName(ctx), }, diff --git a/routers/web/feed/profile.go b/routers/web/feed/profile.go index 08cbcd9e12..6dd2d14cc6 100644 --- a/routers/web/feed/profile.go +++ b/routers/web/feed/profile.go @@ -46,9 +46,7 @@ func showUserFeed(ctx *context.Context, formatType string) { Links: markup.Links{ Base: ctx.ContextUser.HTMLURL(), }, - Metas: map[string]string{ - "user": ctx.ContextUser.GetDisplayName(), - }, + Metas: markup.ComposeSimpleDocumentMetas(), }, ctx.ContextUser.Description) if err != nil { ctx.ServerError("RenderString", err) diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 544f5362b8..18648d33cd 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -189,7 +189,7 @@ func prepareOrgProfileReadme(ctx *context.Context, viewRepositories bool) bool { Base: profileDbRepo.Link(), BranchPath: path.Join("branch", util.PathEscapeSegments(profileDbRepo.DefaultBranch)), }, - Metas: map[string]string{"mode": "document"}, + Metas: markup.ComposeSimpleDocumentMetas(), }, bytes); err != nil { log.Error("failed to RenderString: %v", err) } else { diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 2b8312f10a..13f6b69493 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -289,9 +289,8 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { } rctx := &markup.RenderContext{ - Ctx: ctx, - ContentMode: markup.RenderContentAsWiki, - Metas: ctx.Repo.Repository.ComposeDocumentMetas(ctx), + Ctx: ctx, + Metas: ctx.Repo.Repository.ComposeWikiMetas(ctx), Links: markup.Links{ Base: ctx.Repo.RepoLink, }, diff --git a/routers/web/shared/user/header.go b/routers/web/shared/user/header.go index ef111cff80..9467b0986b 100644 --- a/routers/web/shared/user/header.go +++ b/routers/web/shared/user/header.go @@ -50,7 +50,7 @@ func PrepareContextForProfileBigAvatar(ctx *context.Context) { ctx.Data["OpenIDs"] = openIDs if len(ctx.ContextUser.Description) != 0 { content, err := markdown.RenderString(&markup.RenderContext{ - Metas: map[string]string{"mode": "document"}, + Metas: markup.ComposeSimpleDocumentMetas(), Ctx: ctx, }, ctx.ContextUser.Description) if err != nil { diff --git a/templates/repo/view_file.tmpl b/templates/repo/view_file.tmpl index fb5bc14fe2..89bb371e7c 100644 --- a/templates/repo/view_file.tmpl +++ b/templates/repo/view_file.tmpl @@ -29,7 +29,7 @@
{{if .ReadmeInList}} {{svg "octicon-book" 16 "tw-mr-2"}} - {{.FileName}} + {{.FileName}} {{else}} {{template "repo/file_info" .}} {{end}} From c3dedcffa79acfb8be594124cc4554f1a7956d48 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 16 Nov 2024 09:52:16 -0800 Subject: [PATCH 9/9] Fix basic auth with webauthn (#32531) --- services/auth/basic.go | 10 ++++++ tests/integration/api_twofa_test.go | 53 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/services/auth/basic.go b/services/auth/basic.go index 90bd642370..1f6c3a442d 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -5,6 +5,7 @@ package auth import ( + "errors" "net/http" "strings" @@ -141,6 +142,15 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore } if skipper, ok := source.Cfg.(LocalTwoFASkipper); !ok || !skipper.IsSkipLocalTwoFA() { + // Check if the user has webAuthn registration + hasWebAuthn, err := auth_model.HasWebAuthnRegistrationsByUID(req.Context(), u.ID) + if err != nil { + return nil, err + } + if hasWebAuthn { + return nil, errors.New("Basic authorization is not allowed while webAuthn enrolled") + } + if err := validateTOTP(req, u); err != nil { return nil, err } diff --git a/tests/integration/api_twofa_test.go b/tests/integration/api_twofa_test.go index aad806b6dc..18e6fa91b7 100644 --- a/tests/integration/api_twofa_test.go +++ b/tests/integration/api_twofa_test.go @@ -53,3 +53,56 @@ func TestAPITwoFactor(t *testing.T) { req.Header.Set("X-Gitea-OTP", passcode) MakeRequest(t, req, http.StatusOK) } + +func TestBasicAuthWithWebAuthn(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // user1 has no webauthn enrolled, he can request API with basic auth + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + unittest.AssertNotExistsBean(t, &auth_model.WebAuthnCredential{UserID: user1.ID}) + req := NewRequest(t, "GET", "/api/v1/user") + req.SetBasicAuth(user1.Name, "password") + MakeRequest(t, req, http.StatusOK) + + // user1 has no webauthn enrolled, he can request git protocol with basic auth + req = NewRequest(t, "GET", "/user2/repo1/info/refs") + req.SetBasicAuth(user1.Name, "password") + MakeRequest(t, req, http.StatusOK) + + // user1 has no webauthn enrolled, he can request container package with basic auth + req = NewRequest(t, "GET", "/v2/token") + req.SetBasicAuth(user1.Name, "password") + resp := MakeRequest(t, req, http.StatusOK) + + type tokenResponse struct { + Token string `json:"token"` + } + var tokenParsed tokenResponse + DecodeJSON(t, resp, &tokenParsed) + assert.NotEmpty(t, tokenParsed.Token) + + // user32 has webauthn enrolled, he can't request API with basic auth + user32 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 32}) + unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{UserID: user32.ID}) + + req = NewRequest(t, "GET", "/api/v1/user") + req.SetBasicAuth(user32.Name, "notpassword") + resp = MakeRequest(t, req, http.StatusUnauthorized) + + type userResponse struct { + Message string `json:"message"` + } + var userParsed userResponse + DecodeJSON(t, resp, &userParsed) + assert.EqualValues(t, "Basic authorization is not allowed while webAuthn enrolled", userParsed.Message) + + // user32 has webauthn enrolled, he can't request git protocol with basic auth + req = NewRequest(t, "GET", "/user2/repo1/info/refs") + req.SetBasicAuth(user32.Name, "notpassword") + MakeRequest(t, req, http.StatusUnauthorized) + + // user32 has webauthn enrolled, he can't request container package with basic auth + req = NewRequest(t, "GET", "/v2/token") + req.SetBasicAuth(user1.Name, "notpassword") + MakeRequest(t, req, http.StatusUnauthorized) +}