diff --git a/README.md b/README.md index ec00de7..6b760de 100644 --- a/README.md +++ b/README.md @@ -523,8 +523,10 @@ $ bdrive share wiki/report.html https://drive.example.com/s/eacc1df3ee6a6ebbdacc535c2796dc30 ``` -Links always serve the file's **latest** synced content (right for wiki -pages and living reports), and live until revoked — `bdrive share --list` +Links are **per-file** — there is no folder link, and `bdrive share` on a +folder says so and names a file inside it to share instead. Links always +serve the file's **latest** synced content (right for wiki pages and +living reports), and live until revoked — `bdrive share --list` and `--revoke ` manage them, `--expires 24h` makes one self-destruct. The web UI has a Share button on every file, and its dialog can put an expiry on the link it just minted (24 hours, 7 days, diff --git a/cmd/bdrive/share.go b/cmd/bdrive/share.go index 7072cbe..2f45d2e 100644 --- a/cmd/bdrive/share.go +++ b/cmd/bdrive/share.go @@ -93,6 +93,11 @@ Use --force to share anyway.`, if resp.StatusCode == http.StatusNotFound { return fmt.Errorf("%s (if you just saved it, wait a few seconds for the daemon or run `bdrive sync`)", strings.TrimSpace(readBody(resp))) } + // The server writes this one as a sentence to be read (e.g. "share + // links are per-file"); httpBodyError would prefix "400 Bad Request: ". + if resp.StatusCode == http.StatusBadRequest { + return fmt.Errorf("%s", strings.TrimSpace(readBody(resp))) + } // Before the generic fallthrough: httpBodyError would print the raw // JSON, and this is the one status the user can act on. if resp.StatusCode == http.StatusConflict { diff --git a/internal/webapp/cli_e2e_test.go b/internal/webapp/cli_e2e_test.go index 0b91728..9773a45 100644 --- a/internal/webapp/cli_e2e_test.go +++ b/internal/webapp/cli_e2e_test.go @@ -996,6 +996,57 @@ func TestCLIShareSecretGate(t *testing.T) { } } +// bdrive share used to say the folder wasn't synced yet, so users went +// looking for a sync fault that wasn't there — and the CLI bolted a "run bdrive +// sync" hint onto it. Folders now get the real answer, hint-free. +func TestCLIShareFolder(t *testing.T) { + e := newCLIEnv(t) + run := e.run + + work := t.TempDir() + if err := os.MkdirAll(filepath.Join(work, "notes"), 0o755); err != nil { + t.Fatal(err) + } + for _, f := range []string{"notes/readme.md", "notes/zeta.md"} { + if err := os.WriteFile(filepath.Join(work, filepath.FromSlash(f)), []byte("# "+f+"\n"), 0o644); err != nil { + t.Fatal(err) + } + } + if out, err := run(work, "init", "--name", "share-folder", "--yes"); err != nil { + t.Fatalf("init: %v\n%s", err, out) + } + defer run(work, "stop", work) + if out, err := run(work, "sync"); err != nil { + t.Fatalf("sync: %v\n%s", err, out) + } + + out, err := run(work, "share", "notes") + if err == nil { + t.Fatalf("share of a folder succeeded:\n%s", out) + } + for _, want := range []string{"per-file", "notes/readme.md"} { + if !strings.Contains(out, want) { + t.Fatalf("folder error missing %q:\n%s", want, out) + } + } + for _, unwanted := range []string{"wait a few seconds", "not synced", "400 Bad Request"} { + if strings.Contains(out, unwanted) { + t.Fatalf("folder error should not contain %q:\n%s", unwanted, out) + } + } + + // A path that really is missing keeps the sync-timing diagnosis and its hint. + out, err = run(work, "share", "missing.md") + if err == nil { + t.Fatalf("share of a missing file succeeded:\n%s", out) + } + for _, want := range []string{"not synced to this project yet", "wait a few seconds"} { + if !strings.Contains(out, want) { + t.Fatalf("missing-file error missing %q:\n%s", want, out) + } + } +} + // TestCLIStatusReportsUnscannedWork is BEA-106: with the daemon stopped, // `status` answered from the state cache and the journal — neither of which // has seen an edit nobody scanned — and reported the folder clean. A wrong diff --git a/internal/webapp/shares.go b/internal/webapp/shares.go index 1b97cb9..7a2bf27 100644 --- a/internal/webapp/shares.go +++ b/internal/webapp/shares.go @@ -239,6 +239,22 @@ func (db *ShareDB) List(project string) []Share { return out } +// firstFileUnder returns the lexicographically smallest file under the folder +// p, or "" if p is not a folder. The prefix is p+"/" and never bare p: "notes" +// is a prefix of "notes-archive/x.md", and that mistake turns a genuine +// "not synced" into a wrong "that's a folder". Smallest, not whatever map +// iteration hands back, so identical calls suggest the same file. +func firstFileUnder(files map[string]FileInfo, p string) string { + prefix := p + "/" + best := "" + for k := range files { + if strings.HasPrefix(k, prefix) && (best == "" || k < best) { + best = k + } + } + return best +} + // ---- HTTP ---- // handleShareCreate mints (or returns) the share link for a file. Any @@ -268,6 +284,13 @@ func (s *Server) handleShareCreate(v *volume, w http.ResponseWriter, r *http.Req return } if _, ok := snap.files[p]; !ok { + // snap.files maps FILES, so a fully synced folder misses here just like + // a path that does not exist. Tell those apart before answering, or the + // user goes off to fix a sync fault that isn't there. + if inside := firstFileUnder(snap.files, p); inside != "" { + http.Error(w, fmt.Sprintf("share links are per-file; %s is a folder - try a file inside it, e.g. %s", p, inside), http.StatusBadRequest) + return + } http.Error(w, fmt.Sprintf("%s is not synced to this project yet", p), http.StatusNotFound) return } diff --git a/internal/webapp/shares_test.go b/internal/webapp/shares_test.go index 11fa776..47c7ad6 100644 --- a/internal/webapp/shares_test.go +++ b/internal/webapp/shares_test.go @@ -902,3 +902,48 @@ func TestShareUnaffectedByAnUnmovedFile(t *testing.T) { t.Fatalf("unmoved share: %d %s", rec.Code, rec.Body) } } + +// A folder is never a key in the files map, so a fully synced folder used to +// 404 as "not synced yet" and send the user off to fix a sync fault that +// wasn't there. +func TestShareFolderIsNotASyncProblem(t *testing.T) { + srv, p, _, _, h := shareHub(t) + + // wiki/ is fully synced; sharing it is a per-file problem, not a timing one + req := jsonReq(t, "POST", "/api/p/"+p.ID+"/shares", map[string]string{"path": "wiki"}) + authAs(t, srv, req) + rec := doHTTP(h, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("share of a folder: %d, want 400", rec.Code) + } + body := rec.Body.String() + if !strings.Contains(body, "per-file") { + t.Fatalf("folder error must say share links are per-file, got %q", body) + } + // names a file inside, and the SMALLEST one so repeat calls agree + if !strings.Contains(body, "wiki/notes.md") { + t.Fatalf("folder error must name a file inside it, got %q", body) + } + if strings.Contains(body, "not synced") { + t.Fatalf("folder error must not blame sync, got %q", body) + } + if n := len(srv.Shares.List(p.ID)); n != 0 { + t.Fatalf("folder share minted %d links, want 0", n) + } + + // a path that really is unknown keeps today's answer + req = jsonReq(t, "POST", "/api/p/"+p.ID+"/shares", map[string]string{"path": "never-synced.md"}) + authAs(t, srv, req) + if rec := doHTTP(h, req); rec.Code != http.StatusNotFound || !strings.Contains(rec.Body.String(), "not synced to this project yet") { + t.Fatalf("unknown path: %d %s, want 404 not-synced", rec.Code, rec.Body) + } + + // "wik" is a PREFIX of wiki/report.html but is not a folder: still unknown. + // A bare HasPrefix(k, p) would call it a folder — the same wrong-diagnosis + // class of bug this fix is about. + req = jsonReq(t, "POST", "/api/p/"+p.ID+"/shares", map[string]string{"path": "wik"}) + authAs(t, srv, req) + if rec := doHTTP(h, req); rec.Code != http.StatusNotFound { + t.Fatalf("prefix-of-a-file path: %d, want 404", rec.Code) + } +} diff --git a/web/docs/src/content/docs/reference/cli.md b/web/docs/src/content/docs/reference/cli.md index 41c9293..d93ab63 100644 --- a/web/docs/src/content/docs/reference/cli.md +++ b/web/docs/src/content/docs/reference/cli.md @@ -21,7 +21,7 @@ One binary, `bdrive` — the CLI, the sync daemon, and the web server. | `bdrive stale [folder]` | Find synced markdown that links to a file written **after** the doc itself — staleness by what moved, not by the calendar. `-l` prints outgrown paths only, `-n` caps the docs printed (default 50, `0` = all). Pure read: no daemon, no lock, no network. Exit status is 0 whether or not anything is stale | | `bdrive forget ...` | Stop syncing a path and remove it from the hub. Adds the rule to `.bdriveignore` (which syncs) and prunes in one step. Local files are never touched, here or on teammates' devices | | `bdrive url [path]` | Internal hub link for a file or folder — sign-in and membership required. `--sync` pushes first; no argument gives the project home. Computed locally | -| `bdrive share ` | Public URL for a synced file. `--list`, `--revoke`, `--expires` (the hub's Share dialog can also set an expiry on an existing link). Refuses a file whose first 1 MiB holds credential-shaped strings — `--force` shares it anyway | +| `bdrive share ` | Public URL for a synced file — links are per-file, so a folder is refused with a file inside it named instead. `--list`, `--revoke`, `--expires` (the hub's Share dialog can also set an expiry on an existing link). Refuses a file whose first 1 MiB holds credential-shaped strings — `--force` shares it anyway | | `bdrive sync [folder]` | Run one sync cycle now. Refuses folders this device never `init`ed and folders paused by `bdrive stop`. `--note ` stamps session context onto changes; `--note-ttl` (default 30m) bounds it, and a plain `bdrive sync` with no `--note` clears it. `--prune` also removes from the hub what `.bdriveignore` now excludes (files stay on disk everywhere). `--hook