From 36f3a51c6523ca4b296dadec66bf8080263932c1 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Fri, 31 Jul 2026 17:04:01 +0100 Subject: [PATCH] Detect content types when browsing files --- go.mod | 3 +- go.sum | 6 +- internal/server/browse.go | 128 +++++++++++++++---------- internal/server/browse_bench_test.go | 25 +++++ internal/server/browse_test.go | 138 ++++++++++++++++----------- 5 files changed, 190 insertions(+), 110 deletions(-) diff --git a/go.mod b/go.mod index 2f62065..e6e0a08 100644 --- a/go.mod +++ b/go.mod @@ -5,9 +5,10 @@ go 1.25.6 require ( github.com/BurntSushi/toml v1.6.0 github.com/CycloneDX/cyclonedx-go v0.11.0 - github.com/git-pkgs/archives v0.3.1 + github.com/git-pkgs/archives v0.4.0 github.com/git-pkgs/cooldown v0.1.1 github.com/git-pkgs/enrichment v0.6.4 + github.com/git-pkgs/magic v0.1.0 github.com/git-pkgs/purl v0.1.15 github.com/git-pkgs/registries v0.6.4 github.com/git-pkgs/spdx v0.1.4 diff --git a/go.sum b/go.sum index 4f356f9..c239240 100644 --- a/go.sum +++ b/go.sum @@ -244,12 +244,14 @@ github.com/fzipp/gocyclo v0.6.0 h1:lsblElZG7d3ALtGMx9fmxeTKZaLLpU8mET09yN4BBLo= github.com/fzipp/gocyclo v0.6.0/go.mod h1:rXPyn8fnlpa0R2csP/31uerbiVBugk5whMdlyaLkLoA= github.com/ghostiam/protogetter v0.3.20 h1:oW7OPFit2FxZOpmMRPP9FffU4uUpfeE/rEdE1f+MzD0= github.com/ghostiam/protogetter v0.3.20/go.mod h1:FjIu5Yfs6FT391m+Fjp3fbAYJ6rkL/J6ySpZBfnODuI= -github.com/git-pkgs/archives v0.3.1 h1:GKUuw++0YXAAElxweVHiR4AaSShKKYoVQmyxlF5blG4= -github.com/git-pkgs/archives v0.3.1/go.mod h1:408oQv3FxLCtePa33zp3sg3njXnwH74vnHZFxkRqoPo= +github.com/git-pkgs/archives v0.4.0 h1:KNmmIsLiSH27lUdT27EfUkQXFaLgXV5KezE81iyOIgo= +github.com/git-pkgs/archives v0.4.0/go.mod h1:tfio0OIuPKEBKHs/UCL5XBUvYmKpnvtnba2iDlfSd6g= github.com/git-pkgs/cooldown v0.1.1 h1:9OqqzCB8gANz/y44SmqGD0Jp8Qtu81D1sCbKl6Ehg7w= github.com/git-pkgs/cooldown v0.1.1/go.mod h1:v7APuK/UouTiu8mWQZbdDmj7DfxxkGUeuhjaRB5gv9E= github.com/git-pkgs/enrichment v0.6.4 h1:mGrfenttwmcUfPXRkWpB0wBJiiGj55ltniUh66Pq4bU= github.com/git-pkgs/enrichment v0.6.4/go.mod h1:zz1vPUak/w8Jhajll0KDRN2MjKaEYeCzQTxumWnVhqY= +github.com/git-pkgs/magic v0.1.0 h1:xLrqq7CMXB9g5bJnmJyKw17Rvlh0GFiEmO6e5RFsoeY= +github.com/git-pkgs/magic v0.1.0/go.mod h1:3ndidt+yvFaI1M0aEkkzkOlFnLPkeVQASIUojazcxCI= github.com/git-pkgs/packageurl-go v0.3.1 h1:WM3RBABQZLaRBxgKyYughc3cVBE8KyQxbSC6Jt5ak7M= github.com/git-pkgs/packageurl-go v0.3.1/go.mod h1:rcIxiG37BlQLB6FZfgdj9Fm7yjhRQd3l+5o7J0QPAk4= github.com/git-pkgs/pom v0.1.5 h1:TGT8Az2OMxGWsXnSagtUMGzZm7Oax8HrSCteA+mi0qY= diff --git a/internal/server/browse.go b/internal/server/browse.go index 504f5f1..3ea5676 100644 --- a/internal/server/browse.go +++ b/internal/server/browse.go @@ -1,6 +1,7 @@ package server import ( + "bufio" "encoding/json" "fmt" "io" @@ -10,29 +11,22 @@ import ( "github.com/git-pkgs/archives" "github.com/git-pkgs/archives/diff" + "github.com/git-pkgs/magic" "github.com/git-pkgs/proxy/internal/database" "github.com/git-pkgs/purl" "github.com/go-chi/chi/v5" ) -const contentTypePlainText = "text/plain; charset=utf-8" +const ( + contentTypePlainText = "text/plain; charset=utf-8" + browseSniffSize = 512 +) // maxBrowseArchiveSize caps how much data openArchive will buffer for // prefix detection. Artifacts larger than this are rejected to prevent // memory exhaustion from a single request. const maxBrowseArchiveSize = 512 << 20 // 512 MB -// archiveFilename returns a filename suitable for archive format detection. -// Some ecosystems (e.g. composer) store artifacts with bare hash filenames -// that have no extension. This adds .zip when the original has no extension -// and the content is likely a zip archive. -func archiveFilename(filename string) string { - if path.Ext(filename) == "" { - return filename + ".zip" - } - return filename -} - // detectSingleRootDir returns the single top-level directory name if all files // in the archive live under one common directory (e.g. GitHub zipballs use // "repo-hash/"). Returns "" if there's no single root or the archive is flat. @@ -66,8 +60,6 @@ func detectSingleRootDir(reader archives.Reader) string { // and stripping a single top-level directory prefix (like GitHub zipballs). // For npm, the hardcoded "package/" prefix takes precedence. func openArchive(filename string, content io.Reader, ecosystem string) (archives.Reader, error) { //nolint:ireturn // wraps multiple archive implementations - fname := archiveFilename(filename) - limited := io.LimitReader(content, maxBrowseArchiveSize+1) data, err := io.ReadAll(limited) if err != nil { @@ -78,17 +70,17 @@ func openArchive(filename string, content io.Reader, ecosystem string) (archives } if ecosystem == "npm" { - return archives.OpenBytesWithPrefix(fname, data, "package/") + return archives.OpenBytesWithPrefix(filename, data, "package/") } - probe, err := archives.OpenBytes(fname, data) + probe, err := archives.OpenBytes(filename, data) if err != nil { return nil, err } prefix := detectSingleRootDir(probe) _ = probe.Close() - return archives.OpenBytesWithPrefix(fname, data, prefix) + return archives.OpenBytesWithPrefix(filename, data, prefix) } // BrowseListResponse contains the file listing for a directory in an archives. @@ -361,7 +353,14 @@ func (s *Server) browseFile(w http.ResponseWriter, r *http.Request, ecosystem, n } defer func() { _ = fileReader.Close() }() - contentType := detectContentType(filePath) + contentType, knownPath := detectContentTypeFromPath(filePath) + var content io.Reader = fileReader + if !knownPath { + bufferedFile := bufio.NewReaderSize(fileReader, browseSniffSize) + prefix, _ := bufferedFile.Peek(browseSniffSize) + contentType = detectContentType(filePath, prefix) + content = bufferedFile + } w.Header().Set("Content-Type", contentType) w.Header().Set("Content-Security-Policy", "sandbox") w.Header().Set("X-Content-Type-Options", "nosniff") @@ -370,85 +369,112 @@ func (s *Server) browseFile(w http.ResponseWriter, r *http.Request, ecosystem, n w.Header().Set("Content-Disposition", fmt.Sprintf("inline; filename=%q", filename)) // Stream the file - _, _ = io.Copy(w, fileReader) + _, _ = io.Copy(w, content) } -// detectContentType returns an appropriate content type based on file extension. -func detectContentType(filename string) string { +// detectContentType returns an appropriate content type. Known filenames and +// extensions take precedence; content detection handles the remaining files. +func detectContentType(filename string, prefix []byte) string { + if contentType, ok := detectContentTypeFromPath(filename); ok { + return contentType + } + return detectContentTypeFromPrefix(prefix) +} + +func detectContentTypeFromPath(filename string) (string, bool) { ext := strings.ToLower(path.Ext(filename)) switch ext { // Text formats case ".txt", ".md", ".markdown": - return contentTypePlainText + return contentTypePlainText, true case ".html", ".htm", ".xhtml": - return contentTypePlainText + return contentTypePlainText, true case ".css": - return "text/css; charset=utf-8" + return "text/css; charset=utf-8", true case ".js", ".mjs": - return "application/javascript; charset=utf-8" + return "application/javascript; charset=utf-8", true case ".json": - return "application/json; charset=utf-8" + return "application/json; charset=utf-8", true case ".xml": - return "application/xml; charset=utf-8" + return "application/xml; charset=utf-8", true case ".yaml", ".yml": - return "text/yaml; charset=utf-8" + return "text/yaml; charset=utf-8", true case ".toml": - return "text/toml; charset=utf-8" + return "text/toml; charset=utf-8", true // Programming languages case ".go": - return "text/x-go; charset=utf-8" + return "text/x-go; charset=utf-8", true case ".rs": - return "text/x-rust; charset=utf-8" + return "text/x-rust; charset=utf-8", true case ".py": - return "text/x-python; charset=utf-8" + return "text/x-python; charset=utf-8", true case ".rb": - return "text/x-ruby; charset=utf-8" + return "text/x-ruby; charset=utf-8", true case ".java": - return "text/x-java; charset=utf-8" + return "text/x-java; charset=utf-8", true case ".c", ".h": - return "text/x-c; charset=utf-8" + return "text/x-c; charset=utf-8", true case ".cpp", ".cc", ".cxx", ".hpp": - return "text/x-c++; charset=utf-8" + return "text/x-c++; charset=utf-8", true case ".ts": - return "text/typescript; charset=utf-8" + return "text/typescript; charset=utf-8", true case ".tsx": - return "text/tsx; charset=utf-8" + return "text/tsx; charset=utf-8", true case ".jsx": - return "text/jsx; charset=utf-8" + return "text/jsx; charset=utf-8", true case ".php": - return "text/x-php; charset=utf-8" + return "text/x-php; charset=utf-8", true // Config files case ".conf", ".config", ".ini": - return contentTypePlainText + return contentTypePlainText, true case ".sh", ".bash": - return "text/x-shellscript; charset=utf-8" + return "text/x-shellscript; charset=utf-8", true case ".dockerfile": - return "text/x-dockerfile; charset=utf-8" + return "text/x-dockerfile; charset=utf-8", true // Images case ".png": - return "image/png" + return "image/png", true case ".jpg", ".jpeg": - return "image/jpeg" + return "image/jpeg", true case ".gif": - return "image/gif" + return "image/gif", true case ".svg": - return contentTypePlainText + return contentTypePlainText, true case ".ico": - return "image/x-icon" + return "image/x-icon", true // Archives case ".zip", ".tar", ".gz", ".bz2", ".xz": - return "application/octet-stream" + return "application/octet-stream", true default: - // Try to detect if it looks like text if isLikelyText(filename) { - return contentTypePlainText + return contentTypePlainText, true } + return "", false + } +} + +func detectContentTypeFromPrefix(prefix []byte) string { + result := magic.DetectPrefix(prefix) + if result.Kind == magic.KindText { + return contentTypePlainText + } + + switch result.Format { + case "png": + return "image/png" + case "jpeg": + return "image/jpeg" + case "gif": + return "image/gif" + case "pdf": + return "application/pdf" + default: return "application/octet-stream" } } diff --git a/internal/server/browse_bench_test.go b/internal/server/browse_bench_test.go index 03f3f02..16b2419 100644 --- a/internal/server/browse_bench_test.go +++ b/internal/server/browse_bench_test.go @@ -55,3 +55,28 @@ func BenchmarkOpenArchive(b *testing.B) { }) } } + +func BenchmarkDetectContentType(b *testing.B) { + cases := []struct { + name string + filename string + prefix []byte + }{ + {"known-path", "README.md", nil}, + {"text-prefix", "artifact", bytes.Repeat([]byte("a"), browseSniffSize)}, + {"png-prefix", "artifact", append([]byte("\x89PNG\r\n\x1a\n"), make([]byte, browseSniffSize-8)...)}, + } + + for _, tc := range cases { + b.Run(tc.name, func(b *testing.B) { + b.ReportAllocs() + var contentType string + for b.Loop() { + contentType = detectContentType(tc.filename, tc.prefix) + } + if contentType == "" { + b.Fatal("empty content type") + } + }) + } +} diff --git a/internal/server/browse_test.go b/internal/server/browse_test.go index f1fb993..6b1e487 100644 --- a/internal/server/browse_test.go +++ b/internal/server/browse_test.go @@ -137,29 +137,44 @@ func TestHandleBrowseFile(t *testing.T) { t.Fatalf("failed to upsert artifact: %v", err) } - // Test fetching a file - req := httptest.NewRequest("GET", "/ui/api/browse/npm/test-browse/1.0.0/file/README.md", nil) - w := httptest.NewRecorder() - ts.handler.ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("expected status 200, got %d: %s", w.Code, w.Body.String()) + files := []struct { + path string + content string + contentType string + }{ + {"README.md", "# Test Package\n", contentTypePlainText}, + {"notes.data", "short text\n", contentTypePlainText}, + {"logo", "\x89PNG\r\n\x1a\nimage data", "image/png"}, + {"page", "", contentTypePlainText}, + {"misleading.txt", "\x89PNG\r\n\x1a\nimage data", contentTypePlainText}, } + for _, file := range files { + t.Run(file.path, func(t *testing.T) { + req := httptest.NewRequest("GET", "/ui/api/browse/npm/test-browse/1.0.0/file/"+file.path, nil) + w := httptest.NewRecorder() + ts.handler.ServeHTTP(w, req) - body := w.Body.String() - if body != "# Test Package\n" { - t.Errorf("unexpected file content: %q", body) - } - - // Check content type - contentType := w.Header().Get("Content-Type") - if contentType != contentTypePlainText { - t.Errorf("expected text/plain content type, got %q", contentType) + if w.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", w.Code, w.Body.String()) + } + if w.Body.String() != file.content { + t.Errorf("unexpected file content: %q", w.Body.String()) + } + if got := w.Header().Get("Content-Type"); got != file.contentType { + t.Errorf("Content-Type = %q, want %q", got, file.contentType) + } + if got := w.Header().Get("Content-Security-Policy"); got != "sandbox" { + t.Errorf("Content-Security-Policy = %q, want sandbox", got) + } + if got := w.Header().Get("X-Content-Type-Options"); got != "nosniff" { + t.Errorf("X-Content-Type-Options = %q, want nosniff", got) + } + }) } // Test fetching non-existent file - req = httptest.NewRequest("GET", "/ui/api/browse/npm/test-browse/1.0.0/file/nonexistent.txt", nil) - w = httptest.NewRecorder() + req := httptest.NewRequest("GET", "/ui/api/browse/npm/test-browse/1.0.0/file/nonexistent.txt", nil) + w := httptest.NewRecorder() ts.handler.ServeHTTP(w, req) if w.Code != http.StatusNotFound { @@ -169,34 +184,47 @@ func TestHandleBrowseFile(t *testing.T) { func TestDetectContentType(t *testing.T) { tests := []struct { + name string filename string + prefix []byte expectedCT string }{ - {"file.txt", contentTypePlainText}, - {"file.md", contentTypePlainText}, - {"file.json", "application/json; charset=utf-8"}, - {"file.js", "application/javascript; charset=utf-8"}, - {"file.go", "text/x-go; charset=utf-8"}, - {"file.py", "text/x-python; charset=utf-8"}, - {"file.rs", "text/x-rust; charset=utf-8"}, - {"file.html", contentTypePlainText}, - {"file.htm", contentTypePlainText}, - {"file.xhtml", contentTypePlainText}, - {"file.svg", contentTypePlainText}, - {"file.png", "image/png"}, - {"file.jpg", "image/jpeg"}, - {"README", contentTypePlainText}, - {"LICENSE", contentTypePlainText}, - {"Makefile", contentTypePlainText}, - {".gitignore", contentTypePlainText}, - {"file.bin", "application/octet-stream"}, + {"text extension", "file.txt", nil, contentTypePlainText}, + {"markdown extension", "file.md", nil, contentTypePlainText}, + {"JSON extension", "file.json", nil, "application/json; charset=utf-8"}, + {"JavaScript extension", "file.js", nil, "application/javascript; charset=utf-8"}, + {"Go extension", "file.go", nil, "text/x-go; charset=utf-8"}, + {"Python extension", "file.py", nil, "text/x-python; charset=utf-8"}, + {"Rust extension", "file.rs", nil, "text/x-rust; charset=utf-8"}, + {"HTML extension", "file.html", nil, contentTypePlainText}, + {"HTM extension", "file.htm", nil, contentTypePlainText}, + {"XHTML extension", "file.xhtml", nil, contentTypePlainText}, + {"SVG extension", "file.svg", nil, contentTypePlainText}, + {"PNG extension", "file.png", nil, "image/png"}, + {"JPEG extension", "file.jpg", nil, "image/jpeg"}, + {"README", "README", nil, contentTypePlainText}, + {"LICENSE", "LICENSE", nil, contentTypePlainText}, + {"Makefile", "Makefile", nil, contentTypePlainText}, + {"gitignore", ".gitignore", nil, contentTypePlainText}, + {"unknown empty", "file.bin", nil, "application/octet-stream"}, + {"extensionless PNG", "asset", []byte("\x89PNG\r\n\x1a\n"), "image/png"}, + {"extensionless JPEG", "asset", []byte("\xff\xd8\xff"), "image/jpeg"}, + {"extensionless GIF", "asset", []byte("GIF89a"), "image/gif"}, + {"extensionless PDF", "asset", []byte("%PDF-1.7"), "application/pdf"}, + {"extensionless text", "asset", []byte("plain text\n"), contentTypePlainText}, + {"extensionless HTML", "asset", []byte(""), contentTypePlainText}, + {"extensionless XML", "asset", []byte(""), contentTypePlainText}, + {"extensionless SVG", "asset", []byte(""), contentTypePlainText}, + {"extensionless ZIP", "asset", []byte("PK\x03\x04"), "application/octet-stream"}, + {"extensionless binary", "asset", []byte{0, 1, 2}, "application/octet-stream"}, + {"known path wins", "file.txt", []byte("\x89PNG\r\n\x1a\n"), contentTypePlainText}, } for _, tt := range tests { - t.Run(tt.filename, func(t *testing.T) { - got := detectContentType(tt.filename) + t.Run(tt.name, func(t *testing.T) { + got := detectContentType(tt.filename, tt.prefix) if got != tt.expectedCT { - t.Errorf("detectContentType(%q) = %q, want %q", tt.filename, got, tt.expectedCT) + t.Errorf("detectContentType(%q, %q) = %q, want %q", tt.filename, tt.prefix, got, tt.expectedCT) } }) } @@ -255,6 +283,10 @@ func createTestArchive(t *testing.T) []byte { "package/lib/index.js": "module.exports = {};", "package/lib/helper.js": "module.exports.help = () => {};", "package/test/index.test.js": "// tests", + "package/notes.data": "short text\n", + "package/logo": "\x89PNG\r\n\x1a\nimage data", + "package/page": "", + "package/misleading.txt": "\x89PNG\r\n\x1a\nimage data", } for path, content := range files { @@ -609,25 +641,19 @@ func TestHandleComparePage(t *testing.T) { } } -func TestArchiveFilename(t *testing.T) { - tests := []struct { - input string - want string - }{ - {"package.tar.gz", "package.tar.gz"}, - {"d2e2f014ccd6ec9fae8dbe6336a4164346a2a856", "d2e2f014ccd6ec9fae8dbe6336a4164346a2a856.zip"}, - {"file.zip", "file.zip"}, - {"archive.tgz", "archive.tgz"}, - {"noext", "noext.zip"}, +func TestOpenArchiveDetectsExtensionlessTarGz(t *testing.T) { + reader, err := openArchive("artifact", bytes.NewReader(createTestArchive(t)), "npm") + if err != nil { + t.Fatalf("openArchive failed: %v", err) } + defer func() { _ = reader.Close() }() - for _, tt := range tests { - t.Run(tt.input, func(t *testing.T) { - got := archiveFilename(tt.input) - if got != tt.want { - t.Errorf("archiveFilename(%q) = %q, want %q", tt.input, got, tt.want) - } - }) + files, err := reader.List() + if err != nil { + t.Fatalf("List failed: %v", err) + } + if len(files) == 0 { + t.Fatal("expected files in extensionless archive") } }