fix: series removal sent its status line twice
adminSeriesRemove answered the list surface with two h.render calls -- the row fragment and the out-of-band heading -- and h.render writes a status line each time, so every removal logged "superfluous response.WriteHeader call". The heading is an append to a response already committed, so it now executes straight onto w, the way writeChromeOOB already does it. The regression test runs the router under a real server with a captured ErrorLog: a ResponseRecorder never sees this warning, which is why the existing removal test did not catch it.
This commit is contained in:
@@ -505,10 +505,13 @@ func (h *Handler) adminSeriesRemove(w http.ResponseWriter, r *http.Request) {
|
|||||||
band = 1
|
band = 1
|
||||||
}
|
}
|
||||||
h.render(w, http.StatusOK, "series-row", seriesRow(a, band, time.Now()))
|
h.render(w, http.StatusOK, "series-row", seriesRow(a, band, time.Now()))
|
||||||
|
// The heading is an out-of-band append to a response whose status line has
|
||||||
|
// already gone out with the row, so it is executed straight onto w —
|
||||||
|
// h.render would send a second WriteHeader.
|
||||||
if head, err := h.seriesListHeadView(r); err != nil {
|
if head, err := h.seriesListHeadView(r); err != nil {
|
||||||
log.Printf("series remove %s: %v", site+":"+seriesID, err)
|
log.Printf("series remove %s: %v", site+":"+seriesID, err)
|
||||||
} else {
|
} else if err := h.tmpl.ExecuteTemplate(w, "series-list-head", head); err != nil {
|
||||||
h.render(w, http.StatusOK, "series-list-head", head)
|
log.Printf("series remove %s: render series-list-head oob: %v", site+":"+seriesID, err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,11 +1,13 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"crypto/sha256"
|
"crypto/sha256"
|
||||||
"database/sql"
|
"database/sql"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"log"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"net/url"
|
"net/url"
|
||||||
@@ -4285,6 +4287,52 @@ func TestRemoveFromListAnswersRowAndFreshHeading(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The removal answers two fragments on one response. The second is an
|
||||||
|
// out-of-band append, so it must not send a second status line: net/http
|
||||||
|
// answers a double WriteHeader with "superfluous response.WriteHeader call"
|
||||||
|
// on the server's error log, which a ResponseRecorder never sees. Hence a
|
||||||
|
// real server here.
|
||||||
|
func TestRemoveFromListSendsOneStatusLine(t *testing.T) {
|
||||||
|
st, dsn := newTestStoreURL(t)
|
||||||
|
db, err := sql.Open("pgx", dsn)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("open %s: %v", dsn, err)
|
||||||
|
}
|
||||||
|
defer db.Close()
|
||||||
|
seedSeriesRow(t, st, db, seriesRowSeed{key: "asura:a", url: "u", checkedAt: 9000, bookmarks: 0})
|
||||||
|
|
||||||
|
var errLog bytes.Buffer
|
||||||
|
srv := httptest.NewUnstartedServer(newRouter(st, testConfig()))
|
||||||
|
srv.Config.ErrorLog = log.New(&errLog, "", 0)
|
||||||
|
srv.Start()
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
req, err := http.NewRequest(http.MethodPost, srv.URL+"/admin/series/asura:a/remove",
|
||||||
|
strings.NewReader("filter=no_readers&band=0"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("build request: %v", err)
|
||||||
|
}
|
||||||
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||||
|
req.AddCookie(sessionCookie(t, st))
|
||||||
|
resp, err := srv.Client().Do(req)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("removal request: %v", err)
|
||||||
|
}
|
||||||
|
body, _ := io.ReadAll(resp.Body)
|
||||||
|
resp.Body.Close()
|
||||||
|
if resp.StatusCode != http.StatusOK {
|
||||||
|
t.Fatalf("removal status = %d, want 200", resp.StatusCode)
|
||||||
|
}
|
||||||
|
// Both fragments still travel: the fix must not have dropped the heading.
|
||||||
|
if !strings.Contains(string(body), "Title of asura:a") ||
|
||||||
|
!strings.Contains(string(body), `hx-swap-oob="true"`) {
|
||||||
|
t.Errorf("answer lost a fragment:\n%s", body)
|
||||||
|
}
|
||||||
|
if strings.Contains(errLog.String(), "superfluous") {
|
||||||
|
t.Errorf("removal wrote the status line twice: %s", errLog.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// A removal from the detail page navigates to the No-Readers list: htmx gets
|
// A removal from the detail page navigates to the No-Readers list: htmx gets
|
||||||
// a full navigation (HX-Redirect — a 303 would be followed by the request
|
// a full navigation (HX-Redirect — a 303 would be followed by the request
|
||||||
// and the list page swapped into the press's target), plain clients the 303
|
// and the list page swapped into the press's target), plain clients the 303
|
||||||
|
|||||||
Reference in New Issue
Block a user