feat: many files per drop, uploaded in the background with a visible tray
Two changes the owner asked for, on top of the single-file drop. **N files become N documents.** Each is judged on its own: the good ones are stored, and every refusal is named individually with the reason that applies to it — "Stored 4 documents. 1 refused: scan.png is not text." One bad file no longer costs you the other nineteen, and nothing is dropped without being named. Per-file cap stays 2 MiB; a drop is capped at 20 MiB and 25 files, both refused up front rather than part-way through. All three numbers live in one place and every message and data attribute derives from them, with tests that fail if the advertised number and the enforced one ever disagree. This also removes a silent data loss: a file used to overwrite whatever you had typed in the paste box, with no word from the server. The paste box is now simply one more document, so there is nothing to discard and nothing to warn about. **Uploads happen in the background**, one XHR per file, three in flight, posted to the form's own action. A tray in the corner carries a row per file. Each row has two phases, and the second one is the point: progress events report BYTES SENT, so a fast link finishes the upload instantly and then sits while the server seals, chunks and indexes. A bar that reaches 100% and stops reads as broken, so phase two is indeterminate. It never invents a number for work it cannot measure. A lost answer is not a refusal. If the request gets no reply the row says "got no answer, so whether it was stored is unknown" and is counted separately as unconfirmed — claiming either outcome would be asserting something nobody checked. Verified by killing the server mid-upload. The scriptless path is untouched: form posts multipart, page reloads, same sentence. The tray and the flash cannot drift because both render the same Go strings, and a test compares them. This reverses a property an earlier review had approved — that the drop script has no upload path of its own. The guard was narrowed rather than deleted: fetch, sendBeacon, submit and requestSubmit stay banned, exactly one .open() is allowed, its address must be the endpoint variable, and that variable must be read from the form's own action. Reviewed by a model that did not write it: REQUEST CHANGES, three required, all fixed here. It replayed the mutations rather than trusting them, and two guards did not guard: - TestNoContentInLogs claimed logging a filename on the ingest-failure path would fail it. It did not: nothing in the test made Ingest fail, so that line never ran. It now closes the database under a running app, proves the failure path executed, and only then asserts the absence. - The endpoint guard could be walked past with `endpoint += '/../upload'` — the regex missed compound assignment and only saw double quotes. The third was a refusal the app never verified. Ingest refreshes the index after the transaction commits; if that failed, the document was stored and the message said "could not be stored". It is now counted as stored and says it will not be found by a search until this instance restarts. Fixing it exposed a second bug: the empty-submission branch would have said "there was nothing to store" about a committed row. Every test here was watched failing before it was trusted, and each says what was mutated. The copy has NOT been through its review gate yet, so this is not deployed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
10 files changed,
+3166
−363
internal/memoria/handlers.go+707 −129internal/memoria/lag_test.go+194 −0internal/memoria/static/drop.js+511 −95internal/memoria/static/memoria.css+87 −0internal/memoria/store.go+35 −2internal/memoria/templates/index.html+75 −14internal/memoriatest/drop_test.go+708 −113internal/memoriatest/gates_test.go+148 −5internal/memoriatest/harness_test.go+136 −5internal/memoriatest/tray_test.go+565 −0
diff --git a/internal/memoria/handlers.go b/internal/memoria/handlers.go| index b323588..21012f9 100644 |
| --- a/internal/memoria/handlers.go |
| +++ b/internal/memoria/handlers.go |
| @@ -1,25 +1,76 @@ |
| package memoria |
| import ( |
| + "encoding/json" |
| + "errors" |
| "io" |
| "log/slog" |
| + "mime/multipart" |
| "net/http" |
| "strconv" |
| "strings" |
| "github.com/carlosframework/rastrillo/auth" |
| "github.com/carlosframework/rastrillo/db" |
| + "github.com/carlosframework/rastrillo/flash" |
| "github.com/go-chi/chi/v5" |
| ) |
| -// maxUpload caps a pasted or uploaded document. |
| +// The three upload limits, and the ONE place any of them is written |
| +// down. Every sentence, every hint and every data attribute on the page |
| +// is derived from here — see uploadLimit, dropLimit and fileLimit just |
| +// below, and dropZone, which is what the page and the browser read. |
| // |
| -// 2 MiB of text is about 350,000 words — longer than any document anyone |
| -// has pasted into a box, and small enough that chunking and sealing it |
| -// stays inside a request. The number carries its reason here because |
| -// AGENTS.md requires that of a cap, and because the honest alternative |
| -// (a background job) is slice 3's problem, not slice 0's. |
| -const maxUpload = 2 << 20 |
| +// maxUpload caps ONE document, pasted or uploaded. 2 MiB of text is |
| +// about 350,000 words — longer than any document anyone has pasted into |
| +// a box, and small enough that chunking and sealing it stays inside a |
| +// request. The number carries its reason here because AGENTS.md requires |
| +// that of a cap, and because the honest alternative (a background job) |
| +// is slice 3's problem, not slice 0's. |
| +// |
| +// maxDropBytes and maxDropFiles cap a whole SUBMISSION, because one |
| +// submission is now several documents. The per-file cap alone would let |
| +// a drop of twenty-five 2 MiB files ask this handler to seal and index |
| +// 50 MiB inside a single request. Both submission caps are refused UP |
| +// FRONT, before anything at all is stored, so a drop is never |
| +// half-taken by a limit nobody was told about — the half-taken drop is |
| +// this codebase's worst failure shape, and a cap enforced part-way |
| +// through is how you build one. |
| +const ( |
| + maxUpload = 2 << 20 |
| + maxDropBytes = 20 << 20 |
| + maxDropFiles = 25 |
| + |
| + // dropSlack is multipart framing: boundaries, part headers, and the |
| + // title and body fields travelling alongside the files. It is added |
| + // to the BODY cap so a submission carrying exactly maxDropBytes of |
| + // file content still fits — and it is added to ParseMultipartForm's |
| + // memory threshold for a much sharper reason. See createDocument. |
| + dropSlack = 64 << 10 |
| + |
| + // maxInFlight caps how many of a drop's files the tray uploads at |
| + // once. It is the one number here the server does NOT enforce: it |
| + // is a courtesy, and the handler is as safe against twenty-five |
| + // simultaneous posts as against three. It is written here anyway, |
| + // beside the caps, because the browser reads all four off the page |
| + // and a number kept in the script is a number nobody reviewing this |
| + // file would find. |
| + // |
| + // Three, because these instances hibernate. Each upload wakes the |
| + // process, then seals, chunks and indexes inside its own request; |
| + // twenty-five of those at once on a cold instance is a queue nobody |
| + // can see, and the tray would sit at "Storing…" on every row. |
| + maxInFlight = 3 |
| +) |
| + |
| +// uploadLimit, dropLimit and fileLimit are the three caps said out loud, |
| +// and each is the ONLY place its number is put into words. Every refusal |
| +// and every hint reads them, because a refusal that names a different |
| +// number from the hint above it is a page arguing with itself and |
| +// neither line is obviously the wrong one. |
| +func uploadLimit() string { return strconv.Itoa(maxUpload>>20) + " MiB" } |
| +func dropLimit() string { return strconv.Itoa(maxDropBytes>>20) + " MiB" } |
| +func fileLimit() string { return strconv.Itoa(maxDropFiles) } |
| type app struct { |
| db *db.DB |
| @@ -53,72 +104,323 @@ type page struct { |
| Drop dropZone |
| } |
| -// dropZone is everything static/drop.js needs in order to refuse a |
| -// dropped file the way this handler would. It is rendered onto the drop |
| -// zone as data attributes and read from there, so the numbers and the |
| -// words exist once. |
| +// dropZone is the upload form's limits and ALL of its copy, in one |
| +// place. Every user-facing sentence this form can produce is a field |
| +// here: the hint above the input, what the browser says when it refuses |
| +// a dropped file before the post, and what the handler says afterwards |
| +// about what it stored and what it would not take. |
| // |
| -// Written down rather than duplicated in JavaScript because a client |
| -// check that has drifted from the server is worse than no client check: |
| -// it refuses what the server would have taken, or promises to take what |
| -// the server will refuse, and either way the page is lying about the |
| -// rule. The two numbers are the handler's own constants, and |
| -// TestTheAdvertisedCapIsTheHandlersCap and |
| -// TestTheAdvertisedSniffWindowIsTheHandlersWindow post files either side |
| -// of each rendered boundary to prove the handler still agrees with what |
| -// the page advertises. |
| +// Written down once rather than duplicated in JavaScript because a |
| +// client check that has drifted from the server is worse than no client |
| +// check: it refuses what the server would have taken, or promises to |
| +// take what the server will refuse, and either way the page is lying |
| +// about the rule. The browser's half is rendered onto the zone as data |
| +// attributes and read back from there. |
| // |
| // The browser's checks are a courtesy and never the guard. Every one of |
| // them is made again in createDocument, on bytes rather than on a File |
| // object, because a drop is not the only way to reach that handler. |
| +// |
| +// Placeholders are filled by fill(). {limit}, {total} and {max} are the |
| +// caps and are filled HERE, server-side, so neither the browser nor a |
| +// later reader can put a different number in. {name}, {names} and {n} |
| +// are filled by whichever side knows them: the browser for a file it is |
| +// refusing before the post, the handler for one it refused after. |
| type dropZone struct { |
| - MaxBytes int |
| - SniffBytes int |
| - Limit string |
| - TooBig string |
| - Binary string |
| - Empty string |
| - Many string |
| - Folder string |
| - Blocked string |
| + // The limits, published so the browser enforces the server's own |
| + // numbers. TestTheAdvertisedCapIsTheHandlersCap and its neighbours |
| + // post submissions either side of each rendered boundary. |
| + MaxBytes int |
| + MaxTotalBytes int |
| + MaxFiles int |
| + SniffBytes int |
| + |
| + // Labels and the hint under the input. |
| + TitleHint string |
| + FileLabel string |
| + Hint string |
| + |
| + // Whole-submission refusals: nothing is stored, and the sentence |
| + // says so. Both are checked in the browser AND again in the handler. |
| + TooManyFiles string |
| + TooMuchAtOnce string |
| + |
| + // Drop-only refusals. Neither can reach the handler — a folder never |
| + // becomes a part, and a drop the browser would not hand over never |
| + // becomes a request — so these have no server-side twin. |
| + Folder string |
| + Blocked string |
| + |
| + // MaxInFlight caps how many uploads the tray runs at once. It is a |
| + // courtesy to the instance, not a rule the server enforces — a |
| + // hibernating instance woken by twenty-five simultaneous seal-and- |
| + // index requests answers none of them quickly — so it is published |
| + // alongside the caps rather than written down in the script, for the |
| + // same reason they are: one place, and the page is where the browser |
| + // reads it. |
| + MaxInFlight int |
| + |
| + // ---- the tray ----------------------------------------------------- |
| + // |
| + // A drop no longer fills the form's input and wait for Store it: each |
| + // file is posted on its own, in the background, and the tray in the |
| + // corner is where its progress and its outcome are said. Every word |
| + // of that is here, for the reason the rest of this struct is here — |
| + // the tray and the scriptless flash must be incapable of saying |
| + // different things about the same file. |
| + |
| + // The tray's own furniture. |
| + TrayTitle string |
| + TrayDismiss string |
| + |
| + // The three states a row passes through, and only three, because |
| + // there are only three things that can honestly be said. Waiting: the |
| + // file is queued behind MaxInFlight others and nothing has been sent. |
| + // Uploading: {pct} is filled from the browser's own progress event, |
| + // which counts BYTES SENT — and is filled empty when the browser |
| + // declines to say, because a percentage nobody measured is a lie with |
| + // a number in it. Storing: the body is sent and the server is sealing, |
| + // chunking and indexing it, which is work no client can measure at |
| + // all, so the bar goes indeterminate and stops claiming anything. |
| + Waiting string |
| + Uploading string |
| + Storing string |
| + |
| + // A row's own result when its file landed. Short because the row has |
| + // already named the file above it; the refusal clauses below name it |
| + // again because they travel into the summary sentence too. |
| + RowStored string |
| + |
| + // Said once, when everything settles: the document list further down |
| + // the page was rendered before any of this and does not show it. The |
| + // page is NOT reloaded for you — someone mid-drop does not want the |
| + // ground moving — so the offer is a link and the choice is theirs. |
| + Stale string |
| + Reload string |
| + |
| + // The outcome sentence, both halves. The handler composes it after |
| + // storing; the browser composes the refusal half before posting, out |
| + // of the same strings — so a file refused in the page and the same |
| + // file refused by the server are refused in the same words. |
| + StoredOne string |
| + StoredMany string |
| + NoneStored string |
| + RefusedOne string |
| + // RefusedMany's {n} is the count of refusals, and it is the number |
| + // this codebase most needs to be right: a person told "4 stored" |
| + // when five went in has been silently truncated. |
| + RefusedMany string |
| + // RefusedMore only appears when the list of names is too long to |
| + // carry. The count in front of the list is still the true count, so |
| + // a cut-short list never implies it is complete. |
| + RefusedMore string |
| + |
| + // Unsure is the refusal head's twin, for files whose fate is unknown |
| + // rather than decided. It is a separate count on purpose: folding a |
| + // lost answer into "1 refused" would state, on the sentence written |
| + // to stop this app claiming things it has not checked, that a |
| + // document was rejected when it may be sitting in the database. |
| + UnsureOne string |
| + UnsureMany string |
| + |
| + // One reason clause per way a file can be refused. They compose into |
| + // a list: "scan.png is not text; notes.txt is empty". |
| + ReasonTooBig string |
| + ReasonBinary string |
| + ReasonEmpty string |
| + ReasonUnreadable string |
| + ReasonStore string |
| + // ReasonNoAnswer is the one this codebase would otherwise get wrong. |
| + // An upload that dies mid-flight — the network went, the instance |
| + // hibernated, a proxy answered with something that is not ours — is |
| + // NOT a refusal: the request may have been received, sealed and |
| + // stored in full, with only the answer lost. So the clause says the |
| + // one true thing, which is that nobody here knows. |
| + ReasonNoAnswer string |
| + // ReasonNotSearchable is not a refusal either, and it is the twin of |
| + // the mistake above rather than of the ones beside it. The document |
| + // committed and the index refresh after the commit failed, so the row |
| + // is in the database, in the list and in the byte count, and this |
| + // process cannot find it. Saying "could not be stored" about that — |
| + // which is what this handler used to say — is a refusal the app never |
| + // verified, on a document it has. It says the restart out loud because |
| + // the index is rebuilt from the sealed rows at every boot, so a |
| + // restart is the thing that actually fixes it and "yet" on its own |
| + // would promise a recovery that never comes. |
| + ReasonNotSearchable string |
| + |
| + // PasteName is what the paste box is called when it is the thing |
| + // being refused. The title someone typed is content and would then |
| + // travel in a cookie, so the box is named rather than its contents. |
| + PasteName string |
| + // Unnamed covers a part that arrives with no filename at all. It |
| + // cannot be left blank: " is not text" names nothing. |
| + Unnamed string |
| + |
| + // Form-level refusals, reached through /?err= codes. |
| + NothingSubmitted string |
| + Wrong string |
| } |
| -// newDropZone builds that. TooBig and Binary are flashFor's own strings: |
| -// a refusal made in the browser and the same refusal made by the handler |
| -// are then the same sentence, which is the point — a person who drops a |
| -// PDF and a person who picks one from the file chooser are told the same |
| -// thing. |
| +// newDropZone builds that. |
| func newDropZone() dropZone { |
| return dropZone{ |
| - MaxBytes: maxUpload, |
| - SniffBytes: sniffBytes, |
| - // Derived, so the hint cannot come to disagree with the cap it |
| - // describes — nor with the refusal, which says it in the same |
| + MaxBytes: maxUpload, |
| + MaxTotalBytes: maxDropBytes, |
| + MaxFiles: maxDropFiles, |
| + SniffBytes: sniffBytes, |
| + |
| + TitleHint: "(optional — it titles the pasted text; each file keeps its own name)", |
| + FileLabel: "…or upload text files", |
| + // Derived, so the hint cannot come to disagree with the caps it |
| + // describes — nor with the refusals, which say them in the same |
| // words from the same place. |
| - Limit: uploadLimit(), |
| - TooBig: flashFor("toobig"), |
| - Binary: flashFor("binary"), |
| - // Not flashFor("empty"): that sentence is about the form as a |
| - // whole ("the box was empty and no file was chosen"), and here |
| - // a file was very much chosen. |
| - Empty: "That file is empty, so there was nothing to store.", |
| - // "{n}" is filled in by the browser, which is the only place |
| - // that knows the count. |
| - // |
| - // Note what this does NOT say: not "only the first was taken". |
| - // The handler stores one document per submission and the input |
| - // is deliberately not `multiple`, so a drop of five files that |
| - // quietly kept one would be exactly the silent truncation this |
| - // codebase treats as its worst failure — a person would leave |
| - // believing five documents were stored. Nothing is taken, and |
| - // the count is said out loud. |
| - Many: "That drop had {n} files. memoria stores one document at a time, so none of them were taken — drop them one after another.", |
| - Folder: "That is a folder, and memoria stores one text file at a time. Drop a single file instead.", |
| + Hint: fill("…or drop them anywhere on this page. Up to {max} files at once, {limit} each and {total} in all.", |
| + "{max}", fileLimit(), "{limit}", uploadLimit(), "{total}", dropLimit()), |
| + |
| + TooManyFiles: fill("That drop had {n} files. memoria takes {max} at a time, so none of them were stored — drop them in smaller batches.", |
| + "{max}", fileLimit()), |
| + TooMuchAtOnce: fill("That drop was more than {total} altogether. memoria takes {total} at a time, so none of it was stored — drop fewer files at once.", |
| + "{total}", dropLimit()), |
| + |
| + Folder: "That is a folder, and memoria stores text files. Drop the files inside it instead.", |
| // The browsers that refuse this are old, but the failure is |
| - // silent — the file simply never arrives in the input — and a |
| + // silent — the files simply never arrive in the input — and a |
| // silent nothing is the one outcome that must not happen. |
| - Blocked: "This browser would not let a dropped file into the form, so nothing was taken. Choose the file above instead — it stores exactly the same way.", |
| + Blocked: "This browser would not let a dropped file into the form, so nothing was taken. Choose the files above instead.", |
| + |
| + MaxInFlight: maxInFlight, |
| + |
| + TrayTitle: "Uploads", |
| + TrayDismiss: "Dismiss", |
| + |
| + Waiting: "Waiting…", |
| + Uploading: "Uploading {pct}", |
| + Storing: "Storing…", |
| + RowStored: "Stored", |
| + |
| + Stale: "The document list below was drawn before this and does not show it yet.", |
| + Reload: "Reload the page", |
| + |
| + StoredOne: "Stored 1 document.", |
| + StoredMany: "Stored {n} documents.", |
| + NoneStored: "Nothing was stored.", |
| + RefusedOne: "1 refused", |
| + RefusedMany: "{n} refused", |
| + RefusedMore: "and {n} more", |
| + UnsureOne: "1 unconfirmed", |
| + UnsureMany: "{n} unconfirmed", |
| + |
| + ReasonTooBig: fill("{name} is larger than {limit}", "{limit}", uploadLimit()), |
| + ReasonBinary: "{name} is not text", |
| + ReasonEmpty: "{name} is empty", |
| + ReasonUnreadable: "{name} could not be read", |
| + ReasonStore: "{name} could not be stored", |
| + ReasonNoAnswer: "{name} got no answer, so whether it was stored is unknown", |
| + |
| + ReasonNotSearchable: "{name} is stored but will not be found by a search until this instance restarts", |
| + |
| + PasteName: "the pasted text", |
| + Unnamed: "a file with no name", |
| + |
| + NothingSubmitted: "There was nothing to store — the box was empty and no file was chosen.", |
| + Wrong: "That did not work.", |
| + } |
| +} |
| + |
| +// fill substitutes {placeholder} pairs. The joining punctuation lives in |
| +// the code that assembles a sentence, never inside a reviewed string. |
| +func fill(s string, pairs ...string) string { |
| + if len(pairs) == 0 { |
| + return s |
| + } |
| + return strings.NewReplacer(pairs...).Replace(s) |
| +} |
| + |
| +// storedPart is the first half of the outcome sentence. |
| +func (c dropZone) storedPart(n int) string { |
| + switch { |
| + case n == 1: |
| + return c.StoredOne |
| + case n > 1: |
| + return fill(c.StoredMany, "{n}", strconv.Itoa(n)) |
| + default: |
| + return c.NoneStored |
| + } |
| +} |
| + |
| +// refusedHead is the second half's opening, without the names. |
| +func (c dropZone) refusedHead(n int) string { |
| + if n == 1 { |
| + return c.RefusedOne |
| + } |
| + return fill(c.RefusedMany, "{n}", strconv.Itoa(n)) |
| +} |
| + |
| +// flashBudget is how many bytes of refusal clauses will be carried. |
| +// |
| +// The sentence travels in a cookie, and a cookie much over 4 KiB is |
| +// dropped by the browser without a word — which would take the names |
| +// with it. Past this point the list is cut short and says how much it |
| +// left out: a list that quietly stops is the same lie as a count that |
| +// quietly stops. |
| +const flashBudget = 2500 |
| + |
| +// fit cuts a list of clauses down to what is left of the flash budget, |
| +// and says how many it left out rather than stopping quietly. used is |
| +// carried across calls because one sentence can hold two lists and the |
| +// cookie holds them both. |
| +func (c dropZone) fit(clauses []string, used *int) []string { |
| + out := make([]string, 0, len(clauses)) |
| + for i, r := range clauses { |
| + if i > 0 && *used+len(r) > flashBudget { |
| + out = append(out, fill(c.RefusedMore, "{n}", strconv.Itoa(len(clauses)-i))) |
| + break |
| + } |
| + out = append(out, r) |
| + *used += len(r) + 2 |
| } |
| + return out |
| +} |
| + |
| +// outcome composes what a person is told after a submission: how many |
| +// documents were stored, and every refusal by name with its reason. |
| +// |
| +// It is one sentence and it always states both halves. "Stored 4 |
| +// documents." on its own, after five files went in, is the silent |
| +// truncation this codebase treats as its worst failure. |
| +// |
| +// lagging is the third thing that can be true of a file and it is |
| +// counted in stored, not against it: those documents ARE stored, so the |
| +// count is right, and each one is then named with what is wrong with it. |
| +// It carries no count of its own — every clause is a whole sentence |
| +// naming its own file, so there is no number here that could quietly |
| +// disagree with the list beside it. |
| +func (c dropZone) outcome(stored int, refused, lagging []string) (msg, kind string) { |
| + if len(refused) == 0 && len(lagging) == 0 { |
| + return c.storedPart(stored), "good" |
| + } |
| + used := 0 |
| + msg = c.storedPart(stored) |
| + if len(refused) > 0 { |
| + msg += " " + c.refusedHead(len(refused)) + ": " + strings.Join(c.fit(refused, &used), "; ") + "." |
| + } |
| + if len(lagging) > 0 { |
| + msg += " " + strings.Join(c.fit(lagging, &used), "; ") + "." |
| + } |
| + return msg, "danger" |
| +} |
| + |
| +// outcomeCounts is the same sentence with the names left out. It is what |
| +// the page falls back to when the flash cookie did not survive the |
| +// redirect: fewer facts, but never a friendlier set of them — the |
| +// refusal count is still stated, so a submission that lost some files |
| +// cannot render as a clean success. |
| +func (c dropZone) outcomeCounts(stored, refused int) (msg, kind string) { |
| + if refused == 0 { |
| + return c.storedPart(stored), "good" |
| + } |
| + return c.storedPart(stored) + " " + c.refusedHead(refused) + ".", "danger" |
| } |
| // chunkCount returns the indexed passage count, and -1 when it cannot be |
| @@ -139,12 +441,13 @@ func (a *app) chunkCount() int { |
| func (a *app) index(w http.ResponseWriter, r *http.Request) { |
| q := strings.TrimSpace(r.URL.Query().Get("q")) |
| + drop := newDropZone() |
| p := page{ |
| Signed: true, |
| Chunks: a.chunkCount(), |
| Query: q, |
| Searched: q != "", |
| - Drop: newDropZone(), |
| + Drop: drop, |
| } |
| if p.Searched { |
| results, err := a.store.Search(r.Context(), q, 25) |
| @@ -167,105 +470,380 @@ func (a *app) index(w http.ResponseWriter, r *http.Request) { |
| } |
| p.Documents = docs |
| if f := r.URL.Query().Get("stored"); f != "" { |
| - p.Flash, p.FlashKind = "Stored, chunked and indexed.", "good" |
| + p.Flash, p.FlashKind = drop.outcomeCounts(count(f), count(r.URL.Query().Get("refused"))) |
| } |
| if f := r.URL.Query().Get("err"); f != "" { |
| - p.Flash, p.FlashKind = flashFor(f), "danger" |
| + p.Flash, p.FlashKind = flashFor(f, count(r.URL.Query().Get("n"))), "danger" |
| + } |
| + // The cookie carries the sentence that NAMES things, so it wins over |
| + // the counts in the URL. It is taken unconditionally: Take clears it |
| + // whether or not this page uses it, and a one-shot notice that |
| + // outlives its own page is a notice about the wrong thing. |
| + // |
| + // A filename is content, and this cookie is the one place content is |
| + // allowed to travel outside the page body. It is HttpOnly, one-shot, |
| + // sixty seconds, and it goes only to the browser about to be shown |
| + // the same words on the same screen. It is never a log line, never |
| + // an error string, and never a URL — the redirects below carry |
| + // counts and nothing else. |
| + if fl, ok := flash.Take(w, r); ok && fl.Message != "" { |
| + p.Flash, p.FlashKind = fl.Message, fl.Kind |
| } |
| render(w, "index", p) |
| } |
| -// uploadLimit is maxUpload said out loud, and it is the ONLY place the |
| -// cap is put into words. flashFor's refusal and the drop zone's hint |
| -// both read it, because a refusal that names a different number from the |
| -// hint above it is a page arguing with itself and neither line is |
| -// obviously the wrong one. TestTheRefusalNamesTheCapItEnforces posts a |
| -// file over the cap and reads the number back off the refusal. |
| -func uploadLimit() string { return strconv.Itoa(maxUpload>>20) + " MiB" } |
| +// count reads a non-negative integer out of a query parameter. Anything |
| +// unparseable is 0, which is safe here: these are display counts, and |
| +// the sentence they build is chosen by the same numbers the handler |
| +// redirected with. |
| +func count(s string) int { |
| + n, err := strconv.Atoi(s) |
| + if err != nil || n < 0 { |
| + return 0 |
| + } |
| + return n |
| +} |
| -// flashFor maps an error code to copy. Codes travel in the URL rather |
| -// than messages, so nothing a person typed is reflected back into the |
| -// page or into a log line. |
| -func flashFor(code string) string { |
| +// flashFor maps an error code to copy. Codes and counts travel in the |
| +// URL rather than messages, so nothing a person typed and no filename is |
| +// reflected back into the page from there. |
| +// |
| +// Only whole-submission refusals reach this — the ones where nothing was |
| +// stored and no file needs naming, because either nothing was offered or |
| +// none of it was so much as opened. Everything per-file is composed by |
| +// outcome() and travels in the flash cookie. |
| +func flashFor(code string, n int) string { |
| + c := newDropZone() |
| switch code { |
| case "empty": |
| - return "There was nothing to store — the box was empty and no file was chosen." |
| - case "toobig": |
| - return "That is larger than " + uploadLimit() + ". Split it, or wait for background ingestion." |
| - case "binary": |
| - return "That looked like a binary file. memoria indexes text: .txt, .md, .csv." |
| + return c.NothingSubmitted |
| + case "toomany": |
| + return fill(c.TooManyFiles, "{n}", strconv.Itoa(n)) |
| + case "toototal": |
| + return c.TooMuchAtOnce |
| default: |
| - return "That did not work." |
| + return c.Wrong |
| } |
| } |
| -func (a *app) createDocument(w http.ResponseWriter, r *http.Request) { |
| - // Cap the BODY before parsing it. ParseMultipartForm's argument is a |
| - // memory threshold, not a limit: without this, a 3 MiB part parses |
| - // with err=nil and Go has already written the whole thing to a temp |
| - // file in PLAINTEXT — measured, canary present — and a 10 GB upload |
| - // fills the disk. The LimitReader below runs far too late to prevent |
| - // either. Found by a code review; the comment that used to be here |
| - // described a cap that did not exist. |
| - r.Body = http.MaxBytesReader(w, r.Body, maxUpload+(64<<10)) |
| - if err := r.ParseMultipartForm(maxUpload); err != nil { |
| - http.Redirect(w, r, "/?err=toobig", http.StatusSeeOther) |
| +// ---- the asynchronous answer ------------------------------------------ |
| + |
| +// uploadOutcome is what POST /documents answers when the caller asked for |
| +// JSON. It is the SAME judgement the HTML path makes — the same handler, |
| +// the same caps, the same NUL sniff, the same words — said in a shape the |
| +// tray can put beside one file. |
| +// |
| +// Message is the whole outcome sentence, composed by outcome(), so the |
| +// tray and the flash a scriptless browser sees cannot come to say |
| +// different things about the same file. Stored and Refused are the parts |
| +// that sentence was built from, because the tray has to place a refusal |
| +// against the row it belongs to, and a sentence cannot be taken apart |
| +// again once composed. |
| +// |
| +// What is NOT here: any identifier for the stored document. A drop of |
| +// twenty-five files would otherwise hand the browser twenty-five row ids |
| +// it has no use for, and the tray offers a reload instead. |
| +// |
| +// Lagging is the one that has to be carried separately rather than left |
| +// to the sentence. A file that committed and then failed to reach the |
| +// index is COUNTED IN Stored, because it is stored; if the tray had only |
| +// the count it would put the word for a clean success in that file's row |
| +// and drop the caveat on the floor — "stored" alone, which is the |
| +// confident half-truth this whole struct is arranged against. |
| +// |
| +// A filename can appear in Refused, in Lagging and in Message. That is |
| +// content, and it is allowed here for the same reason it is allowed in |
| +// the flash cookie: this body goes to the one browser that just sent the |
| +// file, is rendered on that person's own screen, and is never a log line, |
| +// an error string or a URL. |
| +type uploadOutcome struct { |
| + Stored int `json:"stored"` |
| + Refused []string `json:"refused"` |
| + Lagging []string `json:"lagging"` |
| + Message string `json:"message"` |
| + Kind string `json:"kind"` |
| +} |
| + |
| +// wantsJSON reports whether the caller asked for the tray's answer rather |
| +// than the browser's redirect. |
| +// |
| +// Deliberately narrow: it looks for an explicit application/json in |
| +// Accept, which a navigating browser never sends — Chrome's and Firefox's |
| +// navigation Accept headers offer html, xml, images and */*, and no JSON. |
| +// So a plain form submission cannot fall down this path by accident, and |
| +// the scriptless post keeps its 303 and its flash cookie. |
| +func wantsJSON(r *http.Request) bool { |
| + return strings.Contains(r.Header.Get("Accept"), "application/json") |
| +} |
| + |
| +// writeOutcome answers the JSON path. |
| +// |
| +// The status is 200 for every judgement, including one where nothing was |
| +// stored, because a refusal IS the answer to the question that was asked: |
| +// the request was received, read and judged, and the words for what |
| +// happened are in the body. That leaves a non-200 meaning exactly one |
| +// thing to the tray — no judgement arrived — which is the distinction the |
| +// row has to draw, since a lost answer is not a refusal and must never be |
| +// reported as one. |
| +func writeOutcome(w http.ResponseWriter, out uploadOutcome) { |
| + if out.Refused == nil { |
| + // An absent list and an empty list are the same fact, and the |
| + // tray should not have to know that JSON spells one of them |
| + // null. |
| + out.Refused = []string{} |
| + } |
| + if out.Lagging == nil { |
| + out.Lagging = []string{} |
| + } |
| + body, err := json.Marshal(out) |
| + if err != nil { |
| + // Nothing above can fail to marshal. If it somehow did, the |
| + // answer is a status with no words rather than half a body: the |
| + // tray reads a non-200 as "no answer" and says so, which is |
| + // true. The error itself carries no content. |
| + http.Error(w, "could not answer", http.StatusInternalServerError) |
| return |
| } |
| - title := strings.TrimSpace(r.FormValue("title")) |
| - body := r.FormValue("body") |
| - source := "paste" |
| - key := "" |
| - |
| - if f, hdr, err := r.FormFile("file"); err == nil { |
| - defer f.Close() |
| - // LimitReader at cap+1 so a file exactly at the cap passes and |
| - // one byte over is detectable, rather than silently truncated — |
| - // a truncated document indexes as a complete one, which is a |
| - // citation that lies. |
| - raw, err := io.ReadAll(io.LimitReader(f, maxUpload+1)) |
| - if err != nil { |
| - http.Redirect(w, r, "/?err=toobig", http.StatusSeeOther) |
| + w.Header().Set("Content-Type", "application/json; charset=utf-8") |
| + w.Header().Set("X-Content-Type-Options", "nosniff") |
| + w.WriteHeader(http.StatusOK) |
| + w.Write(body) |
| +} |
| + |
| +// createDocument stores one document per file, plus one more for the |
| +// paste box if anything was typed into it. |
| +// |
| +// Every file is judged on its own merits and the good ones are stored |
| +// even when their neighbours are refused. Each is its own Ingest, its |
| +// own transaction and its own post-commit index refresh, so a failure on |
| +// the third file leaves the first two stored and searchable. |
| +// |
| +// The person is then told both halves: how many documents landed, and |
| +// every refusal by name with the reason for that file. Storing some |
| +// while implying all is this codebase's worst failure shape; so is |
| +// refusing some without saying which. |
| +func (a *app) createDocument(w http.ResponseWriter, r *http.Request) { |
| + c := newDropZone() |
| + |
| + // Cap the BODY before parsing it, and give ParseMultipartForm a |
| + // memory threshold at least as large as the body can now be. |
| + // |
| + // The argument to ParseMultipartForm is a MEMORY threshold, not a |
| + // limit. Without the MaxBytesReader a 3 MiB part parsed with err=nil |
| + // and Go had already written the whole thing to a temp file in |
| + // PLAINTEXT — measured, canary present — and a 10 GB upload filled |
| + // the disk. Found by a code review; the comment that used to be here |
| + // described a cap that did not exist. |
| + // |
| + // Multi-file sharpens the second half of that trap, because the |
| + // threshold is spent CUMULATIVELY across parts: Go holds file parts |
| + // in memory until the running total passes it, then spills the rest |
| + // to disk. A threshold of maxUpload with a body of maxDropBytes |
| + // would write 18 MiB of somebody's documents to a temp file in the |
| + // clear on every large drop, with no error raised anywhere. So the |
| + // threshold is the whole body cap, and nothing is ever spilled. |
| + r.Body = http.MaxBytesReader(w, r.Body, maxDropBytes+dropSlack) |
| + if err := r.ParseMultipartForm(maxDropBytes + dropSlack); err != nil { |
| + var tooBig *http.MaxBytesError |
| + if errors.As(err, &tooBig) { |
| + a.refuse(w, r, "toototal", 0) |
| return |
| } |
| - if len(raw) > maxUpload { |
| - http.Redirect(w, r, "/?err=toobig", http.StatusSeeOther) |
| - return |
| + a.refuse(w, r, "malformed", 0) |
| + return |
| + } |
| + |
| + files := r.MultipartForm.File["file"] |
| + |
| + // Both submission caps are checked here, before anything is stored, |
| + // so the answer is "none of them were taken" rather than "the first |
| + // eleven were". |
| + if len(files) > maxDropFiles { |
| + a.refuse(w, r, "toomany", len(files)) |
| + return |
| + } |
| + total := 0 |
| + for _, hdr := range files { |
| + total += int(hdr.Size) |
| + } |
| + // The MaxBytesReader above refuses anything well over, but it caps |
| + // the BODY, framing included, and this caps the content. A drop |
| + // sitting between the two would otherwise be taken. |
| + if total > maxDropBytes { |
| + a.refuse(w, r, "toototal", 0) |
| + return |
| + } |
| + |
| + stored := 0 |
| + var refused []string |
| + // Stored, and not searchable in this process. Kept apart from |
| + // refused because the two need opposite sentences: see ErrIndexLag, |
| + // and see the clause itself in dropZone. |
| + var lagging []string |
| + |
| + for _, hdr := range files { |
| + // A filename is content. It is named to the person who dropped |
| + // it and nowhere else: never a log line, never an error string, |
| + // never the redirect URL. |
| + name := displayName(c, hdr.Filename) |
| + raw, reason := a.readUpload(c, hdr, name) |
| + if reason != "" { |
| + refused = append(refused, reason) |
| + continue |
| } |
| - if !looksLikeText(raw) { |
| - http.Redirect(w, r, "/?err=binary", http.StatusSeeOther) |
| - return |
| + // The filename is the stable key: re-uploading an edited file |
| + // updates one document instead of leaving its old self indexed |
| + // under the same title. It is the title too — with several files |
| + // in one submission there is no single title field that could |
| + // serve them all, and a title box that silently titled one of |
| + // five would be a worse answer than the filename. |
| + title := strings.TrimSuffix(hdr.Filename, filepathExt(hdr.Filename)) |
| + _, err := a.store.Ingest(r.Context(), "upload", hdr.Filename, title, string(raw)) |
| + switch { |
| + case errors.Is(err, ErrIndexLag): |
| + // The row committed and the index did not take it. Counted as |
| + // stored, because it is, and then named — "could not be |
| + // stored" about a document that is in the list is a refusal |
| + // this app never verified. |
| + a.logger.Error("index refresh failed after commit", "source", "upload", "err", err) |
| + stored++ |
| + lagging = append(lagging, fill(c.ReasonNotSearchable, "{name}", name)) |
| + case err != nil: |
| + // No document content in the log line — AGENTS.md's rule, |
| + // and the reason this carries no title and no filename. |
| + a.logger.Error("ingest failed", "source", "upload", "bytes", len(raw), "err", err) |
| + refused = append(refused, fill(c.ReasonStore, "{name}", name)) |
| + default: |
| + stored++ |
| } |
| - body = string(raw) |
| - source = "upload" |
| - if hdr != nil { |
| - // The filename is the stable key: re-uploading an edited |
| - // file updates one document instead of leaving its old self |
| - // indexed under the same title. |
| - key = hdr.Filename |
| - if title == "" { |
| - title = strings.TrimSuffix(hdr.Filename, filepathExt(hdr.Filename)) |
| + } |
| + |
| + // The paste box is one more document, not something a file |
| + // overwrites. It used to be replaced by any uploaded file without a |
| + // word: real data loss, and the person was told "Stored, chunked and |
| + // indexed." |
| + title := strings.TrimSpace(r.FormValue("title")) |
| + body := strings.TrimSpace(r.FormValue("body")) |
| + if body != "" { |
| + if len(body) > maxUpload { |
| + refused = append(refused, fill(c.ReasonTooBig, "{name}", c.PasteName)) |
| + } else { |
| + _, err := a.store.Ingest(r.Context(), "paste", title, title, body) |
| + switch { |
| + case errors.Is(err, ErrIndexLag): |
| + a.logger.Error("index refresh failed after commit", "source", "paste", "err", err) |
| + stored++ |
| + lagging = append(lagging, fill(c.ReasonNotSearchable, "{name}", c.PasteName)) |
| + case err != nil: |
| + a.logger.Error("ingest failed", "source", "paste", "bytes", len(body), "err", err) |
| + refused = append(refused, fill(c.ReasonStore, "{name}", c.PasteName)) |
| + default: |
| + stored++ |
| } |
| } |
| } |
| - if strings.TrimSpace(body) == "" { |
| - http.Redirect(w, r, "/?err=empty", http.StatusSeeOther) |
| + // Nothing happened at all — no file, no paste. lagging is in the |
| + // condition even though every clause in it is paired with a stored++ |
| + // above: this branch says "there was nothing to store" out loud, and |
| + // it must be structurally incapable of saying that about a document |
| + // sitting in the database. A test that dropped the pairing landed |
| + // exactly here. |
| + if stored == 0 && len(refused) == 0 && len(lagging) == 0 { |
| + a.refuse(w, r, "empty", 0) |
| return |
| } |
| - if len(body) > maxUpload { |
| - http.Redirect(w, r, "/?err=toobig", http.StatusSeeOther) |
| + |
| + msg, kind := c.outcome(stored, refused, lagging) |
| + if wantsJSON(r) { |
| + // No flash cookie on this path. It is one-shot and it outlives |
| + // the request that set it, so a tray that has already said this |
| + // out loud would ALSO ambush the next page load with it — a |
| + // notice about a drop that finished five minutes ago. |
| + writeOutcome(w, uploadOutcome{Stored: stored, Refused: refused, Lagging: lagging, Message: msg, Kind: kind}) |
| return |
| } |
| + flash.Set(w, kind, msg) |
| + target := "/?stored=" + strconv.Itoa(stored) |
| + if len(refused) > 0 { |
| + target += "&refused=" + strconv.Itoa(len(refused)) |
| + } |
| + // The lagging count is deliberately NOT here, and the reason is |
| + // worth writing down. The URL is the fallback for a lost flash |
| + // cookie, and it carries the refusal count because a refusal means a |
| + // document that does not exist. A lagging document does exist, is in |
| + // the list below and is in the byte count, so the fallback sentence |
| + // stays true when it omits it — incomplete, never wrong. Saying it |
| + // here would need a second reviewed sentence with a count in it and |
| + // no name to attach it to, which is more copy for a strictly worse |
| + // version of what the cookie already carries. |
| + http.Redirect(w, r, target, http.StatusSeeOther) |
| +} |
| - if _, err := a.store.Ingest(r.Context(), source, key, title, body); err != nil { |
| - // No document content in the log line — AGENTS.md's rule, and |
| - // the reason this carries no title either. |
| - a.logger.Error("ingest failed", "source", source, "bytes", len(body), "err", err) |
| - http.Redirect(w, r, "/?err=store", http.StatusSeeOther) |
| +// readUpload reads one part and judges it, returning either its bytes or |
| +// a refusal clause naming the file and saying why. |
| +// |
| +// Every check the browser makes is made again here, on bytes rather than |
| +// on a File object: a drop is not the only way to reach this handler. |
| +func (a *app) readUpload(c dropZone, hdr *multipart.FileHeader, name string) ([]byte, string) { |
| + f, err := hdr.Open() |
| + if err != nil { |
| + return nil, fill(c.ReasonUnreadable, "{name}", name) |
| + } |
| + defer f.Close() |
| + // LimitReader at cap+1 so a file exactly at the cap passes and one |
| + // byte over is detectable, rather than silently truncated — a |
| + // truncated document indexes as a complete one, which is a citation |
| + // that lies. |
| + raw, err := io.ReadAll(io.LimitReader(f, maxUpload+1)) |
| + switch { |
| + case err != nil: |
| + return nil, fill(c.ReasonUnreadable, "{name}", name) |
| + case len(raw) > maxUpload: |
| + return nil, fill(c.ReasonTooBig, "{name}", name) |
| + case !looksLikeText(raw): |
| + return nil, fill(c.ReasonBinary, "{name}", name) |
| + case strings.TrimSpace(string(raw)) == "": |
| + return nil, fill(c.ReasonEmpty, "{name}", name) |
| + } |
| + return raw, "" |
| +} |
| + |
| +// refuse answers a whole-submission refusal: nothing was stored, and the |
| +// same sentence goes into the flash cookie and behind the URL code, so a |
| +// browser that dropped the cookie still gets the words. |
| +func (a *app) refuse(w http.ResponseWriter, r *http.Request, code string, n int) { |
| + msg := flashFor(code, n) |
| + if wantsJSON(r) { |
| + // Same words, no cookie, no redirect. See writeOutcome for why a |
| + // refusal is still a 200: it is an answer, and the tray needs a |
| + // non-200 to mean the one other thing. |
| + writeOutcome(w, uploadOutcome{Message: msg, Kind: "danger"}) |
| return |
| } |
| - http.Redirect(w, r, "/?stored=1", http.StatusSeeOther) |
| + flash.Set(w, "danger", msg) |
| + target := "/?err=" + code |
| + if n > 0 { |
| + target += "&n=" + strconv.Itoa(n) |
| + } |
| + http.Redirect(w, r, target, http.StatusSeeOther) |
| +} |
| + |
| +// displayName cleans a filename for display. Control bytes are the only |
| +// thing removed — the name is escaped by the template on its way into |
| +// the page, and shortening it here would produce a name nobody |
| +// recognises. |
| +func displayName(c dropZone, s string) string { |
| + s = strings.Map(func(r rune) rune { |
| + if r < 0x20 || r == 0x7f { |
| + return ' ' |
| + } |
| + return r |
| + }, s) |
| + if s = strings.TrimSpace(s); s == "" { |
| + return c.Unnamed |
| + } |
| + return s |
| } |
| // passage is one rendered passage of a document. |
diff --git a/internal/memoria/lag_test.go b/internal/memoria/lag_test.go| new file mode 100644 |
| index 0000000..aab2673 |
| --- /dev/null |
| +++ b/internal/memoria/lag_test.go |
| @@ -0,0 +1,194 @@ |
| +// A document that committed and then failed to reach the index is |
| +// STORED. This file is the gate on the sentence that says so. |
| +// |
| +// These tests live in package memoria rather than in memoriatest, and |
| +// that is the whole reason the file exists here: reaching this state |
| +// means breaking the in-memory index under a running app, and the only |
| +// way to do that from outside would be to grow a production method whose |
| +// only caller is a test. The index is closed directly instead, one field |
| +// away, and nothing is exported to make it possible. |
| +package memoria |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "encoding/json" |
| + "io" |
| + "log/slog" |
| + "mime/multipart" |
| + "net/http" |
| + "net/http/httptest" |
| + "path/filepath" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/rastrillo/db" |
| + |
| + "amadan.net/oficina/memoria/internal/memoria/seal" |
| +) |
| + |
| +const lagOrigin = "http://app.test" |
| + |
| +// appWithABrokenIndex builds the whole app and then closes its index, so |
| +// every ingest from here commits its row and fails its refresh. That is |
| +// the real shape of the failure: the transaction is already done when the |
| +// index is touched, deliberately (see Ingest), so a broken index cannot |
| +// roll a document back. |
| +func appWithABrokenIndex(t *testing.T) (http.Handler, *app) { |
| + t.Helper() |
| + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| + d, err := db.Open(filepath.Join(t.TempDir(), "app.db"), logger) |
| + if err != nil { |
| + t.Fatalf("db.Open: %v", err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + dek := make([]byte, 32) |
| + for i := range dek { |
| + dek[i] = byte(i * 7) |
| + } |
| + sealer, err := seal.New(dek) |
| + if err != nil { |
| + t.Fatalf("seal.New: %v", err) |
| + } |
| + mux, a, err := AppForTest(d, lagOrigin, sealer, logger) |
| + if err != nil { |
| + t.Fatalf("AppForTest: %v", err) |
| + } |
| + if err := a.store.ix.Close(); err != nil { |
| + t.Fatalf("closing the index: %v", err) |
| + } |
| + return mux, a |
| +} |
| + |
| +// postOneFile submits a single file the way the form does. asJSON asks |
| +// for the tray's judgement instead of the redirect; both go through the |
| +// same handler. |
| +func postOneFile(t *testing.T, h http.Handler, name, body string, asJSON bool) *httptest.ResponseRecorder { |
| + t.Helper() |
| + var buf bytes.Buffer |
| + mw := multipart.NewWriter(&buf) |
| + w, err := mw.CreateFormFile("file", name) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := w.Write([]byte(body)); err != nil { |
| + t.Fatal(err) |
| + } |
| + mw.Close() |
| + req := httptest.NewRequest(http.MethodPost, "/documents", &buf) |
| + req.Header.Set("Content-Type", mw.FormDataContentType()) |
| + req.Header.Set("Origin", lagOrigin) |
| + if asJSON { |
| + req.Header.Set("Accept", "application/json") |
| + } |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + return rec |
| +} |
| + |
| +// A committed document is never called a refusal. |
| +// |
| +// The failure this replaces: Ingest refreshes the index AFTER the |
| +// transaction commits, and any error from that refresh came back as a |
| +// plain error. createDocument read it as a store failure and said "{name} |
| +// could not be stored" — about a row that was in the database, in the |
| +// document list and in the byte count, and that came back searchable on |
| +// the next boot. A refusal the app had not verified, which is the |
| +// cardinal sin in AGENTS.md, asserted on the path most likely to fail |
| +// halfway. Found by a code review of this branch. |
| +// |
| +// The vacuity guard is the first assertion, not the last: the document |
| +// has to actually be in the database, or every sentence checked below is |
| +// being judged against something that was never stored. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted, each observed red: |
| +// |
| +// (a) Ingest's two post-commit returns put back to `return 0, err` — |
| +// the code as it stood before this review → "the reply says 0 |
| +// stored ... and calls it refused: [quarterly-notes.txt could not |
| +// be stored]", plus the message check |
| +// (b) the `errors.Is(err, ErrIndexLag)` case deleted from |
| +// createDocument's upload loop, leaving the error to fall through |
| +// to the refusal branch → the same failure. This is the half that |
| +// would rot first: ErrIndexLag can exist and be ignored. |
| +// (c) `stored++` removed from that case, so the document is named but |
| +// not counted → "the reply says 0 stored and calls it refused: []". |
| +// Worth knowing what this one found on the way: before the |
| +// "nothing happened at all" guard was widened to look at lagging |
| +// too, the same mutation sent the whole submission down the empty |
| +// branch and the app said "There was nothing to store — the box was |
| +// empty and no file was chosen" about a document in the database. |
| +func TestAStoredDocumentIsNeverReportedAsRefused(t *testing.T) { |
| + h, a := appWithABrokenIndex(t) |
| + c := newDropZone() |
| + const name = "quarterly-notes.txt" |
| + |
| + rec := postOneFile(t, h, name, "Revenue was flat across the first region we serve.", true) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("the JSON path answered %d, want 200:\n%s", rec.Code, rec.Body.String()) |
| + } |
| + var reply uploadOutcome |
| + if err := json.Unmarshal(rec.Body.Bytes(), &reply); err != nil { |
| + t.Fatalf("the JSON path did not answer JSON (%v):\n%s", err, rec.Body.String()) |
| + } |
| + |
| + // Vacuity first. If the row is not there, everything below is a test |
| + // of the words for a document that genuinely was not stored. |
| + docs, err := a.store.Summaries(context.Background()) |
| + if err != nil { |
| + t.Fatalf("listing documents: %v", err) |
| + } |
| + if len(docs) != 1 { |
| + t.Fatalf("%d documents are in the database, want 1. The index was broken, not the "+ |
| + "database, so this test is no longer about a committed document at all", len(docs)) |
| + } |
| + |
| + if reply.Stored != 1 || len(reply.Refused) != 0 { |
| + t.Errorf("the reply says %d stored and calls it refused: %v. The document is in the "+ |
| + "database — the list above has it — so a refusal here is a refusal this app "+ |
| + "never verified, on the row it is looking at.", reply.Stored, reply.Refused) |
| + } |
| + if want := fill(c.ReasonStore, "{name}", name); strings.Contains(reply.Message, want) { |
| + t.Errorf("the sentence says %q about a document that is stored:\n %q", want, reply.Message) |
| + } |
| + // And it does say the true thing, by name. An outcome that merely |
| + // stopped lying — "Stored 1 document." and nothing else — would leave |
| + // someone searching for a document this process cannot find. |
| + if want := fill(c.ReasonNotSearchable, "{name}", name); !strings.Contains(reply.Message, want) { |
| + t.Errorf("the sentence does not say the document is unsearchable:\n want %q in\n %q", |
| + want, reply.Message) |
| + } |
| + if len(reply.Lagging) != 1 || !strings.Contains(reply.Lagging[0], name) { |
| + t.Errorf("the tray is handed lagging=%v, so it cannot put that caveat in the file's own "+ |
| + "row and would say the word for a clean success instead", reply.Lagging) |
| + } |
| +} |
| + |
| +// The scriptless half of the same claim. It has no JSON to inspect, so |
| +// what is checked is the redirect: a document that stored must not be |
| +// counted behind &refused=. |
| +// |
| +// MUTATION-TESTED 2026-09-04, reverted: Ingest's post-commit returns put |
| +// back to `return 0, err` → "the scriptless path redirected to |
| +// /?stored=0&refused=1 after storing 1 document". |
| +func TestTheScriptlessRedirectDoesNotCountAStoredDocumentAsRefused(t *testing.T) { |
| + h, a := appWithABrokenIndex(t) |
| + rec := postOneFile(t, h, "hosting-costs.txt", "Hosting cost 40 a month before the migration.", false) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("the scriptless path answered %d, want 303:\n%s", rec.Code, rec.Body.String()) |
| + } |
| + docs, err := a.store.Summaries(context.Background()) |
| + if err != nil { |
| + t.Fatalf("listing documents: %v", err) |
| + } |
| + if len(docs) != 1 { |
| + t.Fatalf("%d documents are in the database, want 1; this test is not about a "+ |
| + "committed document any more", len(docs)) |
| + } |
| + loc := rec.Header().Get("Location") |
| + if loc != "/?stored=1" { |
| + t.Errorf("the scriptless path redirected to %s after storing %d document. The counts in "+ |
| + "the URL are the fallback for a lost flash cookie, and a stored document "+ |
| + "counted as refused there is the same lie in fewer words.", loc, len(docs)) |
| + } |
| +} |
diff --git a/internal/memoria/static/drop.js b/internal/memoria/static/drop.js| index e8a6f23..0728be1 100644 |
| --- a/internal/memoria/static/drop.js |
| +++ b/internal/memoria/static/drop.js |
| @@ -1,38 +1,51 @@ |
| -/* drop.js — drag a text file onto the page and it lands in the upload |
| - form. Enhancement, and nothing more. |
| +/* drop.js — drag text files onto the page and they are stored in the |
| + background, one at a time, with a tray in the corner saying how each |
| + one went. Enhancement, and nothing more. |
| - Three rules it is written to keep, in this order: |
| + Four rules it is written to keep, in this order: |
| 1. It is additive. The file input and the Store it button below it are |
| - untouched; this file only ever puts a File into that input. With |
| - scripts off, with this file blocked, or in a browser missing any |
| - piece it needs, the zone never says "ready", nothing is styled, and |
| - the form is exactly the form it was before. There is no second |
| - upload path — no request of its own, no endpoint of its own, and |
| - it never submits the form for you. The person still presses Store |
| - it, and the ordinary multipart POST does the rest — so everything |
| - the server checks, it still checks. |
| - |
| - 2. It never does less than it was asked and calls it done. A drop of |
| - five files is refused whole, out loud, with the count — not |
| - quietly reduced to one, which is the failure this codebase hates |
| - most: five files in, one document stored, nobody told. The same |
| - goes for the case where assigning to input.files does nothing |
| - (older Safari): the assignment is read back, and if the file is |
| - not there the page says so instead of sitting there looking ready. |
| - |
| - 3. Its rules are the server's rules. The size cap, the sniff window |
| - and the words for both come off the page as data attributes, put |
| - there by handlers.go. Nothing is written down twice, so a browser |
| - refusal and a handler refusal cannot drift apart. Missing or |
| - unreadable attributes are a hard stop, not a default: a guessed |
| - cap would refuse files the server would have taken. |
| - |
| - What it deliberately does not do: submit for you (a file you dropped |
| - by accident is then unrecoverable), read anything but the first |
| - sniff-window bytes, or name the file anywhere but on your own screen. |
| - There is no console output in this file at all — a filename is |
| - somebody's content and this codebase keeps content out of logs. */ |
| + untouched. With scripts off, with this file blocked, or in a |
| + browser missing any piece it needs, the zone never says "ready", |
| + nothing is styled, no tray appears, and the form is exactly the |
| + form it was: choose files, press Store it, the page reloads, the |
| + flash sentence appears. Choosing files by hand still does that |
| + even with this file running — only a DROP takes the background |
| + path. |
| + |
| + 2. There is one place a file can be sent, and this file does not |
| + choose it. The address is read off the form's own action attribute |
| + and used verbatim; there is no address written down in here, no |
| + second endpoint, and the form is never submitted for you. So |
| + everything the handler checks — the three caps, the NUL sniff, the |
| + same-origin check, the whole-body limit that keeps a part off the |
| + disk — is still checked, because it is still the same handler. |
| + TestTheDropScriptPostsOnlyToTheFormsOwnAction holds that shape. |
| + |
| + 3. It never does less than it was asked and calls it done. A drop of |
| + five files where one is a PDF sends four and NAMES the fifth with |
| + the reason, in its own row; it never quietly reduces five to four. |
| + An upload that dies mid-flight is not reported as a refusal |
| + either: the request may have been received and stored in full with |
| + only the answer lost, so that row says the fate is unknown and the |
| + tray offers a reload rather than pretending to know. |
| + |
| + 4. Its words are the server's words. The four numbers, the sniff |
| + window and every sentence come off the page as data attributes, |
| + put there by handlers.go. Nothing is written down twice, so a |
| + refusal in the tray and a refusal in the scriptless flash cannot |
| + drift apart. Missing or unreadable attributes are a hard stop, not |
| + a default: a guessed cap would refuse files the server would have |
| + taken, and a guessed sentence is copy nobody reviewed. |
| + |
| + What it deliberately does not do: reload the page for you when the |
| + uploads finish (someone mid-drop does not want the ground moving — |
| + the tray offers a link and the choice is theirs), read anything but |
| + the first sniff-window bytes of each file, take a folder (that needs |
| + a different API and is out of scope), or name a file anywhere but on |
| + your own screen. There is no console output in this file at all — a |
| + filename is somebody's content and this codebase keeps content out of |
| + logs. */ |
| (function () { |
| "use strict"; |
| @@ -42,27 +55,71 @@ |
| var input = zone.querySelector('input[type="file"]'); |
| var note = zone.querySelector("[rst-dropzone-note]"); |
| var hint = zone.querySelector("[rst-dropzone-hint]"); |
| - var paste = document.getElementById("body"); |
| if (!input || !note || !hint) return; |
| + // The tray, whole. Every piece of it, or none of it: a tray missing |
| + // its status line would upload files and announce nothing, and a tray |
| + // missing its list would upload them and show nothing. |
| + var tray = document.querySelector("[rst-tray]"); |
| + if (!tray) return; |
| + var list = tray.querySelector("[rst-tray-list]"); |
| + var status = tray.querySelector("[rst-tray-status]"); |
| + var stale = tray.querySelector("[rst-tray-stale]"); |
| + var reload = tray.querySelector("[rst-tray-reload]"); |
| + var dismiss = tray.querySelector("[rst-tray-dismiss]"); |
| + if (!list || !status || !stale || !reload || !dismiss) return; |
| + |
| + // The form that owns the input, and the one address anything is sent |
| + // to. Read off the form rather than written down here, so a background |
| + // upload and the scriptless submit are incapable of going to different |
| + // places. No action attribute means a form that posts to whatever page |
| + // it happens to be on, which is not something to guess at — hard stop, |
| + // and the plain form is left alone. |
| + var form = input.form; |
| + if (!form) return; |
| + var endpoint = form.getAttribute("action"); |
| + if (!endpoint) return; |
| + // The part name is the input's own, for the same reason: the handler |
| + // reads the parts under one name and a name invented here would post |
| + // files it never looks at. |
| + var field = input.getAttribute("name"); |
| + if (!field) return; |
| + |
| // Everything this needs, checked before anything is styled. A drop |
| - // target that cannot put a file in the input is a trap, so if any of |
| - // this is missing the visitor gets the plain input instead and never |
| - // learns there was going to be a drop zone. |
| - if (typeof DataTransfer === "undefined") return; |
| + // target that cannot send a file is a trap, so if any of this is |
| + // missing the visitor gets the plain input instead and never learns |
| + // there was going to be a drop zone. |
| + if (typeof XMLHttpRequest === "undefined") return; |
| + if (typeof FormData === "undefined") return; |
| if (typeof FileReader === "undefined") return; |
| var maxBytes = number("data-max-bytes"); |
| + var maxTotal = number("data-max-total-bytes"); |
| + var maxFiles = number("data-max-files"); |
| var sniffBytes = number("data-sniff-bytes"); |
| - if (maxBytes === null || sniffBytes === null) return; |
| + var maxInFlight = number("data-max-in-flight"); |
| + if (maxBytes === null || maxTotal === null || maxFiles === null) return; |
| + if (sniffBytes === null || maxInFlight === null) return; |
| var msg = {}; |
| - var keys = ["toobig", "binary", "empty", "many", "folder", "blocked"]; |
| + var keys = [ |
| + "toomany", "toototal", "folder", "blocked", |
| + "waiting", "uploading", "storing", "row-stored", |
| + "stored-one", "stored-many", "none-stored", |
| + "refused-one", "refused-many", "unsure-one", "unsure-many" |
| + ]; |
| for (var i = 0; i < keys.length; i++) { |
| var text = zone.getAttribute("data-msg-" + keys[i]); |
| if (!text) return; // a refusal with no words is a silent refusal |
| msg[keys[i]] = text; |
| } |
| + var reason = {}; |
| + var reasons = ["toobig", "binary", "empty", "unreadable", "noanswer"]; |
| + for (var j = 0; j < reasons.length; j++) { |
| + var clause = zone.getAttribute("data-reason-" + reasons[j]); |
| + if (!clause) return; |
| + reason[reasons[j]] = clause; |
| + } |
| function number(attr) { |
| var raw = zone.getAttribute(attr); |
| @@ -72,10 +129,21 @@ |
| return n; |
| } |
| + // fill is the server's fill(): {name}, {n}, {names}, {pct}. The joining |
| + // punctuation lives out here in the code rather than inside a reviewed |
| + // sentence. |
| + function fill(template, token, value) { |
| + return template.split(token).join(String(value)); |
| + } |
| + |
| // ---- saying things ------------------------------------------------- |
| - // say puts one sentence in the live region under the input. tone is |
| - // "good" or "danger"; both resolve to tokens in memoria.css. |
| + // say puts one sentence in the live region under the input. It carries |
| + // only the refusals that apply to a WHOLE drop — too many files, too |
| + // much at once, a folder — because those have no row to belong to. |
| + // Everything about a particular file is said in that file's own row. |
| + // |
| + // tone is "good" or "danger"; both resolve to tokens in memoria.css. |
| // |
| // It does NOT unhide the note, and that omission is the point: the |
| // paragraph is unhidden empty at the ready step below, so by the time |
| @@ -90,8 +158,322 @@ |
| note.setAttribute("rst-dropzone-note", tone); |
| } |
| - function mark(tokens) { |
| - zone.setAttribute("rst-dropzone", tokens); |
| + // ---- the tray ------------------------------------------------------ |
| + |
| + // Rows waiting for a slot, and how many are in the air. Nothing here |
| + // is a percentage: this is a queue, not a measurement. |
| + var queue = []; |
| + var flying = 0; |
| + // A count, not a flag. Two drops can overlap — the second arrives |
| + // while the first is still sniffing its files — and with a boolean the |
| + // first one to finish cleared it for both, so the summary for one drop |
| + // was announced while the other was still being judged and the person |
| + // got two half-sentences about one thing. |
| + var judging = 0; |
| + |
| + // What has settled since the last announcement. Announced as one |
| + // sentence when everything stops, not per file: a five-file drop must |
| + // not be five interruptions, and polite regions queue rather than |
| + // interrupt, so five would arrive as five in a row anyway. |
| + var batch = newBatch(); |
| + |
| + function newBatch() { |
| + return { stored: 0, refused: [], lagging: [], unsure: [] }; |
| + } |
| + |
| + function showTray() { |
| + tray.setAttribute("rst-tray", "open"); |
| + } |
| + |
| + function hideTray() { |
| + tray.setAttribute("rst-tray", ""); |
| + status.textContent = ""; |
| + stale.hidden = true; |
| + } |
| + |
| + // addRow puts one file in the tray and returns the handles for |
| + // changing it later. The row is created the moment the file is judged, |
| + // so rows appear in the order they were dropped. |
| + // |
| + // The bar is aria-hidden: its value changes many times a second and it |
| + // says nothing the phase text beside it does not say in words. |
| + function addRow(name) { |
| + var li = document.createElement("li"); |
| + li.setAttribute("rst-tray-row", ""); |
| + |
| + var label = document.createElement("span"); |
| + label.setAttribute("rst-tray-name", ""); |
| + label.textContent = name; |
| + |
| + var phase = document.createElement("span"); |
| + phase.setAttribute("rst-tray-phase", ""); |
| + phase.textContent = msg.waiting; |
| + |
| + var bar = document.createElement("progress"); |
| + bar.setAttribute("rst-tray-bar", ""); |
| + bar.setAttribute("aria-hidden", "true"); |
| + bar.max = 100; |
| + bar.value = 0; |
| + |
| + li.appendChild(label); |
| + li.appendChild(phase); |
| + li.appendChild(bar); |
| + list.appendChild(li); |
| + showTray(); |
| + return { li: li, label: label, phase: phase, bar: bar }; |
| + } |
| + |
| + // indeterminate is phase two: the request body is sent and the server |
| + // is sealing, chunking and indexing it. Nothing in the browser can see |
| + // that work, so a progress element with no value is the honest shape — |
| + // it says "something is happening" and refuses to say how much. |
| + function indeterminate(row) { |
| + row.bar.removeAttribute("value"); |
| + row.phase.textContent = msg.storing; |
| + } |
| + |
| + // settle writes a row's final result and takes its bar away. |
| + // |
| + // named says whether the result text names the file itself. Every |
| + // clause from handlers.go does — they are written to travel into a |
| + // sentence about a whole drop — so for those the row's separate name |
| + // label goes, and the clause becomes the row. Looking at the built |
| + // page is what settled this: leaving both showed |
| + // |
| + // scan.png |
| + // scan.png is not text |
| + // |
| + // which is the same shape as the search result that repeated its own |
| + // heading, and it was invisible to every test here. The alternative — |
| + // a second, shorter set of reasons with the name taken out — would be |
| + // two spellings of one rule, and that is the mistake this whole file |
| + // is arranged to avoid. |
| + function settle(row, text, tone, named) { |
| + if (row.bar.parentNode) row.bar.parentNode.removeChild(row.bar); |
| + if (named && row.label.parentNode) row.label.parentNode.removeChild(row.label); |
| + row.phase.textContent = text; |
| + row.li.setAttribute("rst-tray-row", tone); |
| + } |
| + |
| + // The landed result is the one that does NOT name its file — it is a |
| + // word, not a sentence — so this row keeps its label: the filename, |
| + // and beside it what became of it. |
| + // |
| + // (That word is not written out in this comment on purpose. |
| + // TestEveryWordTheTrayWritesComesFromGo scans this file whole, |
| + // comments included, for any copy the page publishes, and prose that |
| + // quotes a reviewed string trips it. The direction of error is |
| + // deliberate: a comment that fails loudly beats a scan that skips |
| + // strings it should have caught.) |
| + function stored(row) { |
| + settle(row, msg["row-stored"], "good", false); |
| + batch.stored++; |
| + } |
| + |
| + // named is passed through rather than assumed: every reason clause |
| + // names its file, but a whole-submission sentence does not, and a row |
| + // that dropped its label for one of those would be anonymous. |
| + function refuse(row, clause, named) { |
| + settle(row, clause, "danger", named); |
| + batch.refused.push(clause); |
| + } |
| + |
| + // lagged is the third outcome, and the one it would be easiest to |
| + // round off. The server took the document, committed it, and then |
| + // failed to put it in its search index — so it counts among the ones |
| + // that landed, and the row still has to say what is wrong with it. The |
| + // clause comes from the server, names the file, and replaces the row's |
| + // own label the way a refusal's does. |
| + function lagged(row, clause) { |
| + settle(row, clause, "danger", true); |
| + batch.stored++; |
| + batch.lagging.push(clause); |
| + } |
| + |
| + // unsure is the one that is neither. The upload did not come back with |
| + // an answer, so this file may be stored, may not be, and the only true |
| + // thing to say is that nobody here knows. |
| + function unsure(row, clause) { |
| + settle(row, clause, "danger", true); |
| + batch.unsure.push(clause); |
| + } |
| + |
| + // ---- the summary --------------------------------------------------- |
| + // |
| + // The same sentence the handler composes for a scriptless submission, |
| + // built from the same strings: how many were stored, then every file |
| + // that was not, by name, with the reason for that file. |
| + |
| + function storedPart(n) { |
| + if (n === 1) return msg["stored-one"]; |
| + if (n > 1) return fill(msg["stored-many"], "{n}", n); |
| + return msg["none-stored"]; |
| + } |
| + |
| + // clauseList is a count and then every clause. Never a count on its |
| + // own — "2 refused" without the names leaves a person guessing which |
| + // two. |
| + function clauseList(head, list_) { |
| + return head + ": " + list_.join("; ") + "."; |
| + } |
| + |
| + function refusalSentence(refused) { |
| + var head = refused.length === 1 ? msg["refused-one"] : fill(msg["refused-many"], "{n}", refused.length); |
| + return clauseList(head, refused); |
| + } |
| + |
| + function unsureSentence(unknown) { |
| + var head = unknown.length === 1 ? msg["unsure-one"] : fill(msg["unsure-many"], "{n}", unknown.length); |
| + return clauseList(head, unknown); |
| + } |
| + |
| + // announce says what a whole drop did, once, when nothing is left in |
| + // the air. Called after every settled upload and after judging, and it |
| + // says nothing at all until there is nothing left to wait for. |
| + function announce() { |
| + if (judging > 0 || flying > 0 || queue.length > 0) return; |
| + if (batch.stored === 0 && batch.refused.length === 0 && |
| + batch.lagging.length === 0 && batch.unsure.length === 0) return; |
| + |
| + var parts = [storedPart(batch.stored)]; |
| + if (batch.refused.length > 0) parts.push(refusalSentence(batch.refused)); |
| + // No head and no count in front of these: each clause is a whole |
| + // sentence naming its own file, and they are already counted in the |
| + // number at the front — they were stored. Same shape, same order, as |
| + // the sentence handlers.go composes for the scriptless page. |
| + if (batch.lagging.length > 0) parts.push(batch.lagging.join("; ") + "."); |
| + if (batch.unsure.length > 0) parts.push(unsureSentence(batch.unsure)); |
| + status.textContent = parts.join(" "); |
| + |
| + // The document list further down was drawn before any of this. It is |
| + // stale the moment one file lands — and equally stale when a file's |
| + // fate is unknown, because the only way to find out is to look. |
| + if (batch.stored > 0 || batch.unsure.length > 0) stale.hidden = false; |
| + |
| + batch = newBatch(); |
| + } |
| + |
| + // ---- sending ------------------------------------------------------- |
| + |
| + function pump() { |
| + while (flying < maxInFlight && queue.length > 0) { |
| + send(queue.shift()); |
| + flying++; |
| + } |
| + } |
| + |
| + function landed() { |
| + flying--; |
| + pump(); |
| + announce(); |
| + } |
| + |
| + function send(job) { |
| + var row = job.row; |
| + var body = new FormData(); |
| + body.append(field, job.file, job.file.name); |
| + |
| + var xhr = new XMLHttpRequest(); |
| + xhr.open("POST", endpoint, true); |
| + // The one thing that makes this request different from the form's |
| + // own: the handler answers a judgement instead of a redirect and a |
| + // flash cookie. Same handler, same checks, same words. |
| + xhr.setRequestHeader("Accept", "application/json"); |
| + |
| + xhr.upload.onprogress = function (e) { |
| + if (!e.lengthComputable || !e.total) { |
| + // The browser will not say how much has gone. Then neither will |
| + // this: the bar loses its value rather than gaining a number |
| + // nobody measured. |
| + row.bar.removeAttribute("value"); |
| + return; |
| + } |
| + var pct = Math.floor((e.loaded / e.total) * 100); |
| + if (pct > 100) pct = 100; |
| + row.bar.value = pct; |
| + row.phase.textContent = fill(msg.uploading, "{pct}", pct + "%"); |
| + }; |
| + // The body is sent. Everything from here is the server's work, which |
| + // is not measurable from here — see indeterminate(). |
| + xhr.upload.onload = function () { |
| + indeterminate(row); |
| + }; |
| + |
| + xhr.onload = function () { |
| + // finally, because landed() is what frees this file's slot. An |
| + // exception thrown anywhere inside answered() would otherwise |
| + // leave the in-flight count one too high forever, and every file |
| + // queued behind it would sit at its first phase with nothing |
| + // sending it — a tray full of rows that never move, which is the |
| + // silent failure with a progress bar on it. |
| + try { |
| + answered(job, xhr); |
| + } finally { |
| + landed(); |
| + } |
| + }; |
| + // Every way an upload can end without an answer lands here. None of |
| + // them is a refusal. |
| + xhr.onerror = noAnswer; |
| + xhr.onabort = noAnswer; |
| + xhr.ontimeout = noAnswer; |
| + |
| + function noAnswer() { |
| + unsure(row, fill(reason.noanswer, "{name}", job.file.name)); |
| + landed(); |
| + } |
| + |
| + xhr.send(body); |
| + } |
| + |
| + // answered reads one judgement. A body that is not the judgement — a |
| + // proxy's error page, a truncated response, anything from a status |
| + // that is not 200 — is not a refusal and is not a success: it is the |
| + // absence of an answer, and it says so. |
| + function answered(job, xhr) { |
| + var row = job.row; |
| + var name = job.file.name; |
| + var reply = null; |
| + if (xhr.status === 200) { |
| + try { |
| + reply = JSON.parse(xhr.responseText); |
| + } catch (err) { |
| + reply = null; |
| + } |
| + } |
| + if (!reply || typeof reply.stored !== "number") { |
| + unsure(row, fill(reason.noanswer, "{name}", name)); |
| + return; |
| + } |
| + if (reply.refused && reply.refused.length > 0) { |
| + // One file went up, so one clause comes back. If a future handler |
| + // ever answered with more, every one of them is shown: a list cut |
| + // short here is the silent truncation this file is written |
| + // against. Each clause names the file, so the clause is the row. |
| + refuse(row, reply.refused.join("; "), true); |
| + return; |
| + } |
| + if (reply.lagging && reply.lagging.length > 0) { |
| + // It landed, and the server could not index it. This is checked |
| + // BEFORE the count below, because the count includes it: reading |
| + // the count first would put the landed word in this row and throw |
| + // the only sentence that says what actually happened away. |
| + lagged(row, reply.lagging.join("; ")); |
| + return; |
| + } |
| + if (reply.stored > 0) { |
| + stored(row); |
| + return; |
| + } |
| + if (reply.message) { |
| + // Judged, nothing stored, nothing named — a whole-submission |
| + // refusal. That sentence is about the drop and not about this |
| + // file, so the row keeps its own name above it. |
| + refuse(row, reply.message, false); |
| + return; |
| + } |
| + // Judged into silence. Nothing here knows what became of it. |
| + unsure(row, fill(reason.noanswer, "{name}", name)); |
| } |
| // ---- the drag ------------------------------------------------------ |
| @@ -118,6 +500,10 @@ |
| mark(on ? "ready over" : "ready"); |
| } |
| + function mark(tokens) { |
| + zone.setAttribute("rst-dropzone", tokens); |
| + } |
| + |
| document.addEventListener("dragenter", function (e) { |
| if (!hasFiles(e)) return; |
| depth++; |
| @@ -164,37 +550,75 @@ |
| say(msg.blocked, "danger"); |
| return; |
| } |
| - if (files.length > 1) { |
| - say(msg.many.replace("{n}", String(files.length)), "danger"); |
| - return; |
| - } |
| if (folder) { |
| + // Still refused whole, and still out of scope: reading a dropped |
| + // directory needs a different API. A folder alongside files would |
| + // otherwise be dropped on the floor unmentioned. |
| say(msg.folder, "danger"); |
| return; |
| } |
| - |
| - var file = files[0]; |
| - if (file.size > maxBytes) { |
| - say(msg.toobig, "danger"); |
| + // The two submission caps, refused whole and up front, exactly as |
| + // the handler refuses them — a drop half-taken by a limit is the |
| + // shape this codebase treats as its worst failure. |
| + if (files.length > maxFiles) { |
| + say(fill(msg.toomany, "{n}", files.length), "danger"); |
| return; |
| } |
| - if (file.size === 0) { |
| - say(msg.empty, "danger"); |
| + var total = 0; |
| + for (var i = 0; i < files.length; i++) total += files[i].size; |
| + if (total > maxTotal) { |
| + say(msg.toototal, "danger"); |
| return; |
| } |
| - sniff(file, function (binary) { |
| - if (binary === null) { |
| - // Could not read it. Not "it is fine" — the server may still |
| - // take it, but this page will not claim to know either way. |
| - say(msg.blocked, "danger"); |
| + // This drop is going ahead, so the previous drop's whole-drop |
| + // refusal is no longer about anything. Cleared, not overwritten: |
| + // there is nothing to say here now that every file has a row. |
| + say("", ""); |
| + judge(files); |
| + } |
| + |
| + // judge walks the files one at a time — sniffing is asynchronous, so |
| + // this is a loop written as a chain — giving each one a row in the |
| + // order it was dropped, and then either settling that row with the |
| + // reason the file was refused or putting the file in the queue. |
| + function judge(files) { |
| + judging++; |
| + var i = 0; |
| + function next() { |
| + if (i >= files.length) { |
| + judging--; |
| + pump(); |
| + announce(); // a drop of nothing but refusals still has a summary |
| + return; |
| + } |
| + var file = files[i++]; |
| + var row = addRow(file.name); |
| + if (file.size > maxBytes) { |
| + refuse(row, fill(reason.toobig, "{name}", file.name), true); |
| + next(); |
| return; |
| } |
| - if (binary) { |
| - say(msg.binary, "danger"); |
| + if (file.size === 0) { |
| + refuse(row, fill(reason.empty, "{name}", file.name), true); |
| + next(); |
| return; |
| } |
| - hand(file); |
| - }); |
| + sniff(file, function (binary) { |
| + if (binary === null) { |
| + // Could not read it. Not "it is fine" — the server may still |
| + // take it, but this page will not claim to know either way, |
| + // and it will not send what it could not look at. |
| + refuse(row, fill(reason.unreadable, "{name}", file.name), true); |
| + } else if (binary) { |
| + refuse(row, fill(reason.binary, "{name}", file.name), true); |
| + } else { |
| + queue.push({ file: file, row: row }); |
| + pump(); |
| + } |
| + next(); |
| + }); |
| + } |
| + next(); |
| } |
| // A dropped directory arrives as a single File with no useful |
| @@ -241,44 +665,36 @@ |
| } |
| } |
| - // hand puts the file in the form's own input — and then checks that it |
| - // is actually there. Assigning input.files is a no-op in some |
| - // browsers, and it fails silently: the zone would sit there saying |
| - // "ready to store" over an empty input. |
| - function hand(file) { |
| - try { |
| - var dt = new DataTransfer(); |
| - dt.items.add(file); |
| - input.files = dt.files; |
| - } catch (err) { |
| - say(msg.blocked, "danger"); |
| - return; |
| + // ---- dismissing ---------------------------------------------------- |
| + |
| + // Dismiss clears the rows that have finished and leaves the ones that |
| + // have not. A row still in the air is the one thing this tray must not |
| + // hide: its outcome has not been said yet, and a tray that took it |
| + // away would be the silent failure with a close button on it. The tray |
| + // goes when the last row does. |
| + dismiss.addEventListener("click", function () { |
| + var rows = list.querySelectorAll("[rst-tray-row]"); |
| + for (var i = 0; i < rows.length; i++) { |
| + if (rows[i].getAttribute("rst-tray-row")) list.removeChild(rows[i]); |
| } |
| - if (input.files.length !== 1 || |
| - input.files[0].name !== file.name || |
| - input.files[0].size !== file.size) { |
| - say(msg.blocked, "danger"); |
| - return; |
| - } |
| - // Anything watching the input — the browser's own validation, a |
| - // future enhancement — sees a real change. |
| - input.dispatchEvent(new Event("change", { bubbles: true })); |
| - |
| - var text = "Ready to store: " + file.name + ". Press Store it."; |
| - if (paste && paste.value.trim() !== "") { |
| - // The handler lets a file override the pasted text, so saying |
| - // nothing here would lose someone's paste without a word. |
| - text += " The pasted text above will not be stored — a file replaces it."; |
| - } |
| - say(text, "good"); |
| - } |
| + if (!list.firstChild) hideTray(); |
| + }); |
| // ---- and only now does the zone exist ------------------------------ |
| - // The empty live region goes into the accessibility tree first and |
| - // stays there. See say(): this is the only place the note is unhidden, |
| - // and TestTheLiveRegionExistsBeforeItHasAnythingToSay holds it here. |
| + // The empty live regions go into the accessibility tree first and stay |
| + // there. See say(): this is the only place the note is unhidden, and |
| + // TestTheLiveRegionExistsBeforeItHasAnythingToSay holds it here. The |
| + // tray is unhidden here for the same reason and is held by |
| + // TestTheTrayIsALiveRegionThatExistsBeforeItSpeaks; it paints nothing |
| + // until a row is added and it is marked open. |
| note.hidden = false; |
| + tray.hidden = false; |
| + hideTray(); |
| + // A reload keeps whatever is on screen — the search someone is looking |
| + // at, not the front page. The href in the template is the fallback for |
| + // a browser that got this far and no further. |
| + reload.href = location.pathname + location.search; |
| mark("ready"); |
| hint.hidden = false; |
| })(); |
diff --git a/internal/memoria/static/memoria.css b/internal/memoria/static/memoria.css| index 157f630..a698061 100644 |
| --- a/internal/memoria/static/memoria.css |
| +++ b/internal/memoria/static/memoria.css |
| @@ -105,6 +105,93 @@ |
| [rst-dropzone~="ready"] { transition: none; } |
| } |
| +/* The upload tray. |
| + * |
| + * It is in the DOM from the moment drop.js runs, because |
| + * [rst-tray-status] is a live region and a live region has to be in the |
| + * accessibility tree before it has anything to announce. So "not open" |
| + * cannot be display:none — that would take the region back out again. |
| + * Instead the head and the list are hidden, the panel paints nothing, |
| + * and an empty status paragraph is zero-height, which leaves a fixed |
| + * element of no size in the corner. pointer-events go with it: an |
| + * invisible box that swallows clicks over the corner of the page would |
| + * be worse than a visible one. |
| + */ |
| +[rst-tray] { |
| + bottom: 1rem; |
| + font-size: 0.8125rem; |
| + inset-inline-end: 1rem; |
| + position: fixed; |
| + width: min(22rem, calc(100vw - 2rem)); |
| + z-index: 30; |
| +} |
| +[rst-tray]:not([rst-tray~="open"]) { pointer-events: none; } |
| +[rst-tray]:not([rst-tray~="open"]) [rst-tray-head], |
| +[rst-tray]:not([rst-tray~="open"]) [rst-tray-list] { display: none; } |
| +[rst-tray~="open"] { |
| + background: var(--rst-surface); |
| + border: 1px solid var(--rst-line); |
| + border-radius: var(--rst-radius, 8px); |
| + box-shadow: var(--rst-shadow-pop, none); |
| + padding: 0.6rem 0.7rem; |
| +} |
| +[rst-tray-head] { |
| + align-items: center; |
| + display: flex; |
| + gap: 0.5rem; |
| + justify-content: space-between; |
| +} |
| +[rst-tray-title] { font-weight: 620; } |
| +/* 40vh so a drop of twenty-five files scrolls inside the tray rather |
| + * than growing a panel taller than the window. */ |
| +[rst-tray-list] { |
| + list-style: none; |
| + margin: 0.5rem 0 0; |
| + max-height: 40vh; |
| + overflow-y: auto; |
| + padding: 0; |
| +} |
| +[rst-tray-row] { |
| + border-top: 1px solid var(--rst-line); |
| + display: grid; |
| + gap: 0.15rem 0.5rem; |
| + grid-template-columns: 1fr auto; |
| + padding: 0.4rem 0; |
| +} |
| +[rst-tray-row]:first-child { border-top: 0; } |
| +/* A filename has no spaces to break at, and the tray is narrow. */ |
| +[rst-tray-name] { font-weight: 560; overflow-wrap: anywhere; } |
| +[rst-tray-phase] { color: var(--rst-text-muted); text-align: end; } |
| +[rst-tray-row~="good"] [rst-tray-phase] { color: var(--rst-tone-positive-fg); } |
| +/* A refusal is a sentence, not a word: it gets the whole row width and |
| + * reads left to right under the name it belongs to. |
| + */ |
| +[rst-tray-row~="danger"] [rst-tray-phase] { |
| + color: var(--rst-tone-negative-fg); |
| + grid-column: 1 / -1; |
| + text-align: start; |
| +} |
| +/* accent-color rather than ::-webkit-progress-value and friends, and |
| + * that is not a detail: styling those pseudo-elements replaces the |
| + * browser's indeterminate rendering with a static bar, and the |
| + * indeterminate state is the whole point of phase two — a bar that |
| + * stops moving while the server works reads as broken. |
| + */ |
| +[rst-tray-bar] { |
| + accent-color: var(--rst-accent, var(--rst-text)); |
| + block-size: 0.35rem; |
| + grid-column: 1 / -1; |
| + inline-size: 100%; |
| +} |
| +[rst-tray-status] { margin: 0.5rem 0 0; } |
| +/* Unhidden and empty from the moment the script runs, so it must cost no |
| + * space until it says something — and the obvious "fix" for a stray |
| + * half-line in the corner is to hide the region again, which is the bug |
| + * this whole arrangement exists to avoid. |
| + */ |
| +[rst-tray-status]:empty { margin: 0; } |
| +[rst-tray-stale] { color: var(--rst-text-muted); margin: 0.4rem 0 0; } |
| + |
| [rst-results] { list-style: none; margin: 0; padding: 0; } |
| [rst-result] { border-top: 1px solid var(--rst-line); padding: 0.9rem 0; } |
| [rst-result]:first-child { border-top: 0; } |
diff --git a/internal/memoria/store.go b/internal/memoria/store.go| index 98525a6..758d775 100644 |
| --- a/internal/memoria/store.go |
| +++ b/internal/memoria/store.go |
| @@ -67,6 +67,11 @@ func (s *Store) externalID(key string) (string, error) { |
| // which is the stale-passage failure the spec's §4.1 refuses to allow. |
| // key is the STABLE identifier in the source: a file path, a page id, a |
| // pasted document's title. Not the body — see Document.ExternalID. |
| +// |
| +// Two failures, and they are not the same failure. Anything before the |
| +// commit means nothing was stored and the id comes back zero. Anything |
| +// after it means the document IS stored and only the index is behind: the |
| +// id comes back real and the error wraps ErrIndexLag. |
| func (s *Store) Ingest(ctx context.Context, source, key, title, body string) (int64, error) { |
| title = strings.TrimSpace(title) |
| body = strings.TrimSpace(body) |
| @@ -186,13 +191,41 @@ func (s *Store) Ingest(ctx context.Context, source, key, title, body string) (in |
| // index left the document stored and unsearchable, the footer count |
| // disagreeing with the table, and the person told "that did not |
| // work" about a document that exists. |
| + // |
| + // Everything from here on has ALREADY COMMITTED, so a failure here is |
| + // not a failure to store and must not be returned as one. It is |
| + // wrapped in ErrIndexLag and the document id comes back beside it: the |
| + // row exists, it will appear in the list, and the only thing wrong |
| + // with it is that this process cannot find it. The caller has to be |
| + // able to tell those apart, because the sentence for one of them is a |
| + // lie about the other — createDocument used to say "could not be |
| + // stored" about a document sitting in the database. Found by a code |
| + // review of this branch. |
| post := context.WithoutCancel(ctx) |
| if err := s.ix.RemoveDocument(docID); err != nil { |
| - return 0, err |
| + return docID, fmt.Errorf("%w: %v", ErrIndexLag, err) |
| + } |
| + if err := s.indexDocument(post, docID); err != nil { |
| + return docID, fmt.Errorf("%w: %v", ErrIndexLag, err) |
| } |
| - return docID, s.indexDocument(post, docID) |
| + return docID, nil |
| } |
| +// ErrIndexLag is the committed-but-unsearchable outcome: the document is |
| +// in the database and the in-memory index does not have it. |
| +// |
| +// It is a distinct error rather than a plain one because the two states |
| +// need two different sentences. A document that was refused is not there; |
| +// a document that lagged is there, is in the list, is in the byte count, |
| +// and comes back the next time the index is rebuilt from the sealed rows, |
| +// which is every boot. Telling someone the first about the second is the |
| +// refusal this app never verified. |
| +// |
| +// The wrapped cause is a database or index error and carries no document |
| +// content — see the AGENTS.md rule this error is allowed to be logged |
| +// under. |
| +var ErrIndexLag = errors.New("the document is stored, but refreshing the index failed") |
| + |
| // indexDocument opens a document's chunks and adds them to the index. |
| func (s *Store) indexDocument(ctx context.Context, docID int64) error { |
| // The title is indexed alongside every chunk, so it is read here |
diff --git a/internal/memoria/templates/index.html b/internal/memoria/templates/index.html| index 32c25d3..d2be8ca 100644 |
| --- a/internal/memoria/templates/index.html |
| +++ b/internal/memoria/templates/index.html |
| @@ -37,36 +37,63 @@ |
| <section rst-section> |
| <h2>Add something</h2> |
| <form method="post" action="/documents" enctype="multipart/form-data" rst-stack> |
| - <label for="title">Title <span rst-cell-mut>(optional — the first line is used if blank)</span></label> |
| + <label for="title">Title <span rst-cell-mut>{{.Drop.TitleHint}}</span></label> |
| <input type="text" id="title" name="title"> |
| <label for="body">Paste text</label> |
| <textarea id="body" name="body" rows="8" placeholder="Paste a document, notes, minutes…"></textarea> |
| - <label for="file">…or upload a text file</label> |
| + <label for="file">{{.Drop.FileLabel}}</label> |
| {{/* The drop zone WRAPS the file input, it does not replace it. With |
| no script the div is an unstyled box, the hint and the note stay |
| - hidden, and this is the same form it was: choose a file, press |
| + hidden, and this is the same form it was: choose files, press |
| Store it. static/drop.js adds rst-dropzone="ready", which is what |
| the styling keys off — so a page whose script did not load never |
| draws a drop target that cannot take a drop. |
| + What a DROP does has changed: it no longer fills this input and |
| + wait for Store it. Each dropped file is posted on its own, in |
| + the background, to this form's own action, and the tray below |
| + the form says how each one went. Choosing files here and |
| + pressing Store it is untouched and still posts the whole form. |
| + |
| The data attributes are the handler's own limits and the |
| handler's own words (see dropZone in handlers.go). The browser |
| reads them from here rather than carrying a second copy, so it |
| - cannot come to refuse what the server would accept. The input |
| - stays single-file: the handler stores one document per |
| - submission, so `multiple` would let the file chooser hand it |
| - five and have four vanish without a word. */}} |
| + cannot come to refuse what the server would accept. |
| + |
| + The input is `multiple`, and that is a plain HTML affordance |
| + with no script behind it: createDocument iterates every part |
| + named "file" and stores one document each, so a scriptless |
| + chooser that hands it five files gets five documents. It was |
| + deliberately single-file while the handler read only the first |
| + part — four of five would have vanished without a word. */}} |
| {{with .Drop}} |
| <div rst-dropzone |
| data-max-bytes="{{.MaxBytes}}" |
| + data-max-total-bytes="{{.MaxTotalBytes}}" |
| + data-max-files="{{.MaxFiles}}" |
| data-sniff-bytes="{{.SniffBytes}}" |
| - data-msg-toobig="{{.TooBig}}" |
| - data-msg-binary="{{.Binary}}" |
| - data-msg-empty="{{.Empty}}" |
| - data-msg-many="{{.Many}}" |
| + data-max-in-flight="{{.MaxInFlight}}" |
| + data-msg-toomany="{{.TooManyFiles}}" |
| + data-msg-toototal="{{.TooMuchAtOnce}}" |
| data-msg-folder="{{.Folder}}" |
| - data-msg-blocked="{{.Blocked}}"> |
| - <input type="file" id="file" name="file" accept=".txt,.md,.markdown,.csv,text/plain,text/markdown"> |
| + data-msg-blocked="{{.Blocked}}" |
| + data-msg-waiting="{{.Waiting}}" |
| + data-msg-uploading="{{.Uploading}}" |
| + data-msg-storing="{{.Storing}}" |
| + data-msg-row-stored="{{.RowStored}}" |
| + data-msg-stored-one="{{.StoredOne}}" |
| + data-msg-stored-many="{{.StoredMany}}" |
| + data-msg-none-stored="{{.NoneStored}}" |
| + data-msg-refused-one="{{.RefusedOne}}" |
| + data-msg-refused-many="{{.RefusedMany}}" |
| + data-msg-unsure-one="{{.UnsureOne}}" |
| + data-msg-unsure-many="{{.UnsureMany}}" |
| + data-reason-toobig="{{.ReasonTooBig}}" |
| + data-reason-binary="{{.ReasonBinary}}" |
| + data-reason-empty="{{.ReasonEmpty}}" |
| + data-reason-unreadable="{{.ReasonUnreadable}}" |
| + data-reason-noanswer="{{.ReasonNoAnswer}}"> |
| + <input type="file" id="file" name="file" multiple accept=".txt,.md,.markdown,.csv,text/plain,text/markdown"> |
| {{/* No rst-cell-mut here: that utility is a class, `.rst-cell-mut`, |
| and spelled as an attribute it styles nothing at all — the hint |
| would have looked muted in the markup and read as body text on |
| @@ -80,13 +107,47 @@ |
| accessibility tree in the same tick as its first message is |
| commonly not announced at all, and a refusal nobody hears is |
| the one outcome this feature must not have. */}} |
| - <p rst-dropzone-hint hidden>…or drop one anywhere on this page. One file at a time, up to {{.Limit}}.</p> |
| + <p rst-dropzone-hint hidden>{{.Hint}}</p> |
| <p rst-dropzone-note role="status" aria-live="polite" hidden></p> |
| </div> |
| {{end}} |
| <div rst-form-bar><button rst-btn="primary">Store it</button></div> |
| </form> |
| {{if .Flash}}<div rst-callout="{{.FlashKind}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}} |
| + |
| + {{/* The upload tray. It lives OUTSIDE the form on purpose: it is not |
| + something the form submits, and a scriptless browser must find the |
| + form exactly as it was. Scriptless it is `hidden` and empty, so it |
| + is not on the page at all. |
| + |
| + drop.js unhides it — still empty — at its ready step, for the same |
| + reason it unhides the note there: [rst-tray-status] is the live |
| + region that announces what a drop did, and a live region that |
| + enters the accessibility tree in the same tick as its first |
| + message is commonly not announced at all. Only [rst-tray-status] |
| + is live. The rows are not, and must not be: five files uploading |
| + is five rows changing several times a second, and a live region |
| + over that is a screen reader nobody can use. |
| + |
| + Every word here comes from dropZone in handlers.go, like the data |
| + attributes above — the tray and the flash sentence a scriptless |
| + browser sees are the same strings, so they cannot drift. |
| + |
| + The reload link is a real link with a real href, and nothing |
| + reloads on its own: someone mid-drop does not want the ground |
| + moving under them. drop.js repoints it at the current URL so a |
| + reload keeps the search that is on screen. */}} |
| + {{with .Drop}} |
| + <div rst-tray hidden role="region" aria-labelledby="tray-title"> |
| + <div rst-tray-head> |
| + <span rst-tray-title id="tray-title">{{.TrayTitle}}</span> |
| + <button type="button" rst-btn="quiet compact" rst-tray-dismiss>{{.TrayDismiss}}</button> |
| + </div> |
| + <ul rst-tray-list></ul> |
| + <p rst-tray-status role="status" aria-live="polite"></p> |
| + <p rst-tray-stale hidden>{{.Stale}} <a rst-tray-reload href="/">{{.Reload}}</a></p> |
| + </div> |
| + {{end}} |
| </section> |
| <section rst-section> |
diff --git a/internal/memoriatest/drop_test.go b/internal/memoriatest/drop_test.go| index 310eb3e..d1d8c71 100644 |
| --- a/internal/memoriatest/drop_test.go |
| +++ b/internal/memoriatest/drop_test.go |
| @@ -1,11 +1,16 @@ |
| package memoriatest |
| import ( |
| + "bytes" |
| "fmt" |
| + "mime/multipart" |
| "net/http" |
| + "net/http/httptest" |
| "os" |
| "path/filepath" |
| "regexp" |
| + "slices" |
| + "strconv" |
| "strings" |
| "testing" |
| ) |
| @@ -39,12 +44,29 @@ import ( |
| // navigate away, which is what a dropped file used to do. Then read |
| // location.href, input.files.length and [rst-dropzone-note]. |
| // |
| -// Checked that way on 2026-09-04, Chromium 151: one file dropped on the |
| -// textarea and on the document list both landed in the input with the |
| -// page still on "/" and the textarea untouched; two files at once left |
| -// input.files empty and said "That drop had 2 files"; a folder was |
| -// named as a folder; and pressing Store it after a drop stored the |
| -// document, which then came back from a search. |
| +// Checked that way on 2026-09-04, Chromium 151, BEFORE multi-file: one |
| +// file dropped on the textarea and on the document list both landed in |
| +// the input with the page still on "/" and the textarea untouched; two |
| +// files at once left input.files empty and said "That drop had 2 files"; |
| +// a folder was named as a folder; and pressing Store it after a drop |
| +// stored the document, which then came back from a search. |
| +// |
| +// Re-checked the same way on 2026-09-04, Chromium 151, for multi-file. |
| +// Five files dropped at once — three text, one binary, one empty — left |
| +// input.files holding exactly the three text files, the page still on |
| +// "/", and the note reading "Ready to store: alpha.md, beta.md, |
| +// gamma.md. Press Store it. 2 refused: scan.png is not text; blank.txt |
| +// is empty." Text typed into the paste box before the drop was still |
| +// there afterwards; pressing Store it then stored four documents (three |
| +// files and the paste) and the table went from three rows to seven. A |
| +// drop of 26 files left input.files EMPTY and said "That drop had 26 |
| +// files. memoria takes 25 at a time…". A drop of only the binary and |
| +// the empty file left the input empty and said "2 refused: …". Nothing |
| +// but a favicon 404 in the console. |
| +// |
| +// What that run does NOT cover, and nothing here does: the total-size |
| +// refusal in the browser, and a folder dropped alongside files. Both |
| +// are refused by the handler too, and the handler's half is tested. |
| // dropZoneAttr pulls one attribute off the drop zone element. |
| // |
| @@ -79,13 +101,17 @@ func dropZoneTag(t *testing.T, body string) string { |
| // The whole feature is an enhancement, so the form underneath it has to |
| // be the form it was. Three things make that true and each is checked: |
| // the file input still exists inside the drop zone (wrapped, not |
| -// replaced), it is still single-file, and nothing the script controls is |
| -// visible before the script has run. |
| +// replaced), it accepts every file a person chose, and nothing the |
| +// script controls is visible before the script has run. |
| // |
| -// `multiple` is the one to watch. Adding it looks harmless and is not: |
| -// createDocument reads r.FormFile("file"), which takes the first part |
| -// and drops the rest without a word, so a file chooser that let someone |
| -// pick five would store one and say "Stored, chunked and indexed." |
| +// `multiple` is the one to watch, and the assertion on it is INVERTED |
| +// from the one that stood here. While createDocument read one |
| +// r.FormFile("file"), `multiple` was forbidden: it would have let the |
| +// chooser hand over five files and had four vanish without a word. Now |
| +// the handler iterates r.MultipartForm.File["file"] and `multiple` is |
| +// required, because without it the chooser offers one file and four of |
| +// a person's five never become parts at all. The two states are one |
| +// edit apart in each direction, so both reasons are recorded. |
| // |
| // MUTATION-TESTED 2026-09-04, in index.html, each reverted: |
| // |
| @@ -93,8 +119,7 @@ func dropZoneTag(t *testing.T, body string) string { |
| // input; the drop zone replaced it instead of wrapping it" |
| // (a2) moving that input above `{{with .Drop}}`, so it is on the page |
| // but outside the zone → "the file input is outside the drop zone" |
| -// (b) adding `multiple` to it → "the file input is `multiple`, but |
| -// createDocument stores one document per submission" |
| +// (b) removing `multiple` from it → "the file input is not `multiple`" |
| // (c) deleting `hidden` from the hint → "rst-dropzone-hint is visible |
| // before any script has run" |
| // (c2) replacing `hidden` on the hint with `data-was-hidden="yes"` → |
| @@ -129,12 +154,13 @@ func TestTheUploadFormStillWorksWithoutScript(t *testing.T) { |
| "instead of wrapping it, and the form no longer works without script:\n%s", form) |
| } |
| if !strings.Contains(input, `name="file"`) { |
| - t.Errorf("the file input is not named `file`, so createDocument's FormFile(\"file\") will not see it: %s", input) |
| + t.Errorf("the file input is not named `file`, so createDocument's "+ |
| + "MultipartForm.File[\"file\"] will not see it: %s", input) |
| } |
| - if regexp.MustCompile(`\bmultiple\b`).MatchString(input) { |
| - t.Errorf("the file input is `multiple`, but createDocument stores one document per "+ |
| - "submission and takes the first part only — four of five chosen files would "+ |
| - "vanish silently: %s", input) |
| + if !regexp.MustCompile(`\bmultiple\b`).MatchString(input) { |
| + t.Errorf("the file input is not `multiple`. It is the whole scriptless half of "+ |
| + "multi-file upload: without it the chooser takes one file, four of five never "+ |
| + "leave the browser, and the handler happily reports one document stored: %s", input) |
| } |
| // Containment: the input must sit between the drop zone's opening |
| @@ -180,45 +206,150 @@ func TestTheUploadFormStillWorksWithoutScript(t *testing.T) { |
| } |
| } |
| -// The script has to actually be served, and it has to have no upload |
| -// path of its own: the drop fills the form's input and the ordinary |
| -// multipart POST does the rest, so everything createDocument checks is |
| -// still checked. A fetch() in here would be a second, unchecked way in. |
| +// The script has to actually be served, and it may send a file to |
| +// exactly one place: the form's own action, read off the form. |
| +// |
| +// THE PROPERTY HERE WAS NARROWED, on 2026-09-04, on the owner's explicit |
| +// approval of the asynchronous-upload design. What stood here was "the |
| +// drop fills the form's own input and does nothing else", enforced by |
| +// forbidding XMLHttpRequest outright alongside fetch( and both spellings |
| +// of submit. That was the right property while a drop meant filling an |
| +// input; it is not the property of a design where each dropped file is |
| +// posted on its own, in the background, with a progress tray beside it. |
| +// |
| +// What is emphatically NOT narrowed is the reason that rule existed: a |
| +// second way in is a way in that the caps, the NUL sniff, the whole-body |
| +// limit that keeps parts off the disk and the same-origin check were |
| +// never asked about. So this pins the ADDRESS instead of the mechanism, |
| +// and it is stricter about the address than the old one was about |
| +// anything: |
| +// |
| +// - fetch(, sendBeacon, .submit( and requestSubmit( stay forbidden. |
| +// One posting mechanism, so there is one thing to check. |
| +// - exactly ONE .open( call site exists, and its address argument is |
| +// the variable `endpoint` — not a literal, not an expression. |
| +// - `endpoint` is assigned exactly once, from the form's own action |
| +// attribute, and the form is the file input's own form. |
| +// - no address literal appears anywhere in the file. A second endpoint |
| +// has to be spelled somewhere, and this is where. |
| // |
| // Note for whoever edits drop.js next: this scans the file whole, |
| // comments included, so prose in that file may not spell the forbidden |
| -// names either. That is the deliberate direction of error — a comment |
| -// that trips this fails loudly, where stripping comments first would |
| -// risk over-stripping and passing something real. |
| +// names either, nor quote a path. That is the deliberate direction of |
| +// error — a comment that trips this fails loudly, where stripping |
| +// comments first would risk over-stripping and passing something real. |
| // |
| // MUTATION-TESTED 2026-09-04, each reverted: |
| // |
| // (a) `mv static/drop.js static/drop2.js` → "GET /static/drop.js: |
| // status 404, want 200" |
| -// (b) replacing the dispatchEvent line with `fetch("/documents", …)` → |
| -// "the drop script uses fetch(" |
| -// (c) replacing it with `input.form.submit()` → "uses .submit(" |
| -// (c2) replacing it with `input.form.requestSubmit()` → "uses |
| -// requestSubmit(". Recorded because this one was GREEN before: |
| -// `.submit(` does not match `requestSubmit(`, so the modern |
| -// spelling of the thing this test exists to forbid walked straight |
| -// through it, and drop.js could have posted a file the moment it |
| -// landed — no chance to undo an accidental drop. |
| -// (d) adding `console.log("dropped", file.name)` → "the drop script |
| +// (b) `xhr.open("POST", endpoint, true)` → `xhr.open("POST", |
| +// "/upload", true)`, the second endpoint this whole test exists |
| +// for → "the drop script opens a request on something other than |
| +// the endpoint read off the form" AND "the drop script contains |
| +// the address literal "/upload"" |
| +// (c) a second call site added (`var probe = new XMLHttpRequest(); |
| +// probe.open("POST", endpoint, true);`) → "the drop script opens 2 |
| +// requests; exactly one call site may exist" |
| +// (d) `endpoint = form.getAttribute("action")` → `endpoint = |
| +// "/documents"` — the plausible "it is right there in the |
| +// template" edit, and the one that would let the template's action |
| +// and the script's address drift apart → "the drop script does not |
| +// read its address off the form's own action attribute", plus the |
| +// literal failure |
| +// (e) `xhr.send(body)` → `form.requestSubmit()` → "the drop script |
| +// uses requestSubmit(" |
| +// (f) adding `console.log("dropped", file.name)` → "the drop script |
| // writes to the console; a dropped file's name is content" |
| -func TestTheDropScriptIsServedAndHasNoUploadPathOfItsOwn(t *testing.T) { |
| +// |
| +// MUTATION-TESTED 2026-09-04, second round, after a code review walked |
| +// past this test twice on a build it had not been able to fail. Both were |
| +// replayed here and both were green before the two regexes above were |
| +// widened; both are red now, and each was reverted: |
| +// |
| +// (g) `endpoint += '/../upload';` on the line after the read — the |
| +// reviewer's own mutation, and the reason "assigned once" is now |
| +// spelled to include compound assignment → "the drop script assigns |
| +// `endpoint` 2 times" AND "the drop script contains the address |
| +// literal '/../upload'" |
| +// (h) ``var alt = `/upload`;`` — the same address in the third quoting |
| +// style, which the old double-quotes-only pattern could not see → |
| +// "the drop script contains the address literal `/upload`" |
| +// |
| +// And one that does NOT fail, recorded because a reader will try it: |
| +// changing the form's action in index.html. The script reads it, so both |
| +// sides move together — which is the point of reading it rather than |
| +// writing it down twice. |
| +func TestTheDropScriptPostsOnlyToTheFormsOwnAction(t *testing.T) { |
| src := servedDropScript(t, newApp(t)) |
| // requestSubmit( is listed separately and deliberately: it is not a |
| // suffix of ".submit(" and the whole point of the rule is that the |
| // script never submits the form, by any spelling. |
| - for _, forbidden := range []string{"fetch(", "XMLHttpRequest", "navigator.sendBeacon", ".submit(", "requestSubmit("} { |
| + for _, forbidden := range []string{"fetch(", "navigator.sendBeacon", ".submit(", "requestSubmit("} { |
| if strings.Contains(src, forbidden) { |
| - t.Errorf("the drop script uses %s. The drop must fill the form's own input and "+ |
| - "nothing else: a second upload path is a path the server's cap, its NUL "+ |
| - "check and its CSRF middleware were not asked about.", forbidden) |
| + t.Errorf("the drop script uses %s. There is one posting mechanism and one address: "+ |
| + "a second way in is a way in that the server's caps, its NUL check, its "+ |
| + "whole-body limit and its same-origin check were not asked about.", forbidden) |
| } |
| } |
| + |
| + // Exactly one request is opened, and its address is the variable — |
| + // not a literal, not something built out of one. |
| + opens := regexp.MustCompile(`\.open\s*\(([^)]*)\)`).FindAllStringSubmatch(src, -1) |
| + if len(opens) != 1 { |
| + t.Fatalf("the drop script opens %d requests; exactly one call site may exist, or this "+ |
| + "test cannot say where all of them go", len(opens)) |
| + } |
| + if !regexp.MustCompile(`^\s*"POST"\s*,\s*endpoint\s*(,[^,]*)?$`).MatchString(opens[0][1]) { |
| + t.Errorf("the drop script opens a request on something other than the endpoint read "+ |
| + "off the form: .open(%s). The address may not be a literal and may not be an "+ |
| + "expression — it is the form's own action, or it is a second endpoint nothing "+ |
| + "on the server side agreed to.", opens[0][1]) |
| + } |
| + |
| + // And that variable is the form's action, assigned once. A second |
| + // assignment could be anything at all. |
| + // |
| + // COMPOUND assignment counts. `endpoint = …` was all this matched |
| + // until a code review wrote `endpoint += '/../upload'` on the line |
| + // after the read and watched the whole test stay green: `+=` is not |
| + // `=`, and the address that request would have gone to is /upload. |
| + // The alternation covers every JavaScript assignment operator, |
| + // longest first, and the trailing [^=] keeps `==` and `===` out. |
| + assign := regexp.MustCompile(`\bendpoint\s*(?:>>>|\*\*|<<|>>|\|\||&&|\?\?|[-+*/%&|^])?=[^=]`) |
| + if n := len(assign.FindAllString(src, -1)); n != 1 { |
| + t.Errorf("the drop script assigns `endpoint` %d times; the address is read once, from "+ |
| + "one place, or the value opened above is not the value read below", n) |
| + } |
| + if !strings.Contains(src, `endpoint = form.getAttribute("action")`) { |
| + t.Error("the drop script does not read its address off the form's own action " + |
| + "attribute. Anything else — a literal, a constructed path, another element's " + |
| + "action — is an address that can drift from the one the scriptless submit uses.") |
| + } |
| + if !strings.Contains(src, "var form = input.form") { |
| + t.Error("the drop script does not take its form from the file input. It has to be THAT " + |
| + "form: the one the person would have posted by pressing Store it.") |
| + } |
| + |
| + // No address is written down in here at all — in ANY of the three |
| + // ways JavaScript spells a string. This used to inspect double-quoted |
| + // strings only, so '/upload' and `/upload` were both invisible to it; |
| + // the same code review that found the compound assignment above wrote |
| + // its second endpoint in single quotes, which is the spelling half |
| + // this file's own selectors already use. |
| + var quoted []string |
| + for _, q := range []string{`"`, `'`, "`"} { |
| + quoted = append(quoted, |
| + q+`(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?//[^`+q+`]*`+q, |
| + q+`/[^`+q+`]*`+q) |
| + } |
| + if lit := regexp.MustCompile(strings.Join(quoted, "|")).FindAllString(src, -1); len(lit) > 0 { |
| + t.Errorf("the drop script contains the address literal %s. Every address it uses comes "+ |
| + "off the page; one written down in here is one the server never agreed to.", |
| + strings.Join(lit, ", ")) |
| + } |
| + |
| // A filename is somebody's content. It never goes to a log, and the |
| // browser console is close enough to one to keep the rule. |
| if strings.Contains(src, "console.") { |
| @@ -239,12 +370,13 @@ func TestTheDropScriptIsServedAndHasNoUploadPathOfItsOwn(t *testing.T) { |
| // |
| // (a) data-max-bytes="100000" in index.html → "the page advertises a |
| // cap of 100000 bytes but the handler answered "/?stored=1" for a |
| -// file 1 byte over it" |
| +// file 1 byte over it", plus "the refusal does not name the file |
| +// it refused" and "2 documents are stored ... want 1" |
| // (b) `io.LimitReader(f, maxUpload+1)` → `io.LimitReader(f, maxUpload)` |
| -// in createDocument, which truncates instead of refusing → the |
| -// same failure, at the real cap. This is the mutation worth |
| -// knowing: an over-cap upload came back "/?stored=1" having |
| -// quietly lost its last byte. |
| +// in readUpload, which truncates instead of refusing → the same |
| +// failure, at the real cap. This is the mutation worth knowing: an |
| +// over-cap upload came back "/?stored=1" having quietly lost its |
| +// last byte. |
| // |
| // And one that does NOT fail, recorded because a reader will try it: |
| // changing maxUpload itself. The page renders that constant, so both |
| @@ -268,13 +400,23 @@ func TestTheAdvertisedCapIsTheHandlersCap(t *testing.T) { |
| "the page tells the browser it will be taken", cap, rec.Header().Get("Location")) |
| } |
| - over := at + "x" |
| - rec := upload(t, h, "over-the-cap.txt", over) |
| - if loc := rec.Header().Get("Location"); loc != "/?err=toobig" { |
| + // One byte over: refused, and refused BY NAME. The assertion is on |
| + // the outcome sentence rather than on a URL code, because that is |
| + // the whole of what a person is told and it is the half that can |
| + // silently become "stored". |
| + rec := upload(t, h, "over-the-cap.txt", at+"x") |
| + if loc := rec.Header().Get("Location"); loc != "/?stored=0&refused=1" { |
| t.Errorf("the page advertises a cap of %d bytes but the handler answered %q for a "+ |
| "file 1 byte over it; the browser would refuse a file this server accepts, "+ |
| "or the other way round", cap, loc) |
| } |
| + said := outcome(t, h, rec) |
| + if !strings.Contains(said, "over-the-cap.txt") { |
| + t.Errorf("the refusal does not name the file it refused: %q", said) |
| + } |
| + if n := storedCount(t, h); n != 1 { |
| + t.Errorf("%d documents are stored after one at the cap and one over it, want 1", n) |
| + } |
| } |
| // Same again for the sniff window. looksLikeText reads a prefix and the |
| @@ -291,10 +433,10 @@ func TestTheAdvertisedCapIsTheHandlersCap(t *testing.T) { |
| // |
| // (a) data-sniff-bytes="64" in index.html → "a NUL at byte 64 — one |
| // past the advertised 64-byte window — was answered |
| -// "/?err=binary", want /?stored=1" |
| +// "/?stored=0&refused=1", want /?stored=1" |
| // (b) looksLikeText's window hard-coded back to 4096 → "a NUL at byte |
| // 8191 — the last byte inside the advertised 8192-byte window — |
| -// was answered "/?stored=1", want /?err=binary" |
| +// was answered "/?stored=1", want a refusal" |
| // |
| // As with the cap: changing sniffBytes itself moves both sides and |
| // stays green on purpose. |
| @@ -305,11 +447,14 @@ func TestTheAdvertisedSniffWindowIsTheHandlersWindow(t *testing.T) { |
| t.Fatalf("data-sniff-bytes is %d; the tests below would prove nothing", window) |
| } |
| - // A NUL at the last byte the window covers: refused. |
| + // A NUL at the last byte the window covers: refused, by name. |
| inside := strings.Repeat("a", window-1) + "\x00" + strings.Repeat("b", 64) |
| - if loc := upload(t, h, "inside.txt", inside).Header().Get("Location"); loc != "/?err=binary" { |
| + rec := upload(t, h, "inside.txt", inside) |
| + if loc := rec.Header().Get("Location"); loc != "/?stored=0&refused=1" { |
| t.Errorf("a NUL at byte %d — the last byte inside the advertised %d-byte window — "+ |
| - "was answered %q, want /?err=binary", window-1, window, loc) |
| + "was answered %q, want a refusal", window-1, window, loc) |
| + } else if said := outcome(t, h, rec); !strings.Contains(said, "inside.txt") { |
| + t.Errorf("the refusal does not name the file it refused: %q", said) |
| } |
| // A NUL one byte past the window: stored, because neither side |
| @@ -323,59 +468,149 @@ func TestTheAdvertisedSniffWindowIsTheHandlersWindow(t *testing.T) { |
| } |
| } |
| -// A drop of several files is refused whole, and the refusal says so. |
| +// The file-count cap the page advertises is the one the handler |
| +// enforces, and a drop over it is refused WHOLE — nothing stored, and |
| +// the sentence says nothing was stored. |
| // |
| -// This pins the copy, not the behaviour — the behaviour is JavaScript |
| -// and was checked in a browser (recipe at the top of this file). Pinning |
| -// the copy is still worth a test, because the copy is where the decision |
| -// lives: the moment someone rewrites this sentence to "only the first |
| -// file was stored", they have decided to truncate silently, and the |
| -// review that lets it through would be a review of a JavaScript diff. |
| +// The number is read off the rendered page, so this cannot pass by |
| +// agreeing with itself: exactly that many files are stored and exactly |
| +// one more is refused. The half that matters is the second — a cap |
| +// applied part-way through a drop would store the first twenty-five of |
| +// twenty-six and report success, which is this codebase's worst failure |
| +// shape wearing a limit as a costume. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| // |
| -// MUTATION-TESTED 2026-09-04: rewriting Many in newDropZone to "Only the |
| -// first file was taken." fails all three assertions at once — no {n}, no |
| -// "one document at a time", and "does not say the drop was refused |
| -// whole". Reverted. |
| -func TestSeveralFilesAtOnceAreRefusedRatherThanTruncated(t *testing.T) { |
| - many := dropZoneAttr(t, get(t, newApp(t), "/").Body.String(), "data-msg-many") |
| +// (a) `len(files) > maxDropFiles` → `len(files) > maxDropFiles*2` in |
| +// createDocument, so the advertised cap is not the enforced one → |
| +// "26 files — one over the advertised cap of 25 — stored 26 |
| +// documents" |
| +// (b) truncating instead of refusing (`files = files[:maxDropFiles]` |
| +// in place of the refusal) → "26 files ... stored 25 documents", |
| +// which is the silent truncation itself |
| +// (c) data-max-files="4" in index.html → "5 files — one over the |
| +// advertised cap of 4 — stored 5 documents": the handler still |
| +// refuses at 25, so page and handler have drifted and the test |
| +// says so from the page's side |
| +func TestTheAdvertisedFileCountCapIsTheHandlersCap(t *testing.T) { |
| + h := newApp(t) |
| + maxFiles := atoi(t, dropZoneAttr(t, get(t, h, "/").Body.String(), "data-max-files")) |
| + if maxFiles < 2 || maxFiles > 200 { |
| + t.Fatalf("data-max-files is %d; the drop built below would prove nothing", maxFiles) |
| + } |
| - if !strings.Contains(many, "{n}") { |
| - t.Errorf("the multi-file refusal does not carry the count placeholder {n}, so it "+ |
| - "cannot tell the person how many files were refused: %q", many) |
| + atCap := make([]file, 0, maxFiles) |
| + for i := 0; i < maxFiles; i++ { |
| + atCap = append(atCap, file{fmt.Sprintf("at-%02d.txt", i), fmt.Sprintf("Passage %d about quarterly revenue and hosting.", i)}) |
| } |
| - if !strings.Contains(many, "one document at a time") { |
| - t.Errorf("the multi-file refusal does not say only one file is taken at a time: %q", many) |
| + submit(t, h, "", "", atCap...) |
| + if n := storedCount(t, h); n != maxFiles { |
| + t.Fatalf("%d files, the advertised cap, stored %d documents", maxFiles, n) |
| } |
| - if !strings.Contains(many, "none of them were taken") { |
| - t.Errorf("the multi-file refusal does not say the drop was refused whole. Silently "+ |
| - "keeping one of five and reporting success is the failure this app treats as "+ |
| - "its worst: %q", many) |
| + |
| + h2 := newApp(t) |
| + overCap := append(atCap, file{"one-too-many.txt", "One more passage about revenue."}) |
| + rec := submit(t, h2, "", "", overCap...) |
| + if n := storedCount(t, h2); n != 0 { |
| + t.Errorf("%d files — one over the advertised cap of %d — stored %d documents. A "+ |
| + "submission cap must refuse the drop whole: storing some of it and reporting "+ |
| + "success is the silent truncation this app treats as its worst failure", |
| + len(overCap), maxFiles, n) |
| + } |
| + said := outcome(t, h2, rec) |
| + if !strings.Contains(said, strconv.Itoa(len(overCap))) || !strings.Contains(said, strconv.Itoa(maxFiles)) { |
| + t.Errorf("the refusal names neither how many were dropped (%d) nor how many are "+ |
| + "taken at a time (%d): %q", len(overCap), maxFiles, said) |
| + } |
| +} |
| + |
| +// Same again for the total-size cap. A drop over it is refused before |
| +// anything is opened, so the answer is "none of it was stored". |
| +// |
| +// This one is enforced twice on purpose and the test does not care |
| +// which half caught it: http.MaxBytesReader caps the BODY (framing |
| +// included) and the handler sums the parts' own sizes against the |
| +// advertised total. A drop landing between the two would otherwise be |
| +// taken, which is why the second check exists at all. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) removing the `total > maxDropBytes` check AND raising the |
| +// MaxBytesReader to `maxDropBytes*4` → "a drop of 23068672 bytes, |
| +// over the advertised 20971520, stored 11 documents" |
| +// (b) data-max-total-bytes="{{.MaxBytes}}" in index.html, so the page |
| +// advertises the per-file cap as the total → "the advertised |
| +// total (2097152) is not larger than the advertised per-file cap |
| +// (2097152); a drop over the total could not be built out of |
| +// legal files". A page that advertises a total it does not have |
| +// fails here rather than passing quietly. |
| +func TestTheAdvertisedTotalCapIsTheHandlersCap(t *testing.T) { |
| + h := newApp(t) |
| + body := get(t, h, "/").Body.String() |
| + total := atoi(t, dropZoneAttr(t, body, "data-max-total-bytes")) |
| + per := atoi(t, dropZoneAttr(t, body, "data-max-bytes")) |
| + if total <= per { |
| + t.Fatalf("the advertised total (%d) is not larger than the advertised per-file cap "+ |
| + "(%d); a drop over the total could not be built out of legal files", total, per) |
| + } |
| + |
| + // Every file legal on its own — at the per-file cap — and one more |
| + // of them than the total allows. Nothing here is refusable except |
| + // the total. |
| + filler := "The quarterly figure was recorded in full and then again.\n\n" |
| + one := strings.Repeat(filler, per/len(filler)+1)[:per] |
| + files := make([]file, 0, total/per+1) |
| + for i := 0; len(files)*per <= total; i++ { |
| + files = append(files, file{fmt.Sprintf("big-%02d.txt", i), one}) |
| + } |
| + rec := submit(t, h, "", "", files...) |
| + if n := storedCount(t, h); n != 0 { |
| + t.Errorf("a drop of %d bytes, over the advertised %d, stored %d documents. The total "+ |
| + "cap must refuse the drop whole, before anything is stored", |
| + len(files)*per, total, n) |
| + } |
| + if said := outcome(t, h, rec); !strings.Contains(said, "20 MiB") { |
| + t.Errorf("the refusal does not name the total it enforces: %q", said) |
| } |
| } |
| // The browser refuses in the handler's own words. A person who drops a |
| // PDF and a person who picks one from the file chooser are told the same |
| -// thing, because it is the same sentence — read off flashFor. |
| +// thing about it, because it is the same sentence with the same |
| +// filename in it — the clause on the drop zone and the clause the |
| +// handler composes come from one field of newDropZone. |
| +// |
| +// The server half is produced by actually posting the file, not by |
| +// reading a string back off a page: a template that renders the clause |
| +// correctly and a handler that composes a different one would otherwise |
| +// agree perfectly. |
| // |
| -// MUTATION-TESTED 2026-09-04, each reverted: (a) data-msg-toobig set to |
| -// a literal "That file is too big." → the toobig mismatch, printing both |
| -// sentences; (b) data-msg-binary pointed at {{.Empty}} → the binary |
| -// mismatch. Editing flashFor itself moves both sides at once and stays |
| +// MUTATION-TESTED 2026-09-04, each reverted: (a) data-reason-binary set |
| +// to a literal "{name} is a binary file." in index.html → the binary |
| +// mismatch, printing both sentences; (b) ReasonEmpty used in place of |
| +// ReasonBinary in readUpload → the same failure from the handler's |
| +// side. Editing the field in newDropZone moves both at once and stays |
| // green, which is what sharing the sentence is for. |
| func TestTheBrowserRefusesInTheHandlersWords(t *testing.T) { |
| h := newApp(t) |
| - tag := get(t, h, "/").Body.String() |
| + page := get(t, h, "/").Body.String() |
| - for _, c := range []struct{ code, attr string }{ |
| - {"toobig", "data-msg-toobig"}, |
| - {"binary", "data-msg-binary"}, |
| + for _, c := range []struct { |
| + attr, name, body string |
| + }{ |
| + {"data-reason-binary", "scan.png", "PNG\x00\x00 not text at all"}, |
| + {"data-reason-empty", "blank.txt", " \n "}, |
| } { |
| - page := dropZoneAttr(t, tag, c.attr) |
| - server := flashText(t, get(t, h, "/?err="+c.code).Body.String()) |
| - if page != server { |
| + // The browser's clause, with the filename in it. |
| + browser := strings.ReplaceAll(dropZoneAttr(t, page, c.attr), "{name}", c.name) |
| + // The handler's clause, produced by refusing the real file. It |
| + // arrives inside the outcome sentence, so this looks for the |
| + // clause within it rather than for equality. |
| + said := outcome(t, h, submit(t, h, "", "", file{c.name, c.body})) |
| + if !strings.Contains(said, browser) { |
| t.Errorf("the drop zone and the handler refuse %s differently.\n browser: %q\n server: %q\n"+ |
| "Two sentences for one rule means one of them is out of date and nobody can tell which.", |
| - c.code, page, server) |
| + c.attr, browser, said) |
| } |
| } |
| } |
| @@ -484,35 +719,52 @@ func TestTheLiveRegionExistsBeforeItHasAnythingToSay(t *testing.T) { |
| } |
| } |
| -// The refusal names the cap it enforces. flashFor's "larger than N MiB", |
| -// the drop zone's "up to N MiB" and maxUpload are one number said three |
| -// times, and only one of them is written down. |
| +// Every refusal names the cap it enforces, and the hint above the input |
| +// names all three. The per-file cap, the total and the file count are |
| +// each one number said in several places, and each is written down once. |
| // |
| // Not pedantry: the refusal used to carry "2 MiB" as a literal while the |
| // hint beside it derived its number from maxUpload. Raising the cap |
| // would have left the page refusing a 3 MiB file with the words "that is |
| // larger than 2 MiB" — and the person, who had just read a hint saying |
| // 4 MiB, would have no way to tell which of the two lines was lying. |
| +// With three caps there are three ways to make that mistake. |
| // |
| -// MUTATION-TESTED 2026-09-04, reverted: putting the literal back — |
| -// `"That is larger than 2 MiB. …"` in flashFor — with maxUpload raised |
| -// to `4 << 20` fails with "the refusal names 2 MiB but the cap it |
| -// enforces is 4 MiB". Changing maxUpload ALONE keeps this green, which |
| -// is the point: one number, said everywhere from one place. |
| -func TestTheRefusalNamesTheCapItEnforces(t *testing.T) { |
| +// The per-file refusal comes from a real over-cap upload rather than |
| +// from a rendered attribute, so a handler composing its own sentence |
| +// cannot pass by agreeing with a template. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `ReasonTooBig: "{name} is larger than 2 MiB"` — the literal put |
| +// back — with maxUpload raised to `4 << 20` → "the refusal names |
| +// 2 MiB but the per-file cap it enforces is 4 MiB" |
| +// (b) `dropLimit()` swapped for `uploadLimit()` in the Hint → "the |
| +// hint does not name the total cap (20 MiB)" |
| +// (c) `fileLimit()` replaced with `"25"` in the Hint, with maxDropFiles |
| +// raised to 30 → "the hint does not name the file-count cap (30)" |
| +// |
| +// Changing any of the three constants ALONE keeps this green, which is |
| +// the point: one number, said everywhere from one place. |
| +func TestTheRefusalsNameTheCapsTheyEnforce(t *testing.T) { |
| h := newApp(t) |
| body := get(t, h, "/").Body.String() |
| capBytes := atoi(t, dropZoneAttr(t, body, "data-max-bytes")) |
| - mib := capBytes >> 20 |
| - if mib < 1 { |
| - t.Fatalf("the advertised cap is %d bytes, under a MiB; this test compares whole "+ |
| - "MiB and would prove nothing", capBytes) |
| + totalBytes := atoi(t, dropZoneAttr(t, body, "data-max-total-bytes")) |
| + maxFiles := atoi(t, dropZoneAttr(t, body, "data-max-files")) |
| + if capBytes>>20 < 1 || totalBytes>>20 < 1 { |
| + t.Fatalf("the advertised caps are %d and %d bytes; this test compares whole MiB and "+ |
| + "would prove nothing", capBytes, totalBytes) |
| } |
| - want := fmt.Sprintf("%d MiB", mib) |
| + want := fmt.Sprintf("%d MiB", capBytes>>20) |
| + wantTotal := fmt.Sprintf("%d MiB", totalBytes>>20) |
| - // The refusal the handler actually renders. |
| - refusal := flashText(t, get(t, h, "/?err=toobig").Body.String()) |
| + // The refusal the handler actually composes, for a real file over |
| + // the real cap. |
| + filler := "The quarterly figure was recorded in full and then again.\n\n" |
| + over := strings.Repeat(filler, capBytes/len(filler)+2)[:capBytes+1] |
| + refusal := outcome(t, h, upload(t, h, "over.txt", over)) |
| said := regexp.MustCompile(`\d+\s*MiB`).FindAllString(refusal, -1) |
| if len(said) == 0 { |
| t.Fatalf("the too-big refusal names no size at all, so it can neither agree nor "+ |
| @@ -520,21 +772,28 @@ func TestTheRefusalNamesTheCapItEnforces(t *testing.T) { |
| } |
| for _, got := range said { |
| if got != want { |
| - t.Errorf("the refusal names %s but the cap it enforces is %s (%d bytes, read off "+ |
| - "the same page the browser reads). One of the two moved and the other did "+ |
| - "not: %q", got, want, capBytes, refusal) |
| + t.Errorf("the refusal names %s but the per-file cap it enforces is %s (%d bytes, "+ |
| + "read off the same page the browser reads). One of the two moved and the "+ |
| + "other did not: %q", got, want, capBytes, refusal) |
| } |
| } |
| - // And the hint above the input says the same thing, because it is |
| - // the same string from the same place. |
| + // And the hint above the input says all three, because they are the |
| + // same numbers from the same place. |
| hint := regexp.MustCompile(`<p rst-dropzone-hint[^>]*>([^<]*)</p>`).FindStringSubmatch(body) |
| if hint == nil { |
| t.Fatalf("no drop zone hint on the page:\n%s", body) |
| } |
| - if !strings.Contains(hint[1], want) { |
| - t.Errorf("the hint says %q while the refusal says %s; a person is told one limit "+ |
| - "before the upload and a different one after it", strings.TrimSpace(hint[1]), want) |
| + for _, w := range []struct{ what, value string }{ |
| + {"per-file cap", want}, |
| + {"total cap", wantTotal}, |
| + {"file-count cap", strconv.Itoa(maxFiles)}, |
| + } { |
| + if !strings.Contains(hint[1], w.value) { |
| + t.Errorf("the hint does not name the %s (%s): %q — a person is told one set of "+ |
| + "limits before the upload and a different one after it", |
| + w.what, w.value, strings.TrimSpace(hint[1])) |
| + } |
| } |
| } |
| @@ -574,3 +833,339 @@ func atoi(t *testing.T, s string) int { |
| } |
| return n |
| } |
| + |
| +// ---- one submission, several documents -------------------------------- |
| + |
| +// Five files in, five documents out — each with its own title, its own |
| +// chunks and its own row. |
| +// |
| +// The assertions are on the document TABLE, never on the page: the front |
| +// page renders the search results, the flash and the list, and the flash |
| +// alone contains the words "5 documents". A filename appearing somewhere |
| +// in the HTML is the false pass this whole file is written against. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `for _, hdr := range files` → `for _, hdr := range files[:1]` in |
| +// createDocument → "5 files stored 1 documents: [alpha]" — the |
| +// old single-file behaviour, caught |
| +// (b) `stored++` moved outside the loop (a plain `stored = 1` after |
| +// it) → "the outcome sentence does not say how many documents |
| +// were stored: "Stored 1 document."" |
| +func TestEveryFileInADropBecomesItsOwnDocument(t *testing.T) { |
| + h := newApp(t) |
| + files := []file{ |
| + {"alpha.md", "## Revenue\nAlpha revenue was 100 in the quarter just gone."}, |
| + {"beta.md", "## Revenue\nBeta revenue was 200 in the quarter just gone."}, |
| + {"gamma.md", "## Costs\nGamma hosting fell after the migration completed."}, |
| + {"delta.md", "## Risks\nDelta concentration is the single largest risk."}, |
| + {"epsilon.md", "## Notes\nEpsilon minutes from the August meeting."}, |
| + } |
| + rec := submit(t, h, "", "", files...) |
| + |
| + titles := storedTitles(t, follow(t, h, rec).Body.String()) |
| + if len(titles) != len(files) { |
| + t.Fatalf("%d files stored %d documents: %v", len(files), len(titles), titles) |
| + } |
| + for _, f := range files { |
| + want := strings.TrimSuffix(f.name, ".md") |
| + if !slices.Contains(titles, want) { |
| + t.Errorf("%q is not in the document list: %v", want, titles) |
| + } |
| + } |
| + // Each is separately indexed, not one document with five sections. |
| + for _, term := range []string{"epsilon", "gamma"} { |
| + if n := strings.Count(get(t, h, "/?q="+term).Body.String(), "rst-result-title"); n != 1 { |
| + t.Errorf("searching %q returns %d results, want 1; the files did not become "+ |
| + "separate documents", term, n) |
| + } |
| + } |
| + if said := outcome(t, h, rec); !strings.Contains(said, "Stored 5 documents") { |
| + t.Errorf("the outcome sentence does not say how many documents were stored: %q", said) |
| + } |
| +} |
| + |
| +// The half that matters: some of a drop is refused and the rest is |
| +// stored anyway, and the person is told BOTH — how many landed, and |
| +// every refusal by name with the reason for that file. |
| +// |
| +// Storing four of five while saying "Stored 4 documents." and nothing |
| +// else is the silent truncation this codebase treats as its worst |
| +// failure. So is refusing one without saying which one. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `refused = append(...)` in the binary branch of readUpload |
| +// replaced with a bare `continue` (refuse it, say nothing) → |
| +// "nothing in the outcome names the refused file: "Stored 4 |
| +// documents."" |
| +// (b) the per-file refusal replaced by a whole-drop refusal (`a.refuse` |
| +// and return, instead of appending) → "4 good files among 5 |
| +// stored 2 documents: [two one]" — the two that happened to come |
| +// before the bad one, which is the half-taken drop itself |
| +// (c) `fill(c.ReasonBinary, ...)` → `fill(c.ReasonEmpty, ...)` for the |
| +// binary case → "the refusal gives the wrong reason for |
| +// scan.png": it said "is empty" for a file that is not text |
| +func TestAPartialDropStoresTheGoodOnesAndNamesTheBad(t *testing.T) { |
| + h := newApp(t) |
| + rec := submit(t, h, "", "", |
| + file{"one.txt", "Revenue was 100 for the first region we serve."}, |
| + file{"two.txt", "Revenue was 200 for the second region we serve."}, |
| + file{"scan.png", "\x89PNG\x00\x00\x00 binary payload that is not text"}, |
| + file{"three.txt", "Revenue was 300 for the third region we serve."}, |
| + file{"four.txt", "Revenue was 400 for the fourth region we serve."}, |
| + ) |
| + |
| + page := follow(t, h, rec).Body.String() |
| + titles := storedTitles(t, page) |
| + if len(titles) != 4 { |
| + t.Fatalf("4 good files among 5 stored %d documents: %v", len(titles), titles) |
| + } |
| + if slices.Contains(titles, "scan") { |
| + t.Errorf("the binary file was stored anyway: %v", titles) |
| + } |
| + |
| + said := flashText(t, page) |
| + if !strings.Contains(said, "Stored 4 documents") { |
| + t.Errorf("the outcome does not say how many landed: %q", said) |
| + } |
| + if !strings.Contains(said, "scan.png") { |
| + t.Errorf("nothing in the outcome names the refused file: %q", said) |
| + } |
| + if !strings.Contains(said, "1 refused") { |
| + t.Errorf("the outcome does not say how many were refused: %q", said) |
| + } |
| + if !strings.Contains(said, "scan.png is not text") { |
| + t.Errorf("the refusal gives the wrong reason for scan.png: %q", said) |
| + } |
| + // And the good ones are not named as refusals. The clause list is |
| + // everything after the count, so this is where a reason attached to |
| + // the wrong file would show up. Guarded on the count being there at |
| + // all: a missing "1 refused" is already reported above, and slicing |
| + // from -1 would panic rather than fail. |
| + if at := strings.Index(said, "1 refused"); at >= 0 { |
| + for _, good := range []string{"one.txt", "two.txt", "three.txt", "four.txt"} { |
| + if strings.Contains(said[at:], good) { |
| + t.Errorf("%s was stored but appears in the refusal list: %q", good, said) |
| + } |
| + } |
| + } |
| +} |
| + |
| +// The count in the sentence is the number of documents that exist. |
| +// |
| +// "A count that fails is not a count of zero" is already an AGENTS.md |
| +// rule; this is its neighbour — a count that is written by hand beside |
| +// the loop that does the work drifts from it the first time the loop |
| +// gains a branch. So the number is read out of the sentence and compared |
| +// with the rows in the table. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `stored++` → `stored += 2` in the file loop → "the outcome says |
| +// 7 documents were stored but the list holds 4" |
| +// (b) `c.storedPart(stored)` → `c.storedPart(stored+len(refused))` in |
| +// outcome(), the plausible "say how many we looked at" edit → |
| +// "the outcome says 6 documents were stored but the list holds 4" |
| +// (c) `stored++` deleted from the paste branch → "the outcome says 3 |
| +// documents were stored but the list holds 4" |
| +func TestTheStoredCountIsTheNumberActuallyStored(t *testing.T) { |
| + h := newApp(t) |
| + rec := submit(t, h, "Minutes", "The August meeting agreed the migration date.", |
| + file{"good-one.txt", "Revenue was 100 across the first region."}, |
| + file{"bad-one.png", "\x00\x00 binary"}, |
| + file{"good-two.txt", "Revenue was 200 across the second region."}, |
| + file{"bad-two.png", "\x00\x00 binary"}, |
| + file{"good-three.txt", "Revenue was 300 across the third region."}, |
| + ) |
| + |
| + page := follow(t, h, rec).Body.String() |
| + rows := len(storedTitles(t, page)) |
| + said := flashText(t, page) |
| + |
| + m := regexp.MustCompile(`Stored (\d+) documents?\.`).FindStringSubmatch(said) |
| + if m == nil { |
| + t.Fatalf("the outcome sentence states no stored count, so nothing here can be "+ |
| + "compared: %q", said) |
| + } |
| + if got := atoi(t, m[1]); got != rows { |
| + t.Errorf("the outcome says %d documents were stored but the list holds %d.\n %q\n %v", |
| + got, rows, said, storedTitles(t, page)) |
| + } |
| + // Three files and the paste box: four. Stated so a change to the |
| + // fixture cannot quietly make the comparison above trivial. |
| + if rows != 4 { |
| + t.Errorf("3 good files and a paste stored %d documents, want 4: %v", rows, storedTitles(t, page)) |
| + } |
| +} |
| + |
| +// Dropping an edited file again UPDATES its document. The filename is |
| +// the stable key, exactly as it is for a single upload, so a second drop |
| +// of the same five files is five updates and not five more rows. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `a.store.Ingest(ctx, "upload", hdr.Filename, …)` → `Ingest(ctx, |
| +// "upload", "", …)` in createDocument, so the key falls back to |
| +// the title → still green (the title is derived from the filename, |
| +// so it is the same key by another route). Recorded because it is |
| +// the mutation a reader will try first and it proves nothing. |
| +// (b) the key changed back to the body — `Ingest(ctx, "upload", |
| +// string(raw), …)`, the old content-hash scheme → "a re-drop of 2 |
| +// files made 3 documents" and "the superseded passage is still |
| +// searchable", which is the duplication this test exists for |
| +// (c) the stale-passage half: `s.ix.RemoveDocument(docID)` commented |
| +// out in Store.Ingest → "the superseded passage is still |
| +// searchable" |
| +func TestRedroppingFilesUpdatesThemRatherThanDuplicating(t *testing.T) { |
| + h := newApp(t) |
| + submit(t, h, "", "", |
| + file{"board.md", "## Revenue\nThe zebrafinch figure was 100 for the quarter."}, |
| + file{"minutes.md", "## Present\nEveryone was at the August meeting."}, |
| + ) |
| + submit(t, h, "", "", |
| + file{"board.md", "## Revenue\nThe wildebeest figure was 200 for the quarter."}, |
| + file{"minutes.md", "## Present\nEveryone was at the August meeting."}, |
| + ) |
| + |
| + if n := storedCount(t, h); n != 2 { |
| + t.Errorf("a re-drop of 2 files made %d documents", n) |
| + } |
| + if got := get(t, h, "/?q=zebrafinch").Body.String(); strings.Contains(got, "<mark>") { |
| + t.Error("the superseded passage is still searchable; a citation would point at text " + |
| + "that is no longer there") |
| + } |
| + if got := get(t, h, "/?q=wildebeest").Body.String(); !strings.Contains(got, "<mark>") { |
| + t.Error("the edited passage was not indexed") |
| + } |
| +} |
| + |
| +// The paste box is one more document, and files no longer overwrite it. |
| +// |
| +// This was real data loss: a file in the same submission silently |
| +// replaced whatever had been typed, and the page said "Stored, chunked |
| +// and indexed." With one document per file the conflict is gone — |
| +// everything submitted is stored, and the title field titles the paste. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `if body != ""` → `if body != "" && len(files) == 0`, which is |
| +// what the old overwrite amounted to → "a paste submitted |
| +// alongside 2 files stored 2 documents, want 3: [costs revenue]" |
| +// (b) `Ingest(ctx, "paste", title, title, body)` → `Ingest(ctx, |
| +// "paste", title, "", body)`, so the title field stops titling |
| +// the paste → "the paste did not become a document under its |
| +// title: [The August meeting agreed the pangolin timetable. costs |
| +// revenue]" |
| +func TestThePasteBoxIsOneMoreDocumentAndIsNeverDiscarded(t *testing.T) { |
| + h := newApp(t) |
| + rec := submit(t, h, "Board minutes", "The August meeting agreed the pangolin timetable.", |
| + file{"revenue.txt", "Revenue was 100 across the first region we serve."}, |
| + file{"costs.txt", "Hosting costs fell after the migration completed."}, |
| + ) |
| + |
| + page := follow(t, h, rec).Body.String() |
| + titles := storedTitles(t, page) |
| + if len(titles) != 3 { |
| + t.Fatalf("a paste submitted alongside 2 files stored %d documents, want 3: %v", |
| + len(titles), titles) |
| + } |
| + if !slices.Contains(titles, "Board minutes") { |
| + t.Errorf("the paste did not become a document under its title: %v", titles) |
| + } |
| + if n := strings.Count(get(t, h, "/?q=pangolin").Body.String(), "rst-result-title"); n != 1 { |
| + t.Errorf("the pasted text is not searchable: it was discarded. %d results for a term "+ |
| + "only it contains, want 1", n) |
| + } |
| + // The files kept their own names rather than being titled by the |
| + // paste's title field. |
| + for _, want := range []string{"revenue", "costs"} { |
| + if !slices.Contains(titles, want) { |
| + t.Errorf("%q is not in the document list; the title field took a file's name: %v", want, titles) |
| + } |
| + } |
| +} |
| + |
| +// The scriptless path. AGENTS.md: every affordance works scriptless, and |
| +// multi-file upload is one — `multiple` is plain HTML and the handler |
| +// change is server-side, so a browser with JavaScript switched off must |
| +// store all five. |
| +// |
| +// Go cannot switch JavaScript off, so this asserts the two things that |
| +// make the claim true and then exercises the request they produce: the |
| +// rendered form is a plain multipart form with a `multiple` input named |
| +// "file", and a POST of several parts under that one name — which is |
| +// exactly what such a form submits — stores several documents. No part |
| +// of the request comes from drop.js; the harness builds it from the |
| +// attributes read off the page. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) removing `multiple` from index.html → "the rendered file input |
| +// is not `multiple`, so a scriptless chooser can offer only one |
| +// file" |
| +// (b) `enctype="multipart/form-data"` deleted from the form → "the |
| +// upload form is not multipart, so a scriptless browser would post |
| +// filenames and no file contents" |
| +// (c) `for _, hdr := range files` → `files[:1]` in createDocument → |
| +// "a scriptless multipart POST of 3 parts named "file" stored 1 |
| +// document" |
| +func TestTheScriptlessFormStoresEveryFileItSubmits(t *testing.T) { |
| + h := newApp(t) |
| + body := get(t, h, "/").Body.String() |
| + |
| + i := strings.Index(body, `action="/documents"`) |
| + if i < 0 { |
| + t.Fatalf("no upload form on the page:\n%s", body) |
| + } |
| + j := strings.Index(body[i:], "</form>") |
| + if j < 0 { |
| + t.Fatal("the upload form never closes") |
| + } |
| + form := body[i : i+j] |
| + if !strings.Contains(body[max(i-200, 0):i+j], `enctype="multipart/form-data"`) { |
| + t.Errorf("the upload form is not multipart, so a scriptless browser would post "+ |
| + "filenames and no file contents:\n%s", form) |
| + } |
| + input := regexp.MustCompile(`<input[^>]*type="file"[^>]*>`).FindString(form) |
| + if !regexp.MustCompile(`\bmultiple\b`).MatchString(input) { |
| + t.Errorf("the rendered file input is not `multiple`, so a scriptless chooser can "+ |
| + "offer only one file: %s", input) |
| + } |
| + name := regexp.MustCompile(`name="([^"]*)"`).FindStringSubmatch(input) |
| + if name == nil { |
| + t.Fatalf("the file input has no name: %s", input) |
| + } |
| + |
| + // The request that form makes: several parts, one name, no script. |
| + var buf bytes.Buffer |
| + mw := multipart.NewWriter(&buf) |
| + parts := []file{ |
| + {"first.txt", "Revenue was 100 across the first region we serve."}, |
| + {"second.txt", "Revenue was 200 across the second region we serve."}, |
| + {"third.txt", "Revenue was 300 across the third region we serve."}, |
| + } |
| + for _, p := range parts { |
| + w, err := mw.CreateFormFile(name[1], p.name) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := w.Write([]byte(p.body)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + mw.Close() |
| + req := httptest.NewRequest(http.MethodPost, "/documents", &buf) |
| + req.Header.Set("Content-Type", mw.FormDataContentType()) |
| + req.Header.Set("Origin", testOrigin) |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + |
| + if n := storedCount(t, h); n != len(parts) { |
| + t.Errorf("a scriptless multipart POST of %d parts named %q stored %d documents", |
| + len(parts), name[1], n) |
| + } |
| + if said := outcome(t, h, rec); !strings.Contains(said, "Stored 3 documents") { |
| + t.Errorf("the scriptless path does not report what it stored: %q", said) |
| + } |
| +} |
diff --git a/internal/memoriatest/gates_test.go b/internal/memoriatest/gates_test.go| index f6086ba..8061b9c 100644 |
| --- a/internal/memoriatest/gates_test.go |
| +++ b/internal/memoriatest/gates_test.go |
| @@ -3,6 +3,7 @@ package memoriatest |
| import ( |
| "encoding/base64" |
| "errors" |
| + "fmt" |
| "net/http" |
| "os" |
| "path/filepath" |
| @@ -40,7 +41,8 @@ func TestPlaintextNeverReachesTheDatabase(t *testing.T) { |
| // The vacuity guard, and it has to be this specific. An earlier |
| // version checked `rec.Code != 303` and then searched for the string |
| // "result" — but the handler redirects with 303 on FAILURE too |
| - // (/?err=store), and "result" is a substring of "0 results". With |
| + // (/?err=empty, /?stored=0&refused=1), and "result" is a substring |
| + // of "0 results". With |
| // Ingest mutated to always error, that version PASSED against an |
| // empty database: the gate below was scanning a file with nothing in |
| // it. Assert the success location and real result markup. |
| @@ -78,6 +80,79 @@ func TestPlaintextNeverReachesTheDatabase(t *testing.T) { |
| } |
| } |
| +// Gate 1's other half: no plaintext reaches a TEMP file either. |
| +// |
| +// ParseMultipartForm's argument is a MEMORY threshold, and Go spends it |
| +// cumulatively across parts — it holds file parts in memory until the |
| +// running total passes the threshold and writes every part after that to |
| +// a temp file, in the clear, with no error raised anywhere. One 2 MiB |
| +// file could not reach that with the old single-file cap. Twenty-five |
| +// can, trivially, which is why this gate exists now and did not before. |
| +// |
| +// It works because httptest calls the handler directly: net/http's own |
| +// server calls MultipartForm.RemoveAll() when the request finishes, so |
| +// in production a spilled file is deleted — after having been written. |
| +// Here nothing deletes it, so a spill is still on disk to be found. |
| +// |
| +// MUTATION-TESTED 2026-09-04: ParseMultipartForm's argument changed back |
| +// to maxUpload — the per-file cap, which is what it was before |
| +// multi-file — makes this fail naming the spilled file (the suffix is |
| +// random, e.g. "multipart-578803712 in the temp directory holds the |
| +// canary in plaintext: Go spilled a part to disk") and again with |
| +// "1 files were left in the temp directory". Reverted. |
| +func TestAMultiFileDropNeverSpillsPlaintextToATempFile(t *testing.T) { |
| + // The app first, so its database does not land in the directory |
| + // being scanned: TMPDIR is what os.CreateTemp and t.TempDir both |
| + // read, and the point here is to watch one specific directory. |
| + h := newApp(t) |
| + spill := t.TempDir() |
| + t.Setenv("TMPDIR", spill) |
| + |
| + // Twelve parts of half a MiB. Under any per-file threshold this |
| + // passes the budget several times over, because the budget is spent |
| + // across parts and not per part. |
| + body := strings.Repeat(canary+" and more revenue text to pad this passage out.\n\n", 8000) |
| + files := make([]file, 0, 12) |
| + for i := 0; i < 12; i++ { |
| + files = append(files, file{fmt.Sprintf("spill-%02d.txt", i), body}) |
| + } |
| + rec := submit(t, h, "", "", files...) |
| + |
| + // The vacuity guard. A submission that was refused would leave the |
| + // directory empty and this gate would pass having watched nothing. |
| + if loc := rec.Header().Get("Location"); loc != "/?stored=12" { |
| + t.Fatalf("the drop answered %q rather than storing all 12; the scan below would "+ |
| + "prove nothing", loc) |
| + } |
| + |
| + found := 0 |
| + err := filepath.Walk(spill, func(p string, info os.FileInfo, err error) error { |
| + if err != nil || info.IsDir() { |
| + return err |
| + } |
| + found++ |
| + raw, err := os.ReadFile(p) |
| + if err != nil { |
| + return err |
| + } |
| + if bytesContainsFold(raw, canary) { |
| + t.Errorf("%s in the temp directory holds the canary in plaintext: Go spilled a "+ |
| + "part to disk. ParseMultipartForm's argument is a memory threshold, not a "+ |
| + "limit, and it is spent across parts — it must be at least the whole body "+ |
| + "cap or a large drop writes somebody's documents to /tmp in the clear.", |
| + filepath.Base(p)) |
| + } |
| + return nil |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if found > 0 { |
| + t.Errorf("%d files were left in the temp directory by a drop that should have been "+ |
| + "parsed entirely in memory", found) |
| + } |
| +} |
| + |
| // Gate 15's egress half. With the assistant off, nothing leaves the |
| // instance — and in slice 0 there is no assistant at all, so the honest |
| // form of this gate is that no app package can make an outbound request. |
| @@ -169,17 +244,85 @@ func TestFooterDoesNotOverclaim(t *testing.T) { |
| // ships it. It did not, which is how a mailer that logs the magic link |
| // went unnoticed. |
| // |
| -// MUTATION-TESTED 2026-09-04: logging the title in Store.Ingest's error |
| -// path, or the query in the index handler, makes this fail. Reverted. |
| +// A FILENAME is content too, and that is the half multi-file upload |
| +// makes easy to get wrong: a refusal now names the file it refused, and |
| +// the obvious next line to write is that same name into a log. So the |
| +// secret is posted as a filename as well — once on a file that stores |
| +// and once on a file that is refused, because the refusal path is where |
| +// a filename is nearest to hand. |
| +// |
| +// The asynchronous path is exercised too, and for a sharper reason than |
| +// symmetry. It is the path where a filename is nearest to hand — a |
| +// request that answers per-file is the natural place to log which file it |
| +// answered about — and it is the one path whose RESPONSE body is allowed |
| +// to carry a filename, which makes it easy to forget that the log beside |
| +// it is not. So the same secret goes up over that path as well, on a file |
| +// that stores and a file that is refused. |
| +// |
| +// The store-failure line is reached DELIBERATELY, and the reason is the |
| +// whole of this comment's history. This gate used to claim, in a |
| +// MUTATION-TESTED note, that adding `"file", name` to createDocument's |
| +// ingest-failure log line made it go red. A code review replayed that |
| +// mutation and it stayed GREEN: nothing here made Store.Ingest fail — |
| +// the secret-named .png files are refused in readUpload well before that |
| +// line, and the .txt files store cleanly — so the line under test never |
| +// executed and the claim was false. That is exactly the shape AGENTS.md |
| +// opens with, written on the guard for the log line nearest a filename. |
| +// The database is now closed under the running app so that the line runs, |
| +// and its presence is asserted before its contents are, because a gate |
| +// that checks for an absence has to prove first that the thing whose |
| +// absence it checks was there to be found. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted, each observed red: |
| +// |
| +// (a) `"file", name` added to createDocument's ingest-failure log line |
| +// → "a log line carries document content, a filename or the query", |
| +// printing the buffer with WOMBAT…-fails.txt in it. This is the |
| +// mutation the previous version of this comment claimed and did not |
| +// have. |
| +// (b) Store.Ingest's post-transaction error return wrapped as |
| +// `fmt.Errorf("ingest %q: %w", title, err)` — the plausible "which |
| +// document was it?" edit, and the one that puts content in an ERROR |
| +// STRING rather than a log call → the same failure, printing |
| +// err="ingest \"WOMBAT…-fails\": sql: database is closed". |
| +// (c) `a.logger.Info("search", "q", q)` in the index handler |
| +// → the same failure, printing q=WOMBAT… for both searches. |
| +// (d) the vacuity guard's own subject: removing `d.Close()` → "the |
| +// ingest-failure log line never ran", which is the state this gate |
| +// was silently in before. |
| func TestNoContentInLogs(t *testing.T) { |
| - h, logs := newAppWithLog(t) |
| + h, logs, d := newAppWithLog(t) |
| const secret = "WOMBATCONFIDENTIAL9931" |
| store(t, h, secret+" title", "## "+secret+"\n\nA passage about "+secret+" and revenue.") |
| + submit(t, h, "", "", |
| + file{secret + "-good.txt", "A passage about hosting costs and the migration."}, |
| + file{secret + "-refused.png", "\x00\x00 binary payload"}, |
| + ) |
| + // The same again, asynchronously. One file per request, as the tray |
| + // sends them. |
| + submitJSON(t, h, file{secret + "-async.txt", "A passage about the migration date."}) |
| + submitJSON(t, h, file{secret + "-async.png", "\x00\x00 binary payload"}) |
| get(t, h, "/?q="+secret) |
| get(t, h, "/?q=revenue") |
| + |
| + // Now a file whose store genuinely fails. Last, because it takes the |
| + // database away from everything above. |
| + d.Close() |
| + reply := readReply(t, submitJSON(t, h, file{secret + "-fails.txt", "A passage about the migration date."})) |
| + if reply.Stored != 0 || len(reply.Refused) != 1 || |
| + !strings.Contains(reply.Refused[0], secret+"-fails.txt") { |
| + t.Fatalf("the store did not fail, so the log line this gate is aimed at never ran and "+ |
| + "everything below it proves nothing: stored=%d refused=%v", |
| + reply.Stored, reply.Refused) |
| + } |
| + |
| got := logs.String() |
| + if !strings.Contains(got, "ingest failed") { |
| + t.Fatalf("nothing in the log says a store failed, so the line that names a file was "+ |
| + "never written and the check below is vacuous:\n%s", got) |
| + } |
| if strings.Contains(got, secret) { |
| - t.Errorf("a log line carries document content or the query:\n%s", got) |
| + t.Errorf("a log line carries document content, a filename or the query:\n%s", got) |
| } |
| for _, bad := range []string{"sign-in link", "auth/verify?"} { |
| if strings.Contains(got, bad) { |
diff --git a/internal/memoriatest/harness_test.go b/internal/memoriatest/harness_test.go| index 373458e..61d9c54 100644 |
| --- a/internal/memoriatest/harness_test.go |
| +++ b/internal/memoriatest/harness_test.go |
| @@ -22,6 +22,7 @@ import ( |
| "net/http/httptest" |
| "net/url" |
| "path/filepath" |
| + "regexp" |
| "strings" |
| "testing" |
| @@ -73,7 +74,16 @@ func newAppAt(t *testing.T, path string) (http.Handler, string) { |
| // newAppWithLog is newApp with the log captured, for the gate that |
| // asserts no content reaches a log line. |
| -func newAppWithLog(t *testing.T) (http.Handler, *bytes.Buffer) { |
| +// |
| +// The database handle comes back too, and it is not a convenience. The |
| +// log line that gate is aimed at is the one createDocument writes when a |
| +// store FAILS, and nothing a well-formed request can carry makes it fail |
| +// — a binary file is refused before it and a text file stores. Closing |
| +// this handle under the running app is how that path is reached, so the |
| +// gate can watch the line it names actually run. Closing it twice is |
| +// safe: db.Close is sql.DB.Close, which is idempotent, and the cleanup |
| +// below still runs. |
| +func newAppWithLog(t *testing.T) (http.Handler, *bytes.Buffer, *db.DB) { |
| t.Helper() |
| var buf bytes.Buffer |
| logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})) |
| @@ -86,7 +96,7 @@ func newAppWithLog(t *testing.T) (http.Handler, *bytes.Buffer) { |
| if err != nil { |
| t.Fatalf("AppForTest: %v", err) |
| } |
| - return mux, &buf |
| + return mux, &buf, d |
| } |
| // newProductionApp builds the app the way main.go does — no test seam — |
| @@ -147,16 +157,38 @@ func store(t *testing.T, h http.Handler, title, body string) *httptest.ResponseR |
| // upload posts a document as a file, so it carries a filename — the |
| // stable key that decides whether a re-ingest updates or duplicates. |
| func upload(t *testing.T, h http.Handler, filename, body string) *httptest.ResponseRecorder { |
| + t.Helper() |
| + return submit(t, h, "", "", file{filename, body}) |
| +} |
| + |
| +// file is one part of a submission. The zero title/body of submit plus |
| +// several of these is a drop; a title and body with no files is a paste. |
| +type file struct{ name, body string } |
| + |
| +// submit posts the upload form exactly as a browser would: the title and |
| +// body fields, then one part named "file" per file. Several parts under |
| +// ONE name is what `multiple` produces and what createDocument iterates, |
| +// so this is the scriptless path with no script involved — which is the |
| +// only way Go can exercise it. |
| +func submit(t *testing.T, h http.Handler, title, body string, files ...file) *httptest.ResponseRecorder { |
| t.Helper() |
| var buf bytes.Buffer |
| mw := multipart.NewWriter(&buf) |
| - w, err := mw.CreateFormFile("file", filename) |
| - if err != nil { |
| + if err := mw.WriteField("title", title); err != nil { |
| t.Fatal(err) |
| } |
| - if _, err := w.Write([]byte(body)); err != nil { |
| + if err := mw.WriteField("body", body); err != nil { |
| t.Fatal(err) |
| } |
| + for _, f := range files { |
| + w, err := mw.CreateFormFile("file", f.name) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := w.Write([]byte(f.body)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| mw.Close() |
| req := httptest.NewRequest(http.MethodPost, "/documents", &buf) |
| req.Header.Set("Content-Type", mw.FormDataContentType()) |
| @@ -165,3 +197,102 @@ func upload(t *testing.T, h http.Handler, filename, body string) *httptest.Respo |
| h.ServeHTTP(rec, req) |
| return rec |
| } |
| + |
| +// submitJSON posts what the upload tray posts: the same multipart body |
| +// at the same route, with an Accept header asking for the judgement |
| +// instead of a redirect and a flash cookie. |
| +// |
| +// It goes through the SAME handler as submit(), and that is the point of |
| +// having it here rather than a second fixture: every test that uses it is |
| +// asking whether the asynchronous path judges a file exactly as the |
| +// scriptless one does. |
| +// |
| +// The tray sends one file per request. This takes several so the caps — |
| +// which are submission-wide and which the tray never triggers on its own |
| +// — can still be aimed at this path, because a browser is not the only |
| +// thing that can send an Accept header. |
| +func submitJSON(t *testing.T, h http.Handler, files ...file) *httptest.ResponseRecorder { |
| + t.Helper() |
| + var buf bytes.Buffer |
| + mw := multipart.NewWriter(&buf) |
| + for _, f := range files { |
| + w, err := mw.CreateFormFile("file", f.name) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := w.Write([]byte(f.body)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + mw.Close() |
| + req := httptest.NewRequest(http.MethodPost, "/documents", &buf) |
| + req.Header.Set("Content-Type", mw.FormDataContentType()) |
| + req.Header.Set("Origin", testOrigin) |
| + req.Header.Set("Accept", "application/json") |
| + rec := httptest.NewRecorder() |
| + h.ServeHTTP(rec, req) |
| + return rec |
| +} |
| + |
| +// follow does what a browser does with a 303: it GETs the Location, |
| +// carrying back whatever cookies the response set. |
| +// |
| +// The cookie matters. The sentence that NAMES refused files travels in a |
| +// one-shot flash cookie rather than the URL, because a filename is |
| +// content and a redirect target is the sort of thing platform logging |
| +// keeps. A test that does not carry the cookie sees only the counts, so |
| +// every assertion about naming has to come through here. |
| +func follow(t *testing.T, h http.Handler, rec *httptest.ResponseRecorder) *httptest.ResponseRecorder { |
| + t.Helper() |
| + loc := rec.Header().Get("Location") |
| + if loc == "" { |
| + t.Fatalf("the response set no Location (status %d), so there is nothing to follow", rec.Code) |
| + } |
| + req := httptest.NewRequest(http.MethodGet, loc, nil) |
| + for _, c := range rec.Result().Cookies() { |
| + req.AddCookie(c) |
| + } |
| + out := httptest.NewRecorder() |
| + h.ServeHTTP(out, req) |
| + return out |
| +} |
| + |
| +// outcome posts nothing: it follows a submission's redirect and returns |
| +// the sentence the person is actually shown. |
| +func outcome(t *testing.T, h http.Handler, rec *httptest.ResponseRecorder) string { |
| + t.Helper() |
| + return flashText(t, follow(t, h, rec).Body.String()) |
| +} |
| + |
| +// storedTitles returns the titles in the document list, scoped to the |
| +// table and nothing else. |
| +// |
| +// Scoped because the front page renders the search results, the flash |
| +// and the whole document list, so "the filename appears in the HTML" is |
| +// a false pass waiting to happen — the flash alone says "Stored 4 |
| +// documents", which contains "4 documents", which is also what the |
| +// list's own heading says. Only the table rows are evidence that a |
| +// document exists. |
| +func storedTitles(t *testing.T, body string) []string { |
| + t.Helper() |
| + i := strings.Index(body, "<table rst-table") |
| + if i < 0 { |
| + return nil // no table at all: nothing is stored |
| + } |
| + j := strings.Index(body[i:], "</table>") |
| + if j < 0 { |
| + t.Fatal("the document table never closes") |
| + } |
| + rows := regexp.MustCompile(`<td><a href="/documents/\d+">([^<]*)</a></td>`).FindAllStringSubmatch(body[i:i+j], -1) |
| + out := make([]string, 0, len(rows)) |
| + for _, r := range rows { |
| + out = append(out, unescape(r[1])) |
| + } |
| + return out |
| +} |
| + |
| +// storedCount is storedTitles' length, said as a count. |
| +func storedCount(t *testing.T, h http.Handler) int { |
| + t.Helper() |
| + return len(storedTitles(t, get(t, h, "/").Body.String())) |
| +} |
diff --git a/internal/memoriatest/tray_test.go b/internal/memoriatest/tray_test.go| new file mode 100644 |
| index 0000000..7bed79b |
| --- /dev/null |
| +++ b/internal/memoriatest/tray_test.go |
| @@ -0,0 +1,565 @@ |
| +package memoriatest |
| + |
| +import ( |
| + "encoding/json" |
| + "fmt" |
| + "net/http" |
| + "net/http/httptest" |
| + "os" |
| + "path/filepath" |
| + "regexp" |
| + "strconv" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// The tests in this file cover the asynchronous half of upload: a drop |
| +// posts each file on its own, in the background, and a tray in the corner |
| +// says how each one went. drop_test.go covers the drag itself and the |
| +// scriptless form underneath; read its header first, including the CDP |
| +// recipe for driving a real drop, because Go still cannot run the script. |
| +// |
| +// The safety argument for this feature is one sentence: there is ONE |
| +// handler. The tray's request differs from the form's only in an Accept |
| +// header, so the caps, the NUL sniff, the whole-body limit that keeps |
| +// parts off the disk and the same-origin check are not "also applied" to |
| +// it — they are the same lines of code, and |
| +// TestTheJSONPathStillEnforcesEveryServerCheck exists to notice the day |
| +// somebody adds a shortcut that skips them. |
| +// |
| +// What Go CANNOT prove here, and what was checked in a browser instead |
| +// (Chromium 151, 2026-09-04, over CDP, recipe in drop_test.go's header): |
| +// that the tray fills as files upload, that the bar stops claiming a |
| +// percentage the moment the body is sent, that a refusal lands in its own |
| +// file's row rather than as one sentence for the whole drop, that |
| +// dismissing works, and that the scriptless page is unchanged. What the |
| +// run found: five files dropped at once gave five rows; the three text |
| +// files went "Waiting…" then "Uploading …%" then "Storing…" then |
| +// "Stored"; scan.png and blank.txt settled immediately, before any |
| +// request, with "scan.png is not text" and "blank.txt is empty" in their |
| +// own rows; the summary line read "Stored 3 documents. 2 refused: |
| +// scan.png is not text; blank.txt is empty." and the stale notice |
| +// appeared with its reload link; the page never navigated; the document |
| +// list below was still showing its old three rows until the link was |
| +// followed, which is the point of not reloading under someone. |
| + |
| +// trayReply is the JSON contract, spelled out here so a change to it |
| +// fails a test rather than a browser. The tags are the wire names. |
| +type trayReply struct { |
| + Stored int `json:"stored"` |
| + Refused []string `json:"refused"` |
| + Message string `json:"message"` |
| + Kind string `json:"kind"` |
| +} |
| + |
| +// readReply decodes one judgement, and refuses to decode anything that is |
| +// not one: a test that quietly treated an HTML redirect as an empty reply |
| +// would pass against a JSON path that does not exist. |
| +func readReply(t *testing.T, rec *httptest.ResponseRecorder) trayReply { |
| + t.Helper() |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("the JSON path answered %d, want 200. The tray reads any non-200 as \"no "+ |
| + "answer arrived\" and tells the person that file's fate is unknown, so a "+ |
| + "judged submission must not come back as one:\n%s", rec.Code, rec.Body.String()) |
| + } |
| + if ct := rec.Header().Get("Content-Type"); !strings.HasPrefix(ct, "application/json") { |
| + t.Fatalf("the JSON path answered Content-Type %q; the tray parses the body and would "+ |
| + "fall back to \"no answer\"", ct) |
| + } |
| + var reply trayReply |
| + if err := json.Unmarshal(rec.Body.Bytes(), &reply); err != nil { |
| + t.Fatalf("the JSON path did not answer JSON (%v):\n%s", err, rec.Body.String()) |
| + } |
| + return reply |
| +} |
| + |
| +// The asynchronous path is the same judgement in a different shape. |
| +// |
| +// What could still drift, given one handler, is the WORDS. A file posted |
| +// both ways must come back with the identical sentence, or the tray and |
| +// the scriptless flash have begun to disagree about the same file and |
| +// nobody can tell which of them is right. |
| +// |
| +// The comparison is between two apps, not two posts at one app: the same |
| +// filename twice at one app is an update, not a second document, and the |
| +// second sentence would be composed against a different corpus. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) the `wantsJSON` branch deleted from createDocument, so the tray |
| +// gets the redirect → "the JSON path answered 303, want 200" |
| +// (b) `Message: msg` → `Message: c.storedPart(stored)` in |
| +// createDocument's JSON branch — the plausible "the tray shows |
| +// refusals in its rows, the sentence only needs the count" edit → |
| +// "the two paths say different things about scan.png", printing |
| +// both: the JSON one had dropped the names |
| +// (c) `Stored: stored` → `Stored: stored + len(refused)` → "the JSON |
| +// path says 1 stored but the document list holds 0" |
| +func TestTheJSONPathIsTheSameJudgementInADifferentShape(t *testing.T) { |
| + cases := []struct { |
| + name, body string |
| + stored int |
| + }{ |
| + {"revenue.txt", "Revenue was 100 across the first region we serve.", 1}, |
| + {"scan.png", "\x89PNG\x00\x00\x00 binary payload that is not text", 0}, |
| + {"blank.txt", " \n ", 0}, |
| + } |
| + // Vacuity: both outcomes have to be represented, or this proves the |
| + // agreement for successes only and misses the half that matters. |
| + good, bad := 0, 0 |
| + for _, c := range cases { |
| + if c.stored == 1 { |
| + good++ |
| + } else { |
| + bad++ |
| + } |
| + } |
| + if good == 0 || bad == 0 { |
| + t.Fatal("the fixtures cover only one outcome; the comparison below would prove half of it") |
| + } |
| + |
| + for _, c := range cases { |
| + t.Run(c.name, func(t *testing.T) { |
| + viaForm := newApp(t) |
| + viaTray := newApp(t) |
| + |
| + said := outcome(t, viaForm, submit(t, viaForm, "", "", file{c.name, c.body})) |
| + reply := readReply(t, submitJSON(t, viaTray, file{c.name, c.body})) |
| + |
| + if reply.Message != said { |
| + t.Errorf("the two paths say different things about %s.\n form: %q\n tray: %q\n"+ |
| + "One sentence for one judgement, or the tray is copy nobody reviewed.", |
| + c.name, said, reply.Message) |
| + } |
| + // The count is the number of documents that exist, checked |
| + // against the list rather than against itself. |
| + if n := storedCount(t, viaTray); reply.Stored != n { |
| + t.Errorf("the JSON path says %d stored but the document list holds %d", |
| + reply.Stored, n) |
| + } |
| + if reply.Stored != c.stored { |
| + t.Errorf("%s: the JSON path stored %d, want %d", c.name, reply.Stored, c.stored) |
| + } |
| + // A refusal is NAMED, in the list the tray puts beside the |
| + // row. A count with no name leaves a person guessing. |
| + if c.stored == 0 { |
| + if len(reply.Refused) != 1 { |
| + t.Fatalf("%s was not stored and the reply names %d refusals: %v", |
| + c.name, len(reply.Refused), reply.Refused) |
| + } |
| + if !strings.Contains(reply.Refused[0], c.name) { |
| + t.Errorf("the refusal does not name the file it refused: %q", reply.Refused[0]) |
| + } |
| + if reply.Kind != "danger" { |
| + t.Errorf("%s was refused but the reply is kind %q", c.name, reply.Kind) |
| + } |
| + } else if len(reply.Refused) != 0 { |
| + t.Errorf("%s was stored but the reply refuses %v", c.name, reply.Refused) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// The JSON path sets no flash cookie. |
| +// |
| +// The cookie is one-shot and it OUTLIVES the request that set it. The |
| +// tray has already said what happened, out loud, in the row the file |
| +// belongs to — so a cookie set here would ambush whatever page the person |
| +// opens next with a notice about a drop that finished ten minutes ago, |
| +// attached to nothing on the screen. And it is the sentence that carries |
| +// filenames. |
| +// |
| +// The scriptless half is asserted first and deliberately: without it this |
| +// test passes just as well against an app that has stopped setting the |
| +// cookie at all, which would silently delete the naming half of the |
| +// scriptless outcome. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `flash.Set` moved above the `wantsJSON` branch in createDocument |
| +// → "a judged submission on the JSON path set 1 cookie(s)" |
| +// (b) the same in refuse() → the same failure, second case |
| +// (c) `flash.Set` deleted from the HTML path → "the form path set no |
| +// cookie" — the vacuity guard doing its job |
| +func TestTheJSONPathSetsNoFlashCookie(t *testing.T) { |
| + h := newApp(t) |
| + if n := len(submit(t, h, "", "", file{"scan.png", "\x00\x00 not text"}).Result().Cookies()); n == 0 { |
| + t.Fatal("the form path set no cookie; this test would pass against an app that had " + |
| + "stopped naming refusals altogether, which is what the cookie exists for") |
| + } |
| + |
| + for _, c := range []struct { |
| + what string |
| + rec *httptest.ResponseRecorder |
| + }{ |
| + // A judged submission… |
| + {"a judged submission", submitJSON(t, newApp(t), file{"notes.txt", "Revenue was 100 across the region."})}, |
| + // …and a whole-submission refusal, which goes through refuse() |
| + // and is the half that gets forgotten. |
| + {"a whole-submission refusal", submitJSON(t, newApp(t), tooManyFiles(t, newApp(t))...)}, |
| + } { |
| + if n := len(c.rec.Result().Cookies()); n != 0 { |
| + t.Errorf("%s on the JSON path set %d cookie(s): %v. The tray has already said this "+ |
| + "on screen; a one-shot cookie would say it again over the next page the "+ |
| + "person opens.", c.what, n, c.rec.Result().Cookies()) |
| + } |
| + } |
| +} |
| + |
| +// tooManyFiles builds a drop one file over the cap the page advertises. |
| +func tooManyFiles(t *testing.T, h http.Handler) []file { |
| + t.Helper() |
| + n := atoi(t, dropZoneAttr(t, get(t, h, "/").Body.String(), "data-max-files")) |
| + if n < 2 || n > 200 { |
| + t.Fatalf("data-max-files is %d; the drop built here would prove nothing", n) |
| + } |
| + files := make([]file, 0, n+1) |
| + for i := 0; i <= n; i++ { |
| + files = append(files, file{fmt.Sprintf("over-%02d.txt", i), fmt.Sprintf("Passage %d about revenue.", i)}) |
| + } |
| + return files |
| +} |
| + |
| +// Every server-side check still applies on the JSON path. |
| +// |
| +// It applies because it is the same handler, and this test exists to |
| +// notice the day that stops being true — the obvious future shortcut is a |
| +// branch near the top of createDocument that answers the tray without |
| +// walking the rest. So each check is aimed at the JSON path directly: the |
| +// per-file cap, the NUL sniff, and both submission caps, which the tray |
| +// itself never triggers (it sends one file per request) and which are |
| +// therefore exactly the ones a shortcut would take with it. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) the shortcut itself — `if wantsJSON(r) { writeOutcome(w, |
| +// uploadOutcome{Stored: 1, Message: c.StoredOne, Kind: "good"}); |
| +// return }` inserted immediately after ParseMultipartForm → |
| +// THREE of the four subtests fail: "a file 1 byte over the cap was |
| +// answered stored=1", "a NUL inside the sniff window was answered |
| +// stored=1", and "the reply to an over-cap drop is stored=1 |
| +// kind=\"good\"". |
| +// |
| +// The total-size subtest stayed GREEN, and that is worth writing |
| +// down rather than quietly dropping: the body cap is |
| +// http.MaxBytesReader, which fires INSIDE ParseMultipartForm — |
| +// before the insertion point — so that drop was already refused |
| +// by the time the shortcut ran. It is honest protection, not a |
| +// hole, but this test does not prove it: a shortcut placed one |
| +// line higher would take that check too and only three subtests |
| +// would notice. Move the insertion above the MaxBytesReader and |
| +// all four fail. |
| +// (b) `case !looksLikeText(raw)` deleted from readUpload → "a NUL |
| +// inside the sniff window was answered stored=1" and "1 documents |
| +// exist after a binary upload on the JSON path" |
| +func TestTheJSONPathStillEnforcesEveryServerCheck(t *testing.T) { |
| + page := get(t, newApp(t), "/").Body.String() |
| + capBytes := atoi(t, dropZoneAttr(t, page, "data-max-bytes")) |
| + window := atoi(t, dropZoneAttr(t, page, "data-sniff-bytes")) |
| + if capBytes < 1024 || window < 16 { |
| + t.Fatalf("the advertised cap (%d) or sniff window (%d) is too small for these fixtures "+ |
| + "to prove anything", capBytes, window) |
| + } |
| + filler := "The quarterly figure was recorded in full and then again.\n\n" |
| + over := strings.Repeat(filler, capBytes/len(filler)+2)[:capBytes+1] |
| + |
| + t.Run("the per-file cap", func(t *testing.T) { |
| + h := newApp(t) |
| + reply := readReply(t, submitJSON(t, h, file{"over.txt", over})) |
| + if reply.Stored != 0 { |
| + t.Errorf("a file 1 byte over the cap was answered stored=%d", reply.Stored) |
| + } |
| + if n := storedCount(t, h); n != 0 { |
| + t.Errorf("%d documents exist after an over-cap upload on the JSON path", n) |
| + } |
| + if !strings.Contains(reply.Message, "over.txt") { |
| + t.Errorf("the refusal does not name the file: %q", reply.Message) |
| + } |
| + }) |
| + |
| + t.Run("the NUL sniff", func(t *testing.T) { |
| + h := newApp(t) |
| + body := strings.Repeat("a", window-1) + "\x00" + strings.Repeat("b", 64) |
| + reply := readReply(t, submitJSON(t, h, file{"inside.txt", body})) |
| + if reply.Stored != 0 { |
| + t.Errorf("a NUL inside the sniff window was answered stored=%d", reply.Stored) |
| + } |
| + if n := storedCount(t, h); n != 0 { |
| + t.Errorf("%d documents exist after a binary upload on the JSON path", n) |
| + } |
| + }) |
| + |
| + t.Run("the file-count cap", func(t *testing.T) { |
| + h := newApp(t) |
| + files := tooManyFiles(t, h) |
| + reply := readReply(t, submitJSON(t, h, files...)) |
| + if n := storedCount(t, h); n != 0 { |
| + t.Errorf("%d files, one over the cap, stored %d documents on the JSON path. A "+ |
| + "submission cap refuses the drop whole or it is not a submission cap.", |
| + len(files), n) |
| + } |
| + if reply.Stored != 0 || reply.Kind != "danger" { |
| + t.Errorf("the reply to an over-cap drop is stored=%d kind=%q", reply.Stored, reply.Kind) |
| + } |
| + if !strings.Contains(reply.Message, strconv.Itoa(len(files))) { |
| + t.Errorf("the refusal does not say how many were dropped (%d): %q", len(files), reply.Message) |
| + } |
| + }) |
| + |
| + t.Run("the total-size cap", func(t *testing.T) { |
| + h := newApp(t) |
| + total := atoi(t, dropZoneAttr(t, get(t, h, "/").Body.String(), "data-max-total-bytes")) |
| + if total <= capBytes { |
| + t.Fatalf("the advertised total (%d) is not larger than the per-file cap (%d)", total, capBytes) |
| + } |
| + one := strings.Repeat(filler, capBytes/len(filler)+1)[:capBytes] |
| + files := make([]file, 0, total/capBytes+1) |
| + for i := 0; len(files)*capBytes <= total; i++ { |
| + files = append(files, file{fmt.Sprintf("big-%02d.txt", i), one}) |
| + } |
| + reply := readReply(t, submitJSON(t, h, files...)) |
| + if n := storedCount(t, h); n != 0 { |
| + t.Errorf("a drop of %d bytes, over the advertised %d, stored %d documents on the "+ |
| + "JSON path", len(files)*capBytes, total, n) |
| + } |
| + if reply.Stored != 0 { |
| + t.Errorf("the reply to an over-total drop is stored=%d", reply.Stored) |
| + } |
| + }) |
| +} |
| + |
| +// Every word the tray writes into the page comes from Go. |
| +// |
| +// The tray is the second place this app says what happened to a file, and |
| +// the first — the scriptless flash — is composed in handlers.go. A |
| +// sentence typed into drop.js would be copy nobody reviewed, in a file no |
| +// copy pass reads, saying something subtly different from what the same |
| +// person would have seen with scripts off. |
| +// |
| +// So: every string the script writes is published on the drop zone as a |
| +// data attribute, the script reads every one of them by key, and NONE of |
| +// their values appears in the script itself. The third check is the one |
| +// with teeth — the first two would both pass against a script that read |
| +// the attributes and then ignored them. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `row.phase.textContent = msg.storing` → a literal in drop.js → |
| +// "the drop script spells out the copy "Storing…", which is |
| +// published as data-msg-storing" |
| +// (b) "storing" removed from drop.js's key list, leaving the attribute |
| +// rendered → "the page publishes data-msg-storing and the drop |
| +// script never reads it; a sentence nothing reads is a sentence |
| +// that has already drifted" |
| +// (c) `ReasonBinary: "that file is not text"` — the name taken out of |
| +// a clause → "data-reason-binary does not contain {name}" |
| +// |
| +// It caught one for real while being written, which is worth recording: |
| +// a COMMENT added to drop.js used the word the page publishes as |
| +// data-msg-row-stored, and the test went red on the untouched build. |
| +// That is the direction of error this scan is meant to have — it reads |
| +// the file whole, comments included — and the comment was reworded. A |
| +// scan that stripped comments first would risk skipping a real string. |
| +func TestEveryWordTheTrayWritesComesFromGo(t *testing.T) { |
| + h := newApp(t) |
| + tag := dropZoneTag(t, get(t, h, "/").Body.String()) |
| + src := servedDropScript(t, h) |
| + |
| + pairs := regexp.MustCompile(`\bdata-(msg|reason)-([a-z-]+)="([^"]*)"`).FindAllStringSubmatch(tag, -1) |
| + // Vacuity, and it has to be here: a pattern that stopped matching |
| + // would leave this loop empty and the test green over a script full |
| + // of hand-typed sentences. |
| + if len(pairs) < 12 { |
| + t.Fatalf("only %d published messages were found on the drop zone; the tray alone needs "+ |
| + "more than that, so this pattern has stopped matching and the test proves "+ |
| + "nothing:\n%s", len(pairs), tag) |
| + } |
| + |
| + for _, p := range pairs { |
| + attr, key, value := "data-"+p[1]+"-"+p[2], p[2], unescape(p[3]) |
| + |
| + // The script reads it by key: it builds the attribute name from |
| + // the prefix and the key, so the key is what appears in there. |
| + if !strings.Contains(src, `"`+key+`"`) { |
| + t.Errorf("the page publishes %s and the drop script never reads it; a sentence "+ |
| + "nothing reads is a sentence that has already drifted from the one the "+ |
| + "scriptless page shows", attr) |
| + } |
| + // And the script does not carry the words themselves. |
| + if value != "" && strings.Contains(src, value) { |
| + t.Errorf("the drop script spells out the copy %q, which is published as %s. Two "+ |
| + "copies of one sentence means one of them is out of date and nobody can "+ |
| + "tell which — and only one of them was ever reviewed.", value, attr) |
| + } |
| + |
| + // Every REASON clause names its own file, and the tray's layout |
| + // now depends on it. A refused row drops its separate name label |
| + // and lets the clause be the row, because rendering both put |
| + // |
| + // scan.png |
| + // scan.png is not text |
| + // |
| + // on the screen — the same shape as the search result that |
| + // repeated its own heading, found by looking at the page and |
| + // invisible to every test here. A clause that stopped naming its |
| + // file would leave that row anonymous: a refusal with nothing to |
| + // attach it to, which is the silent failure wearing a red tone. |
| + if p[1] == "reason" && !strings.Contains(value, "{name}") { |
| + t.Errorf("%s does not contain {name}: %q. Every reason clause names the file it is "+ |
| + "about — the tray removes a refused row's name label because the clause "+ |
| + "carries it, and the outcome sentence lists these clauses for a whole drop "+ |
| + "where nothing else says which file each one belongs to.", attr, value) |
| + } |
| + } |
| +} |
| + |
| +// The tray is a live region, and the lesson the note taught applies to it |
| +// unchanged: a region unhidden in the same tick as its first message is |
| +// commonly not announced at all. So the tray enters the accessibility |
| +// tree at the ready step, empty, and stays there; the panel paints |
| +// nothing until a row is added. |
| +// |
| +// The other half is what this design could get wrong in the opposite |
| +// direction. Five files uploading is five rows whose text changes several |
| +// times a second, and a live region over THAT is a screen reader nobody |
| +// can use. So exactly one element in the tray is live — the status line, |
| +// written once, when everything has settled — and the row list must not |
| +// be. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) `tray.hidden = false` moved into showTray() → "drop.js unhides |
| +// the tray before the ready step" |
| +// (b) deleting it → "drop.js never unhides the tray" |
| +// (c) `aria-live="polite"` added to <ul rst-tray-list> in index.html → |
| +// "the row list is a live region" |
| +// (d) `role="status" aria-live="polite"` deleted from the status |
| +// paragraph → "the tray status is not a polite live region" |
| +// (e) the [rst-tray-status]:empty rule deleted from memoria.css → |
| +// "memoria.css has no :empty rule for the tray status" |
| +func TestTheTrayIsALiveRegionThatExistsBeforeItSpeaks(t *testing.T) { |
| + h := newApp(t) |
| + src := servedDropScript(t, h) |
| + |
| + const anchor = "and only now does the zone exist" |
| + at := strings.Index(src, anchor) |
| + if at < 0 { |
| + t.Fatalf("drop.js no longer carries the %q marker, so this test cannot tell the ready "+ |
| + "step from the rest of the file and proves nothing", anchor) |
| + } |
| + unhide := regexp.MustCompile(`tray\.hidden\s*=`).FindAllStringIndex(src, -1) |
| + if len(unhide) == 0 { |
| + t.Fatal("drop.js never unhides the tray; every upload then happens invisibly and every " + |
| + "refusal is silent") |
| + } |
| + if len(unhide) > 1 { |
| + t.Errorf("drop.js assigns tray.hidden in %d places. One is the ready step; any other "+ |
| + "can only be unhiding it later, beside its first message — the pattern that "+ |
| + "does not announce.", len(unhide)) |
| + } |
| + for _, u := range unhide { |
| + if u[0] < at { |
| + t.Errorf("drop.js unhides the tray before the ready step (byte %d, marker at %d). "+ |
| + "The live region must enter the accessibility tree empty and early.", u[0], at) |
| + } |
| + } |
| + |
| + body := get(t, h, "/").Body.String() |
| + stat := regexp.MustCompile(`<p rst-tray-status[^>]*>`).FindString(body) |
| + if stat == "" { |
| + t.Fatalf("no tray status element on the page:\n%s", body) |
| + } |
| + if !strings.Contains(stat, `aria-live="polite"`) || !strings.Contains(stat, `role="status"`) { |
| + t.Errorf("the tray status is not a polite live region, so unhiding it early buys "+ |
| + "nothing and a refused file is never announced: %s", stat) |
| + } |
| + rows := regexp.MustCompile(`<ul rst-tray-list[^>]*>`).FindString(body) |
| + if rows == "" { |
| + t.Fatalf("no tray row list on the page:\n%s", body) |
| + } |
| + for _, loud := range []string{"aria-live", `role="status"`, `role="alert"`, "aria-atomic"} { |
| + if strings.Contains(rows, loud) { |
| + t.Errorf("the row list carries %s: %s\nA five-file drop would announce every "+ |
| + "percentage of every row several times a second, which is a screen reader "+ |
| + "nobody can use. Only the status line announces, and only once.", loud, rows) |
| + } |
| + } |
| + |
| + css, err := os.ReadFile(filepath.Join("..", "memoria", "static", "memoria.css")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !regexp.MustCompile(`\[rst-tray-status\]:empty\s*\{[^}]*margin\s*:\s*0`).Match(css) { |
| + t.Error("memoria.css has no :empty rule for the tray status zeroing its margin. It is " + |
| + "unhidden from the moment the script runs, so an empty paragraph leaves a stray " + |
| + "half-line pinned in the corner of every page — and the obvious fix for that is " + |
| + "to hide the region again, which is the bug.") |
| + } |
| +} |
| + |
| +// The tray is not part of the form, and a scriptless page does not have |
| +// one. |
| +// |
| +// Both halves matter. Inside the form it would be markup the scriptless |
| +// submit carries around for nothing, and a <button> inside a form is a |
| +// SUBMIT button unless something says otherwise — a Dismiss control that |
| +// posted the upload form is the kind of bug that ships. Unhidden it would |
| +// pin an empty panel to the corner of a page that cannot upload anything |
| +// in the background at all. |
| +// |
| +// MUTATION-TESTED 2026-09-04, each reverted: |
| +// |
| +// (a) the tray moved inside the upload form in index.html → "the tray |
| +// is inside the upload form" |
| +// (b) `hidden` deleted from the tray → "the tray is visible before any |
| +// script has run" |
| +// (c) `type="button"` deleted from the dismiss control → "the tray's |
| +// dismiss control has no type=button, so it is a submit button" |
| +// (d) writing rst-tray="open" in the template → "the tray is |
| +// pre-marked open in the template" |
| +func TestTheTrayIsOutsideTheFormAndAbsentWithoutScript(t *testing.T) { |
| + body := get(t, newApp(t), "/").Body.String() |
| + |
| + i := strings.Index(body, `action="/documents"`) |
| + if i < 0 { |
| + t.Fatalf("no upload form on the page:\n%s", body) |
| + } |
| + j := strings.Index(body[i:], "</form>") |
| + if j < 0 { |
| + t.Fatal("the upload form never closes") |
| + } |
| + form := body[i : i+j] |
| + |
| + tray := strings.Index(body, "<div rst-tray") |
| + if tray < 0 { |
| + t.Fatalf("no tray on the page:\n%s", body) |
| + } |
| + if strings.Contains(form, "<div rst-tray") { |
| + t.Error("the tray is inside the upload form. It is not something the form submits, " + |
| + "and a control inside a form is one attribute away from submitting it.") |
| + } |
| + |
| + close := strings.Index(body[tray:], ">") |
| + if close < 0 { |
| + t.Fatal("the tray tag never closes") |
| + } |
| + tag := body[tray : tray+close+1] |
| + if !regexp.MustCompile(`(^|\s)hidden(\s|>|=)`).MatchString(tag) { |
| + t.Errorf("the tray is visible before any script has run, so a page that cannot upload "+ |
| + "anything in the background still shows the panel that reports it: %s", tag) |
| + } |
| + if strings.Contains(tag, `rst-tray="`) { |
| + t.Errorf("the tray is pre-marked open in the template: %s\nThe panel paints on "+ |
| + "rst-tray=\"open\", which only the script may write — otherwise a page whose "+ |
| + "script failed pins an empty box to the corner.", tag) |
| + } |
| + |
| + dismiss := regexp.MustCompile(`<button[^>]*rst-tray-dismiss[^>]*>`).FindString(body) |
| + if dismiss == "" { |
| + t.Fatalf("the tray has no dismiss control; it would stay on screen forever:\n%s", body[tray:]) |
| + } |
| + if !strings.Contains(dismiss, `type="button"`) { |
| + t.Errorf("the tray's dismiss control has no type=button, so it is a submit button: %s", |
| + dismiss) |
| + } |
| +} |