From 3632de504b6c01905e85a6724c96a0df3dc21dd3 Mon Sep 17 00:00:00 2001 From: Lea Anthony Date: Mon, 10 Aug 2026 00:05:01 +1000 Subject: [PATCH 1/3] fix(v3): make asset server flush semantics honest Three related problems in the asset server, all found while investigating Go->JS event dispatch and all independent of it. contentTypeSniffer.Flush delegated to the wrapped writer without first completing itself. Until 512 bytes have been seen the sniffer deliberately holds the body back so it can detect a Content-Type, so a flush before that point sent nothing and reported success. Any response that writes a short chunk and then waits - every streaming format - silently delivered nothing. Flush now completes first, sniffing from the short prefix, which is the right reading of an explicit flush: send what has been written so far. The platform response writers did not implement http.Flusher at all, so http.ResponseController(w).Flush() could never reach them. On macOS, iOS and Linux that was only a missing declaration: Write already pushes each chunk straight to the URL scheme task or into the pipe WebKitGTK reads, so a flush has no work to do. Those now declare a documented no-op. Windows deliberately does not, and this is the interesting one. Its Write accumulates into an in-memory bytes.Buffer that is handed to WebView2 only in Finish, so nothing can reach the page mid-response. A no-op Flush there would claim a capability that does not exist. Left unimplemented, ResponseController.Flush keeps returning ErrNotSupported, which is true. It also means streaming cannot work on Windows today regardless of what WebView2 supports - worth knowing before designing around it. dispatchWorkers is declared and read but never assigned, which reads like an oversight and is not: it must stay 0. At 0 each request gets its own goroutine. Any positive value switches to a fixed worker pool, where a long-lived request holds its worker for its whole lifetime and enough of them starve the pool - later requests, including the page's own assets, queue behind them and the app appears to hang at startup with no error anywhere. Documented rather than removed, since the pooled path is still useful for workloads known to be short-lived. Tests cover the sniffer: a short prefix is released on flush, later writes pass through, flushing before any write is a no-op, an explicit Content-Type is untouched, the full-prefix path still completes on its own, and http.ResponseController finds the flusher. Three of them fail without the fix. --- .../assetserver/assetserver_webview.go | 18 ++- .../assetserver/content_type_sniffer.go | 17 ++- .../assetserver/content_type_sniffer_test.go | 123 ++++++++++++++++++ .../webview/responsewriter_darwin.go | 8 ++ .../assetserver/webview/responsewriter_ios.go | 6 + .../webview/responsewriter_linux.go | 7 + .../webview/responsewriter_linux_gtk3.go | 6 + .../webview/responsewriter_windows.go | 11 ++ 8 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 v3/internal/assetserver/content_type_sniffer_test.go diff --git a/v3/internal/assetserver/assetserver_webview.go b/v3/internal/assetserver/assetserver_webview.go index 21c01ede268..99227fb00fd 100644 --- a/v3/internal/assetserver/assetserver_webview.go +++ b/v3/internal/assetserver/assetserver_webview.go @@ -16,8 +16,22 @@ type assetServerWebView struct { // ExpectedWebViewHost is checked against the Request Host of every WebViewRequest, other hosts won't be processed. ExpectedWebViewHost string - dispatchInit sync.Once - dispatchReqC chan<- webview.Request + dispatchInit sync.Once + dispatchReqC chan<- webview.Request + + // dispatchWorkers must stay 0, which is why nothing assigns it. + // + // At 0 every request gets its own goroutine. Any positive value switches + // ServeWebViewRequest to a fixed worker pool, and a request that is + // long-lived by design - a streaming response, or anything that blocks + // waiting on the frontend - occupies its worker for its whole lifetime. + // Enough of those and the pool is starved: later requests, including the + // page's own assets, queue behind them and the app appears to hang during + // startup with no error anywhere. + // + // The field is kept because the pooled path is still useful for a + // request-heavy workload known to be short-lived, but it needs a bound on + // request lifetime before it can be turned on. dispatchWorkers int } diff --git a/v3/internal/assetserver/content_type_sniffer.go b/v3/internal/assetserver/content_type_sniffer.go index fd51a6101a6..c627bafc09d 100644 --- a/v3/internal/assetserver/content_type_sniffer.go +++ b/v3/internal/assetserver/content_type_sniffer.go @@ -135,8 +135,23 @@ func (rw *contentTypeSniffer) closeClient() { } // Flush implements the http.Flusher interface. +// +// The prefix has to be resolved first. Until 512 bytes have been seen the +// sniffer is deliberately holding the body back so it can detect a +// Content-Type, so delegating straight to the wrapped writer would flush +// nothing at all — the caller asks for a flush, gets no error, and no bytes +// reach the client. That silently breaks any response that emits less than +// 512 bytes and then waits, which is every streaming format. +// +// Completing here means the Content-Type is sniffed from a short prefix +// instead of a full one. That is the right trade: an explicit flush is the +// caller stating that what has been written so far should be sent now. func (rw *contentTypeSniffer) Flush() { + // Errors are dropped deliberately: http.Flusher cannot report one, and the + // same write error will surface from the next Write or from complete. + _, _ = rw.complete() + if f, ok := rw.rw.(http.Flusher); ok { f.Flush() } -} \ No newline at end of file +} diff --git a/v3/internal/assetserver/content_type_sniffer_test.go b/v3/internal/assetserver/content_type_sniffer_test.go new file mode 100644 index 00000000000..7ac7ee7e187 --- /dev/null +++ b/v3/internal/assetserver/content_type_sniffer_test.go @@ -0,0 +1,123 @@ +package assetserver + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// A response shorter than the 512-byte sniffing prefix is held back until the +// handler returns. Flush has to release it, or a streaming handler that writes +// a short chunk and then waits delivers nothing while reporting success. +func TestContentTypeSnifferFlushReleasesShortPrefix(t *testing.T) { + rec := httptest.NewRecorder() + rw := newContentTypeSniffer(rec) + + rw.WriteHeader(http.StatusOK) // no Content-Type set, so sniffing applies + if _, err := io.WriteString(rw, "data: first\n\n"); err != nil { + t.Fatalf("write: %v", err) + } + + if got := rec.Body.Len(); got != 0 { + t.Fatalf("prefix should still be buffered before Flush, got %d bytes", got) + } + + rw.Flush() + + if got := rec.Body.String(); got != "data: first\n\n" { + t.Errorf("Flush did not release the buffered prefix, body = %q", got) + } + if !rec.Flushed { + t.Error("Flush did not reach the wrapped writer") + } + if ct := rec.Header().Get(HeaderContentType); ct == "" { + t.Error("Content-Type was not resolved on flush") + } +} + +// After a flush the sniffer is done buffering, so later writes must pass +// straight through rather than being collected into a new prefix. +func TestContentTypeSnifferWritesPassThroughAfterFlush(t *testing.T) { + rec := httptest.NewRecorder() + rw := newContentTypeSniffer(rec) + + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, "one") + rw.Flush() + + if _, err := io.WriteString(rw, "two"); err != nil { + t.Fatalf("write after flush: %v", err) + } + if got := rec.Body.String(); got != "onetwo" { + t.Errorf("body = %q, want %q", got, "onetwo") + } +} + +// Flushing before anything is written must not panic or emit a header. +func TestContentTypeSnifferFlushBeforeWrite(t *testing.T) { + rec := httptest.NewRecorder() + rw := newContentTypeSniffer(rec) + + rw.Flush() + + if rec.Body.Len() != 0 { + t.Errorf("expected no body, got %q", rec.Body.String()) + } +} + +// An explicit Content-Type disables sniffing entirely; Flush must not disturb +// the header or duplicate the body. +func TestContentTypeSnifferFlushWithExplicitContentType(t *testing.T) { + rec := httptest.NewRecorder() + rw := newContentTypeSniffer(rec) + + rw.Header().Set(HeaderContentType, "text/event-stream") + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, "data: x\n\n") + rw.Flush() + + if got := rec.Header().Get(HeaderContentType); got != "text/event-stream" { + t.Errorf("Content-Type = %q, want text/event-stream", got) + } + if got := rec.Body.String(); got != "data: x\n\n" { + t.Errorf("body = %q", got) + } +} + +// The existing behaviour for a full prefix must be unchanged: once 512 bytes +// have accumulated the sniffer completes on its own. +func TestContentTypeSnifferFullPrefixStillCompletesWithoutFlush(t *testing.T) { + rec := httptest.NewRecorder() + rw := newContentTypeSniffer(rec) + + body := strings.Repeat("a", 600) + rw.WriteHeader(http.StatusOK) + if _, err := io.WriteString(rw, body); err != nil { + t.Fatalf("write: %v", err) + } + + if got := rec.Body.String(); got != body { + t.Errorf("body length = %d, want %d", len(got), len(body)) + } + if ct := rec.Header().Get(HeaderContentType); ct == "" { + t.Error("Content-Type was not sniffed once the prefix filled") + } +} + +// http.ResponseController must find the sniffer's Flush and report success. +func TestContentTypeSnifferSupportsResponseController(t *testing.T) { + rec := httptest.NewRecorder() + rw := newContentTypeSniffer(rec) + + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, "chunk") + + if err := http.NewResponseController(rw).Flush(); err != nil { + t.Fatalf("ResponseController.Flush: %v", err) + } + if got := rec.Body.String(); got != "chunk" { + t.Errorf("body = %q, want %q", got, "chunk") + } +} diff --git a/v3/internal/assetserver/webview/responsewriter_darwin.go b/v3/internal/assetserver/webview/responsewriter_darwin.go index ff29a08bbb9..72eb40a1a4e 100644 --- a/v3/internal/assetserver/webview/responsewriter_darwin.go +++ b/v3/internal/assetserver/webview/responsewriter_darwin.go @@ -113,6 +113,14 @@ func (rw *responseWriter) Write(buf []byte) (int, error) { return contentLen, nil } +// Flush implements the http.Flusher interface. +// +// Write hands each chunk straight to the URL scheme task via +// didReceiveData, so there is nothing buffered on this side and a flush has +// no work to do. Declaring it anyway is what lets http.ResponseController +// report success truthfully to a streaming handler. +func (rw *responseWriter) Flush() {} + func (rw *responseWriter) WriteHeader(code int) { rw.code = code if rw.wroteHeader || rw.finished { diff --git a/v3/internal/assetserver/webview/responsewriter_ios.go b/v3/internal/assetserver/webview/responsewriter_ios.go index 7df697b6ebf..b2906ae1b82 100644 --- a/v3/internal/assetserver/webview/responsewriter_ios.go +++ b/v3/internal/assetserver/webview/responsewriter_ios.go @@ -118,6 +118,12 @@ func (rw *responseWriter) Write(buf []byte) (int, error) { return contentLen, nil } +// Flush implements the http.Flusher interface. +// +// As on macOS, Write pushes each chunk to the URL scheme task immediately, +// so there is nothing to flush. +func (rw *responseWriter) Flush() {} + func (rw *responseWriter) WriteHeader(code int) { rw.code = code if rw.wroteHeader || rw.finished { diff --git a/v3/internal/assetserver/webview/responsewriter_linux.go b/v3/internal/assetserver/webview/responsewriter_linux.go index 488bde07656..5dce0df7510 100644 --- a/v3/internal/assetserver/webview/responsewriter_linux.go +++ b/v3/internal/assetserver/webview/responsewriter_linux.go @@ -64,6 +64,13 @@ func (rw *responseWriter) Write(buf []byte) (int, error) { return rw.w.Write(buf) } +// Flush implements the http.Flusher interface. +// +// Write goes into the pipe whose read end WebKitGTK consumes as a +// GUnixInputStream, so bytes are already on their way out and there is +// nothing held back here. +func (rw *responseWriter) Flush() {} + func (rw *responseWriter) WriteHeader(code int) { rw.code = code if rw.wroteHeader || rw.finished { diff --git a/v3/internal/assetserver/webview/responsewriter_linux_gtk3.go b/v3/internal/assetserver/webview/responsewriter_linux_gtk3.go index aa4c4912d02..3af2c18b6a1 100644 --- a/v3/internal/assetserver/webview/responsewriter_linux_gtk3.go +++ b/v3/internal/assetserver/webview/responsewriter_linux_gtk3.go @@ -64,6 +64,12 @@ func (rw *responseWriter) Write(buf []byte) (int, error) { return rw.w.Write(buf) } +// Flush implements the http.Flusher interface. +// +// Write goes into the pipe WebKitGTK reads from, so nothing is buffered on +// this side. +func (rw *responseWriter) Flush() {} + func (rw *responseWriter) WriteHeader(code int) { rw.code = code if rw.wroteHeader || rw.finished { diff --git a/v3/internal/assetserver/webview/responsewriter_windows.go b/v3/internal/assetserver/webview/responsewriter_windows.go index c003f00bd83..63a07d46fd5 100644 --- a/v3/internal/assetserver/webview/responsewriter_windows.go +++ b/v3/internal/assetserver/webview/responsewriter_windows.go @@ -30,6 +30,17 @@ func (rw *responseWriter) Header() http.Header { return rw.header } +// Note: this writer deliberately does not implement http.Flusher. +// +// Unlike the other platforms, Write accumulates into an in-memory buffer that +// is only handed to WebView2 in Finish, so there is no way to push bytes to +// the client mid-response. A no-op Flush would therefore claim a capability +// that does not exist and make http.ResponseController report success while +// nothing reaches the page. Leaving it unimplemented keeps +// ResponseController.Flush returning ErrNotSupported, which is the truth. +// +// This is also why a streaming response cannot work on Windows today, +// independently of anything WebView2 does. func (rw *responseWriter) Write(buf []byte) (int, error) { if rw.finished { return 0, errResponseFinished From 73e3f186626ac37c5ff077b0b0ec0ad96ed2d964 Mon Sep 17 00:00:00 2001 From: Lea Anthony Date: Mon, 10 Aug 2026 00:11:12 +1000 Subject: [PATCH 2/3] fix(v3): do not lose the sniffing prefix when a flush write fails Review found a real hole in the previous commit. complete cleared the whole prefix regardless of what the wrapped Write actually accepted, and Flush discarded the error because http.Flusher cannot report one. A writer that failed the flush-triggered write therefore lost the buffered bytes and the failure with them: the request-end complete saw an empty prefix and reported success. complete now advances the prefix only by the number of bytes the writer accepted, so a short or failed write leaves the remainder for a later attempt, and records the error on the sniffer. Write and complete both return that error afterwards, which is how the failure reaches the caller now that Flush cannot. Two tests added, both failing without the change: one fails only the flush-triggered prefix write and asserts the error surfaces from the next Write and from complete, the other uses a short writer and asserts only the unsent remainder is kept. Existing tests also stopped ignoring errors from io.WriteString, so a writer that starts failing can no longer let a test pass while skipping its assertions. --- .../assetserver/content_type_sniffer.go | 30 ++++++- .../assetserver/content_type_sniffer_test.go | 89 ++++++++++++++++++- 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/v3/internal/assetserver/content_type_sniffer.go b/v3/internal/assetserver/content_type_sniffer.go index c627bafc09d..17e12508b50 100644 --- a/v3/internal/assetserver/content_type_sniffer.go +++ b/v3/internal/assetserver/content_type_sniffer.go @@ -19,6 +19,12 @@ type contentTypeSniffer struct { status int headerCommitted bool headerWritten bool + + // err is sticky. complete may fail partway through emitting the sniffing + // prefix, and http.Flusher has no way to report that, so the failure is + // recorded here and surfaced from the next Write or complete instead of + // being lost. + err error } // Unwrap returns the wrapped [http.ResponseWriter] for use with [http.ResponseController]. @@ -31,6 +37,10 @@ func (rw *contentTypeSniffer) Header() http.Header { } func (rw *contentTypeSniffer) Write(chunk []byte) (int, error) { + if rw.err != nil { + return 0, rw.err + } + if !rw.headerCommitted { rw.WriteHeader(http.StatusOK) } @@ -108,11 +118,29 @@ func (rw *contentTypeSniffer) sniff() { // Whoever creates a contentTypeSniffer instance // is responsible for calling complete after the nested handler has returned. func (rw *contentTypeSniffer) complete() (n int, err error) { + if rw.err != nil { + return 0, rw.err + } + rw.sniff() if rw.headerWritten && len(rw.prefix) > 0 { n, err = rw.rw.Write(rw.prefix) - rw.prefix = nil + + // Drop only what actually went out. Clearing the whole prefix on a + // short or failed write would discard bytes that were never sent, and + // the caller would have no way to retry them. + if n < 0 { + n = 0 + } + if n > len(rw.prefix) { + n = len(rw.prefix) + } + rw.prefix = rw.prefix[n:] + + if err != nil { + rw.err = err + } } return diff --git a/v3/internal/assetserver/content_type_sniffer_test.go b/v3/internal/assetserver/content_type_sniffer_test.go index 7ac7ee7e187..ce383d7ca7f 100644 --- a/v3/internal/assetserver/content_type_sniffer_test.go +++ b/v3/internal/assetserver/content_type_sniffer_test.go @@ -1,6 +1,7 @@ package assetserver import ( + "errors" "io" "net/http" "net/http/httptest" @@ -44,7 +45,9 @@ func TestContentTypeSnifferWritesPassThroughAfterFlush(t *testing.T) { rw := newContentTypeSniffer(rec) rw.WriteHeader(http.StatusOK) - _, _ = io.WriteString(rw, "one") + if _, err := io.WriteString(rw, "one"); err != nil { + t.Fatalf("write: %v", err) + } rw.Flush() if _, err := io.WriteString(rw, "two"); err != nil { @@ -75,7 +78,9 @@ func TestContentTypeSnifferFlushWithExplicitContentType(t *testing.T) { rw.Header().Set(HeaderContentType, "text/event-stream") rw.WriteHeader(http.StatusOK) - _, _ = io.WriteString(rw, "data: x\n\n") + if _, err := io.WriteString(rw, "data: x\n\n"); err != nil { + t.Fatalf("write: %v", err) + } rw.Flush() if got := rec.Header().Get(HeaderContentType); got != "text/event-stream" { @@ -112,7 +117,9 @@ func TestContentTypeSnifferSupportsResponseController(t *testing.T) { rw := newContentTypeSniffer(rec) rw.WriteHeader(http.StatusOK) - _, _ = io.WriteString(rw, "chunk") + if _, err := io.WriteString(rw, "chunk"); err != nil { + t.Fatalf("write: %v", err) + } if err := http.NewResponseController(rw).Flush(); err != nil { t.Fatalf("ResponseController.Flush: %v", err) @@ -121,3 +128,79 @@ func TestContentTypeSnifferSupportsResponseController(t *testing.T) { t.Errorf("body = %q, want %q", got, "chunk") } } + +// failingWriter fails the nth write (1-based) and succeeds otherwise, so a +// test can target exactly the write that Flush triggers. +type failingWriter struct { + http.ResponseWriter + failOn int + writes int +} + +func (w *failingWriter) Write(b []byte) (int, error) { + w.writes++ + if w.writes == w.failOn { + return 0, errors.New("write failed") + } + return w.ResponseWriter.Write(b) +} + +func (w *failingWriter) Flush() {} + +// http.Flusher cannot report an error, so a failure while emitting the prefix +// must be remembered and surfaced from the next Write. The unwritten prefix +// must also survive rather than being silently dropped. +func TestContentTypeSnifferFlushWriteFailureIsSurfacedLater(t *testing.T) { + fw := &failingWriter{ResponseWriter: httptest.NewRecorder(), failOn: 1} + rw := newContentTypeSniffer(fw) + + rw.WriteHeader(http.StatusOK) + if _, err := io.WriteString(rw, "held back"); err != nil { + t.Fatalf("write: %v", err) + } + + rw.Flush() // triggers the prefix write, which fails + + if len(rw.prefix) == 0 { + t.Error("prefix was dropped even though the write failed") + } + + if _, err := io.WriteString(rw, "next"); err == nil { + t.Error("the flush-time failure was swallowed; later Write reported success") + } + if _, err := rw.complete(); err == nil { + t.Error("the flush-time failure was swallowed; complete reported success") + } +} + +// A short write during the prefix flush must retain only the unsent remainder. +func TestContentTypeSnifferShortWriteKeepsRemainder(t *testing.T) { + rec := httptest.NewRecorder() + sw := &shortWriter{ResponseWriter: rec, limit: 3} + rw := newContentTypeSniffer(sw) + + rw.WriteHeader(http.StatusOK) + if _, err := io.WriteString(rw, "abcdef"); err != nil { + t.Fatalf("write: %v", err) + } + rw.Flush() + + if got := string(rw.prefix); got != "def" { + t.Errorf("remaining prefix = %q, want %q", got, "def") + } +} + +// shortWriter accepts at most limit bytes per write, without reporting an error. +type shortWriter struct { + http.ResponseWriter + limit int +} + +func (w *shortWriter) Write(b []byte) (int, error) { + if len(b) > w.limit { + b = b[:w.limit] + } + return w.ResponseWriter.Write(b) +} + +func (w *shortWriter) Flush() {} From 05ca219f2e3e77e640e34da4bbb491e6979dbf82 Mon Sep 17 00:00:00 2001 From: Lea Anthony Date: Mon, 10 Aug 2026 00:18:28 +1000 Subject: [PATCH 3/3] test(v3): make the short-write helper honour the io.Writer contract shortWriter returned a short count with a nil error, which io.Writer forbids: an implementation must return a non-nil error whenever it accepts fewer bytes than it was offered. It now reports io.ErrShortWrite, which also makes the test exercise the realistic path - the sniffer records that error and surfaces it from the next Write - rather than a state a conforming writer could never produce. The test now asserts all three: the accepted bytes reach the client, the unsent remainder is retained, and the error reaches the caller. --- .../assetserver/content_type_sniffer_test.go | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/v3/internal/assetserver/content_type_sniffer_test.go b/v3/internal/assetserver/content_type_sniffer_test.go index ce383d7ca7f..9214456926b 100644 --- a/v3/internal/assetserver/content_type_sniffer_test.go +++ b/v3/internal/assetserver/content_type_sniffer_test.go @@ -173,7 +173,8 @@ func TestContentTypeSnifferFlushWriteFailureIsSurfacedLater(t *testing.T) { } } -// A short write during the prefix flush must retain only the unsent remainder. +// A short write during the prefix flush must retain only the unsent remainder, +// and the accompanying error must reach the caller afterwards. func TestContentTypeSnifferShortWriteKeepsRemainder(t *testing.T) { rec := httptest.NewRecorder() sw := &shortWriter{ResponseWriter: rec, limit: 3} @@ -185,22 +186,34 @@ func TestContentTypeSnifferShortWriteKeepsRemainder(t *testing.T) { } rw.Flush() + if got := rec.Body.String(); got != "abc" { + t.Errorf("wrote %q, want the accepted prefix %q", got, "abc") + } if got := string(rw.prefix); got != "def" { t.Errorf("remaining prefix = %q, want %q", got, "def") } + if _, err := io.WriteString(rw, "more"); !errors.Is(err, io.ErrShortWrite) { + t.Errorf("later Write error = %v, want io.ErrShortWrite", err) + } } -// shortWriter accepts at most limit bytes per write, without reporting an error. +// shortWriter accepts at most limit bytes per write. io.Writer requires a +// non-nil error whenever fewer bytes are accepted than were offered, so it +// reports io.ErrShortWrite rather than a bare short count. type shortWriter struct { http.ResponseWriter limit int } func (w *shortWriter) Write(b []byte) (int, error) { - if len(b) > w.limit { - b = b[:w.limit] + if len(b) <= w.limit { + return w.ResponseWriter.Write(b) } - return w.ResponseWriter.Write(b) + n, err := w.ResponseWriter.Write(b[:w.limit]) + if err != nil { + return n, err + } + return n, io.ErrShortWrite } func (w *shortWriter) Flush() {}