From 6aef9e0a2f7df03893f00a657749b29e11ed839e Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 15 Mar 2023 17:51:39 -0400 Subject: [PATCH] Replace `repo.namedBlob` by `git.TreeEntry`. (#22898) `namedBlob` turned out to be a poor imitation of a `TreeEntry`. Using the latter directly shortens this code. This partially undoes https://github.com/go-gitea/gitea/pull/23152/, which I found a merge conflict with, and also expands the test it added to cover the subtle README-in-a-subfolder case. --- models/fixtures/repository.yml | 2 +- routers/web/repo/view.go | 82 ++++++++---------- .../16/633238d370a441f98dca532e4296a619c4c85f | Bin 0 -> 47 bytes .../46/49299398e4d39a5c09eb4f534df6f1e1eb87cc | 4 + .../refs/heads/sub-home-md-img-check | 1 + tests/integration/repo_test.go | 35 ++++++-- 6 files changed, 74 insertions(+), 50 deletions(-) create mode 100644 tests/gitea-repositories-meta/user2/repo1.git/objects/16/633238d370a441f98dca532e4296a619c4c85f create mode 100644 tests/gitea-repositories-meta/user2/repo1.git/objects/46/49299398e4d39a5c09eb4f534df6f1e1eb87cc create mode 100644 tests/gitea-repositories-meta/user2/repo1.git/refs/heads/sub-home-md-img-check diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 32ba8744d4522..4c888e47ed2c4 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -25,7 +25,7 @@ fork_id: 0 is_template: false template_id: 0 - size: 7028 + size: 7320 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 8663e11382085..4d49ab6359522 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -50,12 +50,6 @@ const ( tplMigrating base.TplName = "repo/migrate/migrating" ) -type namedBlob struct { - name string - isSymlink bool - blob *git.Blob -} - // locate a README for a tree in one of the supported paths. // // entries is passed to reduce calls to ListEntries(), so @@ -64,14 +58,14 @@ type namedBlob struct { // entries == ctx.Repo.Commit.SubTree(ctx.Repo.TreePath).ListEntries() // // FIXME: There has to be a more efficient way of doing this -func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (*namedBlob, error) { +func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (string, *git.TreeEntry, error) { // Create a list of extensions in priority order // 1. Markdown files - with and without localisation - e.g. README.en-us.md or README.md // 2. Txt files - e.g. README.txt // 3. No extension - e.g. README exts := append(localizedExtensions(".md", ctx.Language()), ".txt", "") // sorted by priority extCount := len(exts) - readmeFiles := make([]*namedBlob, extCount+1) + readmeFiles := make([]*git.TreeEntry, extCount+1) docsEntries := make([]*git.TreeEntry, 3) // (one of docs/, .gitea/ or .github/) for _, entry := range entries { @@ -98,28 +92,21 @@ func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (*n } if i, ok := util.IsReadmeFileExtension(entry.Name(), exts...); ok { log.Debug("Potential readme file: %s", entry.Name()) - if readmeFiles[i] == nil || base.NaturalSortLess(readmeFiles[i].name, entry.Blob().Name()) { - name := entry.Name() - isSymlink := entry.IsLink() - target := entry - if isSymlink { - var err error - target, err = entry.FollowLinks() + if readmeFiles[i] == nil || base.NaturalSortLess(readmeFiles[i].Name(), entry.Blob().Name()) { + if entry.IsLink() { + target, err := entry.FollowLinks() if err != nil && !git.IsErrBadLink(err) { - return nil, err - } - } - if target != nil && (target.IsExecutable() || target.IsRegular()) { - readmeFiles[i] = &namedBlob{ - name, - isSymlink, - target.Blob(), + return "", nil, err + } else if target != nil && (target.IsExecutable() || target.IsRegular()) { + readmeFiles[i] = entry } + } else { + readmeFiles[i] = entry } } } } - var readmeFile *namedBlob + var readmeFile *git.TreeEntry for _, f := range readmeFiles { if f != nil { readmeFile = f @@ -140,20 +127,20 @@ func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (*n var err error childEntries, err := subTree.ListEntries() if err != nil { - return nil, err + return "", nil, err } - readmeFile, err = findReadmeFileInEntries(ctx, childEntries) + + subfolder, readmeFile, err := findReadmeFileInEntries(ctx, childEntries) if err != nil && !git.IsErrNotExist(err) { - return nil, err + return "", nil, err } if readmeFile != nil { - readmeFile.name = subTreeEntry.Name() + "/" + readmeFile.name - break + return path.Join(subTreeEntry.Name(), subfolder), readmeFile, nil } } } - return readmeFile, nil + return "", readmeFile, nil } func renderDirectory(ctx *context.Context, treeLink string) { @@ -177,16 +164,13 @@ func renderDirectory(ctx *context.Context, treeLink string) { return } - readmeFile, err := findReadmeFileInEntries(ctx, entries) + subfolder, readmeFile, err := findReadmeFileInEntries(ctx, entries) if err != nil { ctx.ServerError("findReadmeFileInEntries", err) return } - if readmeFile == nil { - return - } - renderReadmeFile(ctx, readmeFile, fmt.Sprintf("%s/%s", treeLink, readmeFile.name)) + renderReadmeFile(ctx, subfolder, readmeFile, treeLink) } // localizedExtensions prepends the provided language code with and without a @@ -270,13 +254,23 @@ func getFileReader(repoID int64, blob *git.Blob) ([]byte, io.ReadCloser, *fileIn return buf, dataRc, &fileInfo{st.IsText(), true, meta.Size, &meta.Pointer, st}, nil } -func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelink string) { +func renderReadmeFile(ctx *context.Context, subfolder string, readmeFile *git.TreeEntry, readmeTreelink string) { + target := readmeFile + if readmeFile != nil && readmeFile.IsLink() { + target, _ = readmeFile.FollowLinks() + } + if target == nil { + // if findReadmeFile() failed and/or gave us a broken symlink (which it shouldn't) + // simply skip rendering the README + return + } + ctx.Data["RawFileLink"] = "" ctx.Data["ReadmeInList"] = true ctx.Data["ReadmeExist"] = true - ctx.Data["FileIsSymlink"] = readmeFile.isSymlink + ctx.Data["FileIsSymlink"] = readmeFile.IsLink() - buf, dataRc, fInfo, err := getFileReader(ctx.Repo.Repository.ID, readmeFile.blob) + buf, dataRc, fInfo, err := getFileReader(ctx.Repo.Repository.ID, target.Blob()) if err != nil { ctx.ServerError("getFileReader", err) return @@ -284,11 +278,11 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin defer dataRc.Close() ctx.Data["FileIsText"] = fInfo.isTextFile - ctx.Data["FileName"] = readmeFile.name + ctx.Data["FileName"] = path.Join(subfolder, readmeFile.Name()) ctx.Data["IsLFSFile"] = fInfo.isLFSFile if fInfo.isLFSFile { - filenameBase64 := base64.RawURLEncoding.EncodeToString([]byte(readmeFile.name)) + filenameBase64 := base64.RawURLEncoding.EncodeToString([]byte(readmeFile.Name())) ctx.Data["RawFileLink"] = fmt.Sprintf("%s.git/info/lfs/objects/%s/%s", ctx.Repo.Repository.Link(), url.PathEscape(fInfo.lfsMeta.Oid), url.PathEscape(filenameBase64)) } @@ -306,19 +300,19 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin rd := charset.ToUTF8WithFallbackReader(io.MultiReader(bytes.NewReader(buf), dataRc)) - if markupType := markup.Type(readmeFile.name); markupType != "" { + if markupType := markup.Type(readmeFile.Name()); markupType != "" { ctx.Data["IsMarkup"] = true ctx.Data["MarkupType"] = markupType ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{ Ctx: ctx, - RelativePath: path.Join(ctx.Repo.TreePath, readmeFile.name), // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path). - URLPrefix: path.Dir(readmeTreelink), + RelativePath: path.Join(ctx.Repo.TreePath, readmeFile.Name()), // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path). + URLPrefix: path.Join(readmeTreelink, subfolder), Metas: ctx.Repo.Repository.ComposeDocumentMetas(), GitRepo: ctx.Repo.GitRepo, }, rd) if err != nil { - log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.name, ctx.Repo.Repository, err) + log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err) buf := &bytes.Buffer{} ctx.Data["EscapeStatus"], _ = charset.EscapeControlStringReader(rd, buf, ctx.Locale) ctx.Data["FileContent"] = buf.String() diff --git a/tests/gitea-repositories-meta/user2/repo1.git/objects/16/633238d370a441f98dca532e4296a619c4c85f b/tests/gitea-repositories-meta/user2/repo1.git/objects/16/633238d370a441f98dca532e4296a619c4c85f new file mode 100644 index 0000000000000000000000000000000000000000..310f0ca77a4863dbf610877a415d61ce0564eee6 GIT binary patch literal 47 zcmV+~0MP$<0V^p=O;s>9WH2!R0)>?PI@D_!n`L^mhS ^ e] +3wunzr,].6ԋC$uMr +1zaI\ 㘺(>T6x:Oײ|u9~l"i$c kZ[S +SC;EvM!#G30ǘy] \ No newline at end of file diff --git a/tests/gitea-repositories-meta/user2/repo1.git/refs/heads/sub-home-md-img-check b/tests/gitea-repositories-meta/user2/repo1.git/refs/heads/sub-home-md-img-check new file mode 100644 index 0000000000000..dfe11055c1a09 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo1.git/refs/heads/sub-home-md-img-check @@ -0,0 +1 @@ +4649299398e4d39a5c09eb4f534df6f1e1eb87cc diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index cef9dccf24f60..40f5f56a35e26 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -362,7 +362,7 @@ func TestViewRepoDirectoryReadme(t *testing.T) { missing("symlink-loop", "/user2/readme-test/src/branch/symlink-loop/") } -func TestMarkDownImage(t *testing.T) { +func TestMarkDownReadmeImage(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") @@ -371,13 +371,38 @@ func TestMarkDownImage(t *testing.T) { resp := session.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) - _, exists := htmlDoc.doc.Find(`img[src="/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg"]`).Attr("src") - assert.True(t, exists, "Repo home page markdown image link check failed") + src, exists := htmlDoc.doc.Find(`.markdown img`).Attr("src") + assert.True(t, exists, "Image not found in README") + assert.Equal(t, src, "/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg") req = NewRequest(t, "GET", "/user2/repo1/src/branch/home-md-img-check/README.md") resp = session.MakeRequest(t, req, http.StatusOK) htmlDoc = NewHTMLParser(t, resp.Body) - _, exists = htmlDoc.doc.Find(`img[src="/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg"]`).Attr("src") - assert.True(t, exists, "Repo src page markdown image link check failed") + src, exists = htmlDoc.doc.Find(`.markdown img`).Attr("src") + assert.True(t, exists, "Image not found in markdown file") + assert.Equal(t, src, "/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg") +} + +func TestMarkDownReadmeImageSubfolder(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user2") + + // this branch has the README in the special docs/README.md location + req := NewRequest(t, "GET", "/user2/repo1/src/branch/sub-home-md-img-check") + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + src, exists := htmlDoc.doc.Find(`.markdown img`).Attr("src") + assert.True(t, exists, "Image not found in README") + assert.Equal(t, src, "/user2/repo1/media/branch/sub-home-md-img-check/docs/test-fake-img.jpg") + + req = NewRequest(t, "GET", "/user2/repo1/src/branch/sub-home-md-img-check/docs/README.md") + resp = session.MakeRequest(t, req, http.StatusOK) + + htmlDoc = NewHTMLParser(t, resp.Body) + src, exists = htmlDoc.doc.Find(`.markdown img`).Attr("src") + assert.True(t, exists, "Image not found in markdown file") + assert.Equal(t, src, "/user2/repo1/media/branch/sub-home-md-img-check/docs/test-fake-img.jpg") }