feat(archive): serve in-place instead of redirecting (preserves #anchor links)
Resolved `.archive/<tracking>.html` URLs now serve the target file's bytes inline via http.ServeFile with Cache-Control: no-cache, replacing the previous 302 redirect to the per-transmittal URL. Why: external links like `.archive/<tracking>.html#section` are meant to track the latest revision. A redirect exposes the snapshot URL — any forwarded link then pins to that snapshot instead of "latest." Serving in-place keeps the `.archive/` URL stable as the resolver's "current" target moves over time. Cache-Control: no-cache is intentional. Each load revalidates against the on-disk file's Last-Modified/ETag, so when a new revision lands the resolver picks it and the browser refetches transparently. ACL is unchanged: enforced on both the `.archive` context directory and the resolved target file (per-target denial returns 404, not 403, to avoid disclosing that a tracking number exists in a hidden subtree). archivehandler_test.go status expectations updated 302 → 200; fixture bodies adjusted for body-content verification of the in-place serve. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9ca36f25d8
commit
fe28a73f59
4 changed files with 80 additions and 35 deletions
|
|
@ -1425,8 +1425,16 @@ log is emitted with both paths so the conflict can be diagnosed and corrected.
|
||||||
| `GET /Project/sub/sub/.archive/TRK-001.html` | Same as the top-level Project listing — depth within a project doesn't change scope |
|
| `GET /Project/sub/sub/.archive/TRK-001.html` | Same as the top-level Project listing — depth within a project doesn't change scope |
|
||||||
| `GET /.archive/...` | **404** — root has no project segment |
|
| `GET /.archive/...` | **404** — root has no project segment |
|
||||||
|
|
||||||
All successful responses are `302 Found` redirects to the actual file URL. ACL
|
Successful `.html` responses **serve the resolved file's bytes inline** at the
|
||||||
is enforced on both the `.archive` context directory and the resolved target file.
|
`.archive/` URL — no `Location` redirect. The per-transmittal URL is hidden on
|
||||||
|
purpose: external links of the form `.archive/<tracking>.html#section` keep
|
||||||
|
tracking the latest revision. A redirect would expose the snapshot URL and any
|
||||||
|
forwarded link would pin to that snapshot instead of "latest." Cache-Control is
|
||||||
|
`no-cache` so each load revalidates against the on-disk file's
|
||||||
|
`Last-Modified`/`ETag`; when a new revision lands the resolver picks it and the
|
||||||
|
browser refetches. ACL is enforced on both the `.archive` context directory and
|
||||||
|
the resolved target file (per-target denial returns 404, not 403, to avoid
|
||||||
|
disclosing that the tracking number exists in a hidden subtree).
|
||||||
|
|
||||||
### Why "earliest" transmittal?
|
### Why "earliest" transmittal?
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,10 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
// Resolve parses the .archive request filename and returns the server-relative
|
// Resolve parses the .archive request filename and returns the server-relative
|
||||||
// redirect target URL (no leading slash) within the named project.
|
// path (no leading slash) of the resolved file within the named project. The
|
||||||
|
// caller serves that file in place — the .archive URL is intentionally stable
|
||||||
|
// across revisions so external links like .archive/<tracking>.html#section
|
||||||
|
// keep tracking the latest copy without exposing the per-transmittal URL.
|
||||||
//
|
//
|
||||||
// Project is the top-level segment of the .archive contextPath
|
// Project is the top-level segment of the .archive contextPath
|
||||||
// (/<project>/.../.archive/<filename>). An empty project — i.e. a request
|
// (/<project>/.../.archive/<filename>). An empty project — i.e. a request
|
||||||
|
|
|
||||||
|
|
@ -80,7 +80,20 @@ func ServeArchive(cfg config.Config, idx *archive.Index, w http.ResponseWriter,
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
http.Redirect(w, r, "/"+target, http.StatusFound)
|
// Serve the resolved file in place — DO NOT redirect. The .archive/
|
||||||
|
// URL is meant to be a stable forward-able link (people share
|
||||||
|
// `.archive/<tracking>.html#section` and expect that to keep tracking
|
||||||
|
// the latest revision). A redirect would expose the specific
|
||||||
|
// transmittal-folder URL, and any anchor/hash bookmarked from the
|
||||||
|
// browser bar would pin to that snapshot instead of "the latest."
|
||||||
|
//
|
||||||
|
// Cache-Control no-cache forces a conditional revalidation each
|
||||||
|
// load — http.ServeFile sets Last-Modified/ETag from the on-disk
|
||||||
|
// file, so when the resolver picks a newer target the ETag changes
|
||||||
|
// and the browser refetches.
|
||||||
|
absFile := filepath.Join(cfg.Root, filepath.FromSlash(target))
|
||||||
|
w.Header().Set("Cache-Control", "no-cache")
|
||||||
|
http.ServeFile(w, r, absFile)
|
||||||
}
|
}
|
||||||
|
|
||||||
// projectFromContextPath returns the first non-empty segment of the
|
// projectFromContextPath returns the first non-empty segment of the
|
||||||
|
|
|
||||||
|
|
@ -32,12 +32,15 @@ func archiveTestRoot(t *testing.T) (string, *archive.Index) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
root := t.TempDir()
|
root := t.TempDir()
|
||||||
|
|
||||||
|
// Write each fixture file's relative path as its content so the
|
||||||
|
// in-place .archive serve can be verified body-side (the resolver
|
||||||
|
// no longer issues a redirect — see archivehandler.go).
|
||||||
mk := func(rel string) {
|
mk := func(rel string) {
|
||||||
path := filepath.Join(root, filepath.FromSlash(rel))
|
path := filepath.Join(root, filepath.FromSlash(rel))
|
||||||
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||||
t.Fatalf("mkdir: %v", err)
|
t.Fatalf("mkdir: %v", err)
|
||||||
}
|
}
|
||||||
if err := os.WriteFile(path, []byte("x"), 0o644); err != nil {
|
if err := os.WriteFile(path, []byte(rel), 0o644); err != nil {
|
||||||
t.Fatalf("write %s: %v", path, err)
|
t.Fatalf("write %s: %v", path, err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -319,8 +322,8 @@ func TestServeArchive_ResolveACLDeniedReturns404(t *testing.T) {
|
||||||
// Alice in /ProjectA can resolve all of ProjectA's entries.
|
// Alice in /ProjectA can resolve all of ProjectA's entries.
|
||||||
for _, fn := range []string{"100.html", "100_A.html", "100_~A.html", "100_~A+C1.html"} {
|
for _, fn := range []string{"100.html", "100_A.html", "100_~A.html", "100_~A+C1.html"} {
|
||||||
rec := callArchive(t, cfg, idx, "alice@example.com", "/ProjectA", fn)
|
rec := callArchive(t, cfg, idx, "alice@example.com", "/ProjectA", fn)
|
||||||
if rec.Code != http.StatusFound {
|
if rec.Code != http.StatusOK {
|
||||||
t.Errorf("alice → /ProjectA/.archive/%s: status %d, want 302; body = %s", fn, rec.Code, rec.Body.String())
|
t.Errorf("alice → /ProjectA/.archive/%s: status %d, want 200; body = %s", fn, rec.Code, rec.Body.String())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -332,8 +335,8 @@ func TestServeArchive_ResolveACLDeniedReturns404(t *testing.T) {
|
||||||
|
|
||||||
// Bob has no denies — he can pull 200.html from /ProjectB.
|
// Bob has no denies — he can pull 200.html from /ProjectB.
|
||||||
rec = callArchive(t, cfg, idx, "bob@example.com", "/ProjectB", "200.html")
|
rec = callArchive(t, cfg, idx, "bob@example.com", "/ProjectB", "200.html")
|
||||||
if rec.Code != http.StatusFound {
|
if rec.Code != http.StatusOK {
|
||||||
t.Errorf("bob → /ProjectB/.archive/200.html: status %d, want 302", rec.Code)
|
t.Errorf("bob → /ProjectB/.archive/200.html: status %d, want 200", rec.Code)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -362,9 +365,9 @@ func TestServeArchive_CascadeDirectionsBothEnforced(t *testing.T) {
|
||||||
wantStatus int
|
wantStatus int
|
||||||
why string
|
why string
|
||||||
}{
|
}{
|
||||||
{"bob@example.com", "/ProjectA", "100.html", http.StatusFound, "bob allowed at root → reaches ProjectA target"},
|
{"bob@example.com", "/ProjectA", "100.html", http.StatusOK, "bob allowed at root → reaches ProjectA target"},
|
||||||
{"bob@example.com", "/ProjectB", "200.html", http.StatusFound, "bob allowed at root → reaches ProjectB target"},
|
{"bob@example.com", "/ProjectB", "200.html", http.StatusOK, "bob allowed at root → reaches ProjectB target"},
|
||||||
{"alice@example.com", "/ProjectA", "100.html", http.StatusFound, "alice rescued by ProjectA allow"},
|
{"alice@example.com", "/ProjectA", "100.html", http.StatusOK, "alice rescued by ProjectA allow"},
|
||||||
{"alice@example.com", "/ProjectB", "200.html", http.StatusForbidden, "alice not in ProjectB chain → 403 at contextPath"},
|
{"alice@example.com", "/ProjectB", "200.html", http.StatusForbidden, "alice not in ProjectB chain → 403 at contextPath"},
|
||||||
// mallory denied everywhere; the contextPath gate fires first.
|
// mallory denied everywhere; the contextPath gate fires first.
|
||||||
{"mallory@example.com", "/ProjectA", "100.html", http.StatusForbidden, "mallory blocked at contextPath"},
|
{"mallory@example.com", "/ProjectA", "100.html", http.StatusForbidden, "mallory blocked at contextPath"},
|
||||||
|
|
@ -379,32 +382,41 @@ func TestServeArchive_CascadeDirectionsBothEnforced(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Resolved redirect Location header is the absolute path to the actual file
|
// .archive serves the resolved file in place — the URL never changes.
|
||||||
// under cfg.Root. From any depth within the same project, the resolver
|
// From any depth within the same project the resolver picks the same
|
||||||
// returns the same target — `/ProjectA/.archive/100.html` and
|
// target file, so the bytes returned to the caller must be identical
|
||||||
// `/ProjectA/2025-01-01_T1 (IFR) - Title/.archive/100.html` 302 to the same
|
// across context paths (the per-revision file URL is intentionally
|
||||||
// file because both look up project ProjectA.
|
// hidden so external links remain stable).
|
||||||
func TestServeArchive_ResolveLocationStableAcrossDepthWithinProject(t *testing.T) {
|
func TestServeArchive_ServedBytesStableAcrossDepthWithinProject(t *testing.T) {
|
||||||
root, idx := archiveTestRoot(t)
|
root, idx := archiveTestRoot(t)
|
||||||
writeZddc(t, root, ".", `acl:
|
writeZddc(t, root, ".", `acl:
|
||||||
allow: ["*"]
|
allow: ["*"]
|
||||||
`)
|
`)
|
||||||
cfg := archiveCfg(root)
|
cfg := archiveCfg(root)
|
||||||
|
|
||||||
wantLocPrefix := "/ProjectA/2025-01-01_T1 (IFR) - Title/100_A"
|
wantBodyPrefix := "ProjectA/2025-01-01_T1 (IFR) - Title/100_A"
|
||||||
for _, ctx := range []string{
|
var firstBody string
|
||||||
|
for i, ctx := range []string{
|
||||||
"/ProjectA",
|
"/ProjectA",
|
||||||
"/ProjectA/2025-01-01_T1 (IFR) - Title",
|
"/ProjectA/2025-01-01_T1 (IFR) - Title",
|
||||||
"/ProjectA/2025-02-01_T2 (RTN) - Comments",
|
"/ProjectA/2025-02-01_T2 (RTN) - Comments",
|
||||||
} {
|
} {
|
||||||
rec := callArchive(t, cfg, idx, "alice@example.com", ctx, "100.html")
|
rec := callArchive(t, cfg, idx, "alice@example.com", ctx, "100.html")
|
||||||
if rec.Code != http.StatusFound {
|
if rec.Code != http.StatusOK {
|
||||||
t.Errorf("ctx=%s status=%d body=%s", ctx, rec.Code, rec.Body.String())
|
t.Errorf("ctx=%s status=%d body=%s", ctx, rec.Code, rec.Body.String())
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
loc := rec.Header().Get("Location")
|
if loc := rec.Header().Get("Location"); loc != "" {
|
||||||
if !strings.HasPrefix(loc, wantLocPrefix) {
|
t.Errorf("ctx=%s unexpected Location=%q (.archive must serve in place)", ctx, loc)
|
||||||
t.Errorf("ctx=%s Location=%q, want prefix %q", ctx, loc, wantLocPrefix)
|
}
|
||||||
|
body := rec.Body.String()
|
||||||
|
if !strings.HasPrefix(body, wantBodyPrefix) {
|
||||||
|
t.Errorf("ctx=%s body=%q, want prefix %q", ctx, body, wantBodyPrefix)
|
||||||
|
}
|
||||||
|
if i == 0 {
|
||||||
|
firstBody = body
|
||||||
|
} else if body != firstBody {
|
||||||
|
t.Errorf("ctx=%s body differs from first contextPath (resolver should pick the same target regardless of depth)", ctx)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -418,7 +430,7 @@ func TestServeArchive_CrossProjectSameTrackingNoLeak(t *testing.T) {
|
||||||
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||||
t.Fatalf("mkdir: %v", err)
|
t.Fatalf("mkdir: %v", err)
|
||||||
}
|
}
|
||||||
if err := os.WriteFile(path, []byte("x"), 0o644); err != nil {
|
if err := os.WriteFile(path, []byte(rel), 0o644); err != nil {
|
||||||
t.Fatalf("write %s: %v", path, err)
|
t.Fatalf("write %s: %v", path, err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -436,25 +448,34 @@ func TestServeArchive_CrossProjectSameTrackingNoLeak(t *testing.T) {
|
||||||
const email = "alice@example.com"
|
const email = "alice@example.com"
|
||||||
|
|
||||||
recA := callArchive(t, cfg, idx, email, "/ProjectA", "123.html")
|
recA := callArchive(t, cfg, idx, email, "/ProjectA", "123.html")
|
||||||
if recA.Code != http.StatusFound {
|
if recA.Code != http.StatusOK {
|
||||||
t.Fatalf("ProjectA 123.html status=%d body=%s", recA.Code, recA.Body.String())
|
t.Fatalf("ProjectA 123.html status=%d body=%s", recA.Code, recA.Body.String())
|
||||||
}
|
}
|
||||||
locA := recA.Header().Get("Location")
|
bodyA := recA.Body.String()
|
||||||
if !strings.HasPrefix(locA, "/ProjectA/") {
|
if !strings.HasPrefix(bodyA, "ProjectA/") {
|
||||||
t.Errorf("ProjectA Location=%q, want /ProjectA/ prefix", locA)
|
t.Errorf("ProjectA body=%q, want a ProjectA/ file's content", bodyA)
|
||||||
}
|
}
|
||||||
|
|
||||||
recB := callArchive(t, cfg, idx, email, "/ProjectB", "123.html")
|
recB := callArchive(t, cfg, idx, email, "/ProjectB", "123.html")
|
||||||
if recB.Code != http.StatusFound {
|
if recB.Code != http.StatusOK {
|
||||||
t.Fatalf("ProjectB 123.html status=%d body=%s", recB.Code, recB.Body.String())
|
t.Fatalf("ProjectB 123.html status=%d body=%s", recB.Code, recB.Body.String())
|
||||||
}
|
}
|
||||||
locB := recB.Header().Get("Location")
|
bodyB := recB.Body.String()
|
||||||
if !strings.HasPrefix(locB, "/ProjectB/") {
|
if !strings.HasPrefix(bodyB, "ProjectB/") {
|
||||||
t.Errorf("ProjectB Location=%q, want /ProjectB/ prefix", locB)
|
t.Errorf("ProjectB body=%q, want a ProjectB/ file's content", bodyB)
|
||||||
}
|
}
|
||||||
|
|
||||||
if locA == locB {
|
if bodyA == bodyB {
|
||||||
t.Errorf("cross-project leak: same Location for both projects: %q", locA)
|
t.Errorf("cross-project leak: same body served for both projects: %q", bodyA)
|
||||||
|
}
|
||||||
|
|
||||||
|
// URL must NOT have been rewritten — neither response carries a
|
||||||
|
// Location header. Stable .archive/ links are the whole point.
|
||||||
|
if loc := recA.Header().Get("Location"); loc != "" {
|
||||||
|
t.Errorf("ProjectA: unexpected Location header %q (.archive must serve in place)", loc)
|
||||||
|
}
|
||||||
|
if loc := recB.Header().Get("Location"); loc != "" {
|
||||||
|
t.Errorf("ProjectB: unexpected Location header %q (.archive must serve in place)", loc)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Listing each project shows only its own.
|
// Listing each project shows only its own.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue