From 149bcdd10e454ae432a7e1b1ce6babd1c371c2d2 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sat, 23 May 2026 10:10:27 -0500 Subject: [PATCH] appview/pages/markup: fix readme detection for directories and unsupported formats - IsReadmeFile now takes (name, mode) and only matches regular file blobs (filemode.Regular/Executable). A directory or symlink named "readme" was being picked up by tree handlers, which then tried to fetch its blob and 503'd. - ReadmePattern matches by convention (^readme(?:[._-].+)?$) rather than a fixed extension set. README.rst, README.org, README-old, etc. now surface on index/tree pages and fall through to FormatText in GetFormat for plaintext rendering. - pages.RepoIndex and pages.RepoTree route through markup.GetFormat instead of hardcoded extension switches, so the markdown extension list lives in one place (FileTypePatterns[FormatMarkdown]). - Added format_test.go covering IsReadmeFile (mode/symlink/dir rejection, the directory regression), FileTypePatterns, and GetFormat. Signed-off-by: Evan Jarrett --- appview/pages/markup/format.go | 24 ++++--- appview/pages/markup/format_test.go | 102 ++++++++++++++++++++++++++++ appview/pages/pages.go | 10 ++- appview/repo/index.go | 2 +- appview/repo/tree.go | 2 +- knotserver/xrpc/repo_tree.go | 2 +- 6 files changed, 125 insertions(+), 17 deletions(-) create mode 100644 appview/pages/markup/format_test.go diff --git a/appview/pages/markup/format.go b/appview/pages/markup/format.go index 82e1c206..e403f787 100644 --- a/appview/pages/markup/format.go +++ b/appview/pages/markup/format.go @@ -2,6 +2,8 @@ package markup import ( "regexp" + + "github.com/go-git/go-git/v5/plumbing/filemode" ) type Format string @@ -11,26 +13,32 @@ const ( FormatText Format = "text" ) -var FileTypes map[Format][]string = map[Format][]string{ - FormatMarkdown: {".md", ".markdown", ".mdown", ".mkdn", ".mkd"}, -} - var FileTypePatterns = map[Format]*regexp.Regexp{ FormatMarkdown: regexp.MustCompile(`(?i)\.(md|markdown|mdown|mkdn|mkd)$`), } -var ReadmePattern = regexp.MustCompile(`(?i)^readme(\.(md|markdown|txt))?$`) +var ReadmePattern = regexp.MustCompile(`(?i)^readme(?:[._-].+)?$`) -func IsReadmeFile(filename string) bool { - return ReadmePattern.MatchString(filename) +// IsReadmeFile reports whether name/mode identifies a readme blob. The git +// mode is checked so directories or symlinks named "readme" are filtered out. +func IsReadmeFile(name, mode string) bool { + if !ReadmePattern.MatchString(name) { + return false + } + m, err := filemode.New(mode) + if err != nil { + return false + } + return m == filemode.Regular || m == filemode.Executable } +// GetFormat returns the Format whose extension list matches filename, +// falling back to FormatText. func GetFormat(filename string) Format { for format, pattern := range FileTypePatterns { if pattern.MatchString(filename) { return format } } - // default format return FormatText } diff --git a/appview/pages/markup/format_test.go b/appview/pages/markup/format_test.go new file mode 100644 index 00000000..c2881b84 --- /dev/null +++ b/appview/pages/markup/format_test.go @@ -0,0 +1,102 @@ +package markup + +import "testing" + +func TestIsReadmeFile(t *testing.T) { + const ( + fileMode = "100644" + execMode = "100755" + dirMode = "040000" + ) + + cases := []struct { + name string + mode string + want bool + }{ + {"README.md", fileMode, true}, + {"readme.md", fileMode, true}, + {"ReadMe.MD", fileMode, true}, + {"README.markdown", fileMode, true}, + {"README.mdown", fileMode, true}, + {"README.mkdn", fileMode, true}, + {"README.mkd", fileMode, true}, + {"README.txt", fileMode, true}, + {"readme", fileMode, true}, + {"README", execMode, true}, + + // regression: a directory named "readme" must not be picked up + // as the README blob; tree handlers used to fetch it and 503. + {"readme", dirMode, false}, + {"README.md", dirMode, false}, + + // readme is matched by convention, not by renderable format — + // unsupported markup falls through to plaintext in GetFormat. + {"README.rst", fileMode, true}, + {"README.org", fileMode, true}, + {"readme-old", fileMode, true}, + {"readme_legacy", fileMode, true}, + + {"notreadme.md", fileMode, false}, + {"READMEISH", fileMode, false}, + {"README.md", "", false}, + {"README.md", "120000", false}, // symlink + } + + for _, c := range cases { + t.Run(c.name+"/"+c.mode, func(t *testing.T) { + if got := IsReadmeFile(c.name, c.mode); got != c.want { + t.Errorf("IsReadmeFile(%q, %q) = %v, want %v", c.name, c.mode, got, c.want) + } + }) + } +} + +func TestFileTypePatterns(t *testing.T) { + cases := []struct { + format Format + filename string + want bool + }{ + {FormatMarkdown, "x.md", true}, + {FormatMarkdown, "x.MARKDOWN", true}, + {FormatMarkdown, "x.mkdn", true}, + {FormatMarkdown, "x.mkd", true}, + {FormatMarkdown, "x.mdown", true}, + {FormatMarkdown, "x.txt", false}, + {FormatMarkdown, "x.rst", false}, + } + + for _, c := range cases { + t.Run(string(c.format)+"/"+c.filename, func(t *testing.T) { + p, ok := FileTypePatterns[c.format] + if !ok { + t.Fatalf("FileTypePatterns[%q] missing", c.format) + } + if got := p.MatchString(c.filename); got != c.want { + t.Errorf("FileTypePatterns[%q].MatchString(%q) = %v, want %v", c.format, c.filename, got, c.want) + } + }) + } +} + +func TestGetFormat(t *testing.T) { + cases := []struct { + filename string + want Format + }{ + {"x.md", FormatMarkdown}, + {"x.MARKDOWN", FormatMarkdown}, + {"x.txt", FormatText}, + {"x.rs", FormatText}, // unknown -> default + {"noext", FormatText}, + } + + for _, c := range cases { + t.Run(c.filename, func(t *testing.T) { + if got := GetFormat(c.filename); got != c.want { + t.Errorf("GetFormat(%q) = %q, want %q", c.filename, got, c.want) + } + }) + } +} diff --git a/appview/pages/pages.go b/appview/pages/pages.go index 20d4bfd1..13558305 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -867,9 +867,8 @@ func (p *Pages) RepoIndexPage(w io.Writer, params RepoIndexParams) error { rctx.RendererType = markup.RendererTypeRepoMarkdown if params.ReadmeFileName != "" { - ext := strings.ToLower(filepath.Ext(params.ReadmeFileName)) - switch ext { - case ".md", ".markdown", ".mdown", ".mkdn", ".mkd": + switch markup.GetFormat(params.ReadmeFileName) { + case markup.FormatMarkdown: params.Raw = false htmlString := rctx.RenderMarkdown(params.Readme) sanitized := rctx.SanitizeDefault(htmlString) @@ -961,9 +960,8 @@ func (p *Pages) RepoTree(w io.Writer, params RepoTreeParams) error { rctx.RendererType = markup.RendererTypeRepoMarkdown if params.ReadmeFileName != "" { - ext := strings.ToLower(filepath.Ext(params.ReadmeFileName)) - switch ext { - case ".md", ".markdown", ".mdown", ".mkdn", ".mkd": + switch markup.GetFormat(params.ReadmeFileName) { + case markup.FormatMarkdown: params.Raw = false htmlString := rctx.RenderMarkdown(params.Readme) sanitized := rctx.SanitizeDefault(htmlString) diff --git a/appview/repo/index.go b/appview/repo/index.go index c07efa88..bd9f6f09 100644 --- a/appview/repo/index.go +++ b/appview/repo/index.go @@ -272,7 +272,7 @@ func (rp *Repo) buildIndexResponse(ctx context.Context, repo *models.Repo, ref s treeResp = resp for _, file := range resp.Files { - if markup.IsReadmeFile(file.Name) { + if markup.IsReadmeFile(file.Name, file.Mode) { readmeFileName = file.Name break } diff --git a/appview/repo/tree.go b/appview/repo/tree.go index 8205e977..1b942a43 100644 --- a/appview/repo/tree.go +++ b/appview/repo/tree.go @@ -62,7 +62,7 @@ func (rp *Repo) Tree(w http.ResponseWriter, r *http.Request) { } } files[i] = file - if markup.IsReadmeFile(xrpcFile.Name) { + if markup.IsReadmeFile(xrpcFile.Name, xrpcFile.Mode) { readmeFile = xrpcFile } } diff --git a/knotserver/xrpc/repo_tree.go b/knotserver/xrpc/repo_tree.go index 296b5a62..df773b92 100644 --- a/knotserver/xrpc/repo_tree.go +++ b/knotserver/xrpc/repo_tree.go @@ -50,7 +50,7 @@ func (x *Xrpc) RepoTree(w http.ResponseWriter, r *http.Request) { var readmeFileName string var readmeContents string for _, file := range files { - if markup.IsReadmeFile(file.Name) { + if markup.IsReadmeFile(file.Name, file.Mode) { contents, err := gr.RawContent(filepath.Join(path, file.Name)) if err != nil { x.Logger.Error("failed to read contents of file", "path", path, "file", file.Name) -- 2.51.2