Serve repo files as raw bytes over adminrpc — fixes corrupted images
/repo-raw served visibly broken gifs/images in production: the web frontend fetches file content through adminrpc, whose fileResult carries it as a JSON string, and encoding/json replaces invalid UTF-8 with U+FFFD when marshaling — silently corrupting any binary file. (GetRepoArchive already dodged this by using []byte, which travels as base64.) New GetRepoFileRaw/GetRepoFileRawAtRef on admin.Ops return []byte end to end; the blob and raw handlers now fetch through them (the blob view converts to string, which is lossless in Go). The string-based GetRepoFile stays for README/license callers, which are genuinely text. Includes an end-to-end regression test that commits a file with invalid UTF-8 and a NUL into a bare repo and asserts it round-trips the RPC socket byte-for-byte. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6 fichiers modifiés
+203 −61
M
cmd/gitfed-web/handlers_repo.go
+15 −12
M
internal/admin/admin.go
+17 −0
M
internal/adminrpc/adminrpc_test.go
+83 −0
M
internal/adminrpc/client.go
+12 −0
M
internal/adminrpc/protocol.go
+60 −49
M
internal/adminrpc/server.go
+16 −0
cmd/gitfed-web/handlers_repo.go
@@ -480,28 +480,30 @@ var blobTpl = newTpl("blob", `
// views share: validates access, 404s on an unknown branch or missing file,
// and returns the file's content plus the branch it was read from. ok is
// false for every "show a 404" case; a non-nil err is a real server error.
-func (s *server) repoFileForRequest(r *http.Request, name, path string) (content, branch string, ok bool, err error) {
+// Content comes back as []byte via the Raw ops — the string variants mangle
+// binary files over adminrpc (JSON strings can't carry invalid UTF-8).
+func (s *server) repoFileForRequest(r *http.Request, name, path string) (content []byte, branch string, ok bool, err error) {
if path == "" {
- return "", "", false, nil
+ return nil, "", false, nil
}
repo, err := s.ops.GetRepo(name)
if err != nil || !s.canView(r, repo) {
- return "", "", false, nil
+ return nil, "", false, nil
}
branch, _, err = s.ops.GetRepoBranch(name)
if err != nil {
- return "", "", false, err
+ return nil, "", false, err
}
onDefaultBranch := true
if requested := r.URL.Query().Get("branch"); requested != "" {
branches, err := s.ops.ListBranches(name)
if err != nil {
- return "", "", false, err
+ return nil, "", false, err
}
if !containsBranch(branches, requested) {
- return "", "", false, nil
+ return nil, "", false, nil
}
onDefaultBranch = requested == branch
branch = requested
@@ -509,12 +511,12 @@ func (s *server) repoFileForRequest(r *http.Request, name, path string) (content
var found bool
if onDefaultBranch {
- content, found, err = s.ops.GetRepoFile(name, path)
+ content, found, err = s.ops.GetRepoFileRaw(name, path)
} else {
- content, found, err = s.ops.GetRepoFileAtRef(name, branch, path)
+ content, found, err = s.ops.GetRepoFileRawAtRef(name, branch, path)
}
if err != nil {
- return "", "", false, err
+ return nil, "", false, err
}
return content, branch, found, nil
}
@@ -524,7 +526,7 @@ func (s *server) handleRepoBlob(w http.ResponseWriter, r *http.Request) {
path := strings.Trim(r.URL.Query().Get("path"), "/")
lang := s.lang(r)
- content, branch, ok, err := s.repoFileForRequest(r, name, path)
+ raw, branch, ok, err := s.repoFileForRequest(r, name, path)
if err != nil {
s.serverError(w, r, err)
return
@@ -533,6 +535,7 @@ func (s *server) handleRepoBlob(w http.ResponseWriter, r *http.Request) {
http.NotFound(w, r)
return
}
+ content := string(raw)
fileName, dir := path, ""
if i := strings.LastIndex(fileName, "/"); i >= 0 {
@@ -613,13 +616,13 @@ func (s *server) handleRepoRaw(w http.ResponseWriter, r *http.Request) {
switch ext := strings.ToLower(gopath.Ext(path)); {
case imageMIME[ext] != "":
w.Header().Set("Content-Type", imageMIME[ext])
- case strings.Contains(content, "\x00"):
+ case bytes.Contains(content, []byte{0}):
w.Header().Set("Content-Type", "application/octet-stream")
default:
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
}
w.Header().Set("Content-Length", strconv.Itoa(len(content)))
- _, _ = w.Write([]byte(content))
+ _, _ = w.Write(content)
}
// handleRepoArchive streams a tar.gz/zip snapshot of a branch or tag —
internal/admin/admin.go
@@ -65,6 +65,8 @@ type Ops interface {
ListRepoTreeAtRef(name, ref, path string) (entries []gitexec.TreeEntry, found bool, err error)
GetRepoFile(name, path string) (content string, found bool, err error)
GetRepoFileAtRef(name, ref, path string) (content string, found bool, err error)
+ GetRepoFileRaw(name, path string) (data []byte, found bool, err error)
+ GetRepoFileRawAtRef(name, ref, path string) (data []byte, found bool, err error)
GetRepoBranch(name string) (branch string, found bool, err error)
ListCommits(name string, limit int) (commits []gitexec.Commit, found bool, err error)
ListCommitsPage(name string, limit, offset int) (commits []gitexec.Commit, found bool, err error)
@@ -514,6 +516,21 @@ func (a *Admin) GetRepoFileAtRef(name, ref, path string) (string, bool, error) {
return gitexec.ReadFileAtRef(repo.Path, ref, path)
}
+// GetRepoFileRaw is GetRepoFile as []byte. Locally the two are byte-for-byte
+// identical (a Go string holds arbitrary bytes); the distinction matters over
+// adminrpc, where a JSON string mangles invalid UTF-8 but []byte travels as
+// base64 intact — binary files (the /repo-raw route's images) must use this.
+func (a *Admin) GetRepoFileRaw(name, path string) ([]byte, bool, error) {
+ content, found, err := a.GetRepoFile(name, path)
+ return []byte(content), found, err
+}
+
+// GetRepoFileRawAtRef is GetRepoFileAtRef as []byte — see GetRepoFileRaw.
+func (a *Admin) GetRepoFileRawAtRef(name, ref, path string) ([]byte, bool, error) {
+ content, found, err := a.GetRepoFileAtRef(name, ref, path)
+ return []byte(content), found, err
+}
+
func (a *Admin) GetRepoBranch(name string) (string, bool, error) {
repo, err := a.Store.GetRepo(name)
if err != nil {
internal/adminrpc/adminrpc_test.go
@@ -1,7 +1,10 @@
package adminrpc
import (
+ "bytes"
+ "os/exec"
"path/filepath"
+ "strings"
"testing"
"time"
@@ -60,3 +63,83 @@ func TestErrNotFoundSurvivesRPC(t *testing.T) {
t.Fatalf("got error %q (%T), want the exact store.ErrNotFound sentinel — identity was lost crossing the RPC boundary", err, err)
}
}
+
+// git runs a plumbing command against a bare repo and returns trimmed stdout.
+func gitPlumb(t *testing.T, gitDir string, stdin []byte, args ...string) string {
+ t.Helper()
+ cmd := exec.Command("git", append([]string{"--git-dir=" + gitDir}, args...)...)
+ if stdin != nil {
+ cmd.Stdin = bytes.NewReader(stdin)
+ }
+ var out, errBuf bytes.Buffer
+ cmd.Stdout = &out
+ cmd.Stderr = &errBuf
+ cmd.Env = append(cmd.Environ(),
+ "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t")
+ if err := cmd.Run(); err != nil {
+ t.Fatalf("git %v: %v: %s", args, err, errBuf.String())
+ }
+ return strings.TrimSpace(out.String())
+}
+
+// TestBinaryFileSurvivesRPC reproduces the corrupted-gif bug: fileResult
+// carries content as a JSON string, and encoding/json replaces invalid UTF-8
+// with U+FFFD — every image served by /repo-raw through the string-based
+// GetRepoFile arrived mangled and wouldn't render. GetRepoFileRaw carries
+// []byte (base64 on the wire) and must round-trip byte-for-byte.
+func TestBinaryFileSurvivesRPC(t *testing.T) {
+ dir := t.TempDir()
+ st, err := store.Open(filepath.Join(dir, "gitfed.db"))
+ if err != nil {
+ t.Fatalf("open store: %v", err)
+ }
+ defer st.Close()
+
+ localCA, err := ca.LoadOrCreate(filepath.Join(dir, "ca"))
+ if err != nil {
+ t.Fatalf("load CA: %v", err)
+ }
+ a := admin.New(st, federation.NewResolver(st, "local.test", true), "local.test", dir, localCA)
+ if err := a.CreateRepo("alice/demo", "alice"); err != nil {
+ t.Fatalf("create repo: %v", err)
+ }
+ repo, err := st.GetRepo("alice/demo")
+ if err != nil {
+ t.Fatalf("get repo: %v", err)
+ }
+
+ // A GIF89a header plus bytes that are not valid UTF-8 (0x89, 0xFF) and
+ // a NUL — exactly the kind of content a JSON string cannot carry.
+ binary := []byte("GIF89a\x00\x89\xff\xfe binary\x00tail")
+ blob := gitPlumb(t, repo.Path, binary, "hash-object", "-w", "--stdin")
+ tree := gitPlumb(t, repo.Path, []byte("100644 blob "+blob+"\tdemo.gif\n"), "mktree")
+ commit := gitPlumb(t, repo.Path, nil, "commit-tree", tree, "-m", "add gif")
+ gitPlumb(t, repo.Path, nil, "update-ref", "refs/heads/main", commit)
+
+ socketPath := filepath.Join(dir, "admin.sock")
+ srv := NewServer(a, socketPath)
+ go func() { _ = srv.ListenAndServe() }()
+
+ client := NewClient(socketPath)
+ deadline := time.Now().Add(2 * time.Second)
+ for {
+ if client.Ping() == nil {
+ break
+ }
+ if time.Now().After(deadline) {
+ t.Fatal("server never came up")
+ }
+ time.Sleep(10 * time.Millisecond)
+ }
+
+ data, found, err := client.GetRepoFileRaw("alice/demo", "demo.gif")
+ if err != nil {
+ t.Fatalf("GetRepoFileRaw: %v", err)
+ }
+ if !found {
+ t.Fatal("GetRepoFileRaw: file not found")
+ }
+ if !bytes.Equal(data, binary) {
+ t.Fatalf("binary content corrupted crossing the RPC boundary:\n got %q\nwant %q", data, binary)
+ }
+}
internal/adminrpc/client.go
@@ -247,6 +247,18 @@ func (c *Client) GetRepoFileAtRef(name, ref, path string) (string, bool, error)
return out.Content, out.Found, err
}
+func (c *Client) GetRepoFileRaw(name, path string) ([]byte, bool, error) {
+ var out fileRawResult
+ err := c.call(methodGetRepoFileRaw, pathArgs{Name: name, Path: path}, &out)
+ return out.Data, out.Found, err
+}
+
+func (c *Client) GetRepoFileRawAtRef(name, ref, path string) ([]byte, bool, error) {
+ var out fileRawResult
+ err := c.call(methodGetRepoFileRawAtRef, pathRefArgs{Name: name, Ref: ref, Path: path}, &out)
+ return out.Data, out.Found, err
+}
+
func (c *Client) GetRepoBranch(name string) (string, bool, error) {
var out branchResult
err := c.call(methodGetRepoBranch, nameArgs{Name: name}, &out)
internal/adminrpc/protocol.go
@@ -14,55 +14,57 @@ import (
// method names
const (
- methodListUsers = "ListUsers"
- methodCreateUser = "CreateUser"
- methodAddUserKey = "AddUserKey"
- methodRemoveUserKey = "RemoveUserKey"
- methodDeleteUser = "DeleteUser"
- methodListRepos = "ListRepos"
- methodGetRepo = "GetRepo"
- methodCreateRepo = "CreateRepo"
- methodImportRepo = "ImportRepo"
- methodDeleteRepo = "DeleteRepo"
- methodGetACL = "GetACL"
- methodGrantCollaborator = "GrantCollaborator"
- methodRevokeCollaborator = "RevokeCollaborator"
- methodListTrustedCAs = "ListTrustedCAs"
- methodCountPendingTrust = "CountPendingTrust"
- methodApproveDomain = "ApproveDomain"
- methodListAudit = "ListAudit"
- methodSetRepoPublic = "SetRepoPublic"
- methodSetRepoTopics = "SetRepoTopics"
- methodSetRepoDescription = "SetRepoDescription"
- methodGetRepoReadme = "GetRepoReadme"
- methodListReadmeLangs = "ListRepoReadmeLanguages"
- methodGetRepoReadmeLang = "GetRepoReadmeLang"
- methodGetRepoLicense = "GetRepoLicense"
- methodListRepoTags = "ListRepoTags"
- methodGetRepoArchive = "GetRepoArchive"
- methodListRepoTree = "ListRepoTree"
- methodListRepoTreeAtRef = "ListRepoTreeAtRef"
- methodGetRepoFile = "GetRepoFile"
- methodGetRepoFileAtRef = "GetRepoFileAtRef"
- methodGetRepoBranch = "GetRepoBranch"
- methodGetUser = "GetUser"
- methodSetUserAdmin = "SetUserAdmin"
- methodSetUserBio = "SetUserBio"
- methodSetPassword = "SetPassword"
- methodVerifyPassword = "VerifyPassword"
- methodCreateSession = "CreateSession"
- methodGetSession = "GetSession"
- methodDeleteSession = "DeleteSession"
- methodCheckAccess = "CheckAccess"
- methodListCommits = "ListCommits"
- methodListCommitsPage = "ListCommitsPage"
- methodCountCommits = "CountCommits"
- methodCountContributors = "CountContributors"
- methodDominantLanguage = "DominantLanguage"
- methodRepoDiskUsage = "RepoDiskUsage"
- methodShowCommit = "ShowCommit"
- methodCommitDiff = "CommitDiff"
- methodListBranches = "ListBranches"
+ methodListUsers = "ListUsers"
+ methodCreateUser = "CreateUser"
+ methodAddUserKey = "AddUserKey"
+ methodRemoveUserKey = "RemoveUserKey"
+ methodDeleteUser = "DeleteUser"
+ methodListRepos = "ListRepos"
+ methodGetRepo = "GetRepo"
+ methodCreateRepo = "CreateRepo"
+ methodImportRepo = "ImportRepo"
+ methodDeleteRepo = "DeleteRepo"
+ methodGetACL = "GetACL"
+ methodGrantCollaborator = "GrantCollaborator"
+ methodRevokeCollaborator = "RevokeCollaborator"
+ methodListTrustedCAs = "ListTrustedCAs"
+ methodCountPendingTrust = "CountPendingTrust"
+ methodApproveDomain = "ApproveDomain"
+ methodListAudit = "ListAudit"
+ methodSetRepoPublic = "SetRepoPublic"
+ methodSetRepoTopics = "SetRepoTopics"
+ methodSetRepoDescription = "SetRepoDescription"
+ methodGetRepoReadme = "GetRepoReadme"
+ methodListReadmeLangs = "ListRepoReadmeLanguages"
+ methodGetRepoReadmeLang = "GetRepoReadmeLang"
+ methodGetRepoLicense = "GetRepoLicense"
+ methodListRepoTags = "ListRepoTags"
+ methodGetRepoArchive = "GetRepoArchive"
+ methodListRepoTree = "ListRepoTree"
+ methodListRepoTreeAtRef = "ListRepoTreeAtRef"
+ methodGetRepoFile = "GetRepoFile"
+ methodGetRepoFileAtRef = "GetRepoFileAtRef"
+ methodGetRepoFileRaw = "GetRepoFileRaw"
+ methodGetRepoFileRawAtRef = "GetRepoFileRawAtRef"
+ methodGetRepoBranch = "GetRepoBranch"
+ methodGetUser = "GetUser"
+ methodSetUserAdmin = "SetUserAdmin"
+ methodSetUserBio = "SetUserBio"
+ methodSetPassword = "SetPassword"
+ methodVerifyPassword = "VerifyPassword"
+ methodCreateSession = "CreateSession"
+ methodGetSession = "GetSession"
+ methodDeleteSession = "DeleteSession"
+ methodCheckAccess = "CheckAccess"
+ methodListCommits = "ListCommits"
+ methodListCommitsPage = "ListCommitsPage"
+ methodCountCommits = "CountCommits"
+ methodCountContributors = "CountContributors"
+ methodDominantLanguage = "DominantLanguage"
+ methodRepoDiskUsage = "RepoDiskUsage"
+ methodShowCommit = "ShowCommit"
+ methodCommitDiff = "CommitDiff"
+ methodListBranches = "ListBranches"
methodCreateMergeRequest = "CreateMergeRequest"
methodListMergeRequests = "ListMergeRequests"
@@ -220,6 +222,15 @@ type fileResult struct {
Found bool `json:"found"`
}
+// fileRawResult carries file content as []byte (base64 on the wire, like
+// archiveResult) instead of a JSON string — encoding/json replaces invalid
+// UTF-8 in a string with U+FFFD, which silently corrupts binary files
+// (images served by /repo-raw were the visible casualty).
+type fileRawResult struct {
+ Data []byte `json:"data"`
+ Found bool `json:"found"`
+}
+
type branchResult struct {
Branch string `json:"branch"`
Found bool `json:"found"`
internal/adminrpc/server.go
@@ -275,6 +275,22 @@ func (s *Server) dispatch(req wireRequest) (any, error) {
content, found, err := s.ops.GetRepoFileAtRef(a.Name, a.Ref, a.Path)
return fileResult{Content: content, Found: found}, err
+ case methodGetRepoFileRaw:
+ var a pathArgs
+ if err := json.Unmarshal(req.Args, &a); err != nil {
+ return nil, err
+ }
+ data, found, err := s.ops.GetRepoFileRaw(a.Name, a.Path)
+ return fileRawResult{Data: data, Found: found}, err
+
+ case methodGetRepoFileRawAtRef:
+ var a pathRefArgs
+ if err := json.Unmarshal(req.Args, &a); err != nil {
+ return nil, err
+ }
+ data, found, err := s.ops.GetRepoFileRawAtRef(a.Name, a.Ref, a.Path)
+ return fileRawResult{Data: data, Found: found}, err
+
case methodGetRepoBranch:
var a nameArgs
if err := json.Unmarshal(req.Args, &a); err != nil {