From 7c369904334435d3cd3d119f31ba939a235fbdb8 Mon Sep 17 00:00:00 2001 From: bensynapse <118375461+bensynapse@users.noreply.github.com> Date: Thu, 8 Oct 2026 12:01:49 +0300 Subject: [PATCH] Return destination write errors from rendering engines --- engine.go | 48 +++++--- engine_test.go | 299 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 18 deletions(-) create mode 100644 engine_test.go diff --git a/engine.go b/engine.go index e3103bb..595d31a 100644 --- a/engine.go +++ b/engine.go @@ -87,9 +87,9 @@ func (d Data) Render(w io.Writer, v interface{}) error { d.Write(hw) } - _, _ = w.Write(v.([]byte)) + _, err := w.Write(v.([]byte)) - return nil + return err } // Render a HTML response. @@ -110,9 +110,9 @@ func (h HTML) Render(w io.Writer, binding interface{}) error { h.Write(hw) } - _, _ = buf.WriteTo(w) + _, err = buf.WriteTo(w) - return nil + return err } // Render a JSON response. @@ -141,7 +141,9 @@ func (j JSON) Render(w io.Writer, v interface{}) error { } if len(j.Prefix) > 0 { - _, _ = w.Write(j.Prefix) + if _, err := w.Write(j.Prefix); err != nil { + return err + } } // Remove the newline that json.Encode injects when not indenting the output. @@ -149,9 +151,9 @@ func (j JSON) Render(w io.Writer, v interface{}) error { output = bytes.TrimSuffix(output, []byte("\n")) } - _, _ = w.Write(output) + _, err := w.Write(output) - return nil + return err } func (j JSON) renderStreamingJSON(w io.Writer, v interface{}) error { @@ -160,7 +162,9 @@ func (j JSON) renderStreamingJSON(w io.Writer, v interface{}) error { } if len(j.Prefix) > 0 { - _, _ = w.Write(j.Prefix) + if _, err := w.Write(j.Prefix); err != nil { + return err + } } encoder := j.Encoder(w) @@ -194,16 +198,22 @@ func (j JSONP) Render(w io.Writer, v interface{}) error { j.Write(hw) } - _, _ = w.Write([]byte(j.Callback + "(")) - _, _ = w.Write(result) - _, _ = w.Write([]byte(");")) + if _, err = w.Write([]byte(j.Callback + "(")); err != nil { + return err + } + if _, err = w.Write(result); err != nil { + return err + } + if _, err = w.Write([]byte(");")); err != nil { + return err + } // If indenting, append a new line. if j.Indent { - _, _ = w.Write([]byte("\n")) + _, err = w.Write([]byte("\n")) } - return nil + return err } // Render a text response. @@ -217,9 +227,9 @@ func (t Text) Render(w io.Writer, v interface{}) error { t.Write(hw) } - _, _ = w.Write([]byte(v.(string))) + _, err := w.Write([]byte(v.(string))) - return nil + return err } // Render an XML response. @@ -245,10 +255,12 @@ func (x XML) Render(w io.Writer, v interface{}) error { } if len(x.Prefix) > 0 { - _, _ = w.Write(x.Prefix) + if _, err = w.Write(x.Prefix); err != nil { + return err + } } - _, _ = w.Write(result) + _, err = w.Write(result) - return nil + return err } diff --git a/engine_test.go b/engine_test.go new file mode 100644 index 0000000..99b0e17 --- /dev/null +++ b/engine_test.go @@ -0,0 +1,299 @@ +package render + +import ( + "bytes" + "fmt" + "io" + "math" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +type engineTestWriter struct { + failAt int + partial bool + writes int + body bytes.Buffer +} + +func (w *engineTestWriter) Write(p []byte) (int, error) { + w.writes++ + if w.writes == w.failAt { + n := 0 + if w.partial { + n = len(p) / 2 + } + _, _ = w.body.Write(p[:n]) + + return n, io.ErrClosedPipe + } + + return w.body.Write(p) +} + +type engineTestResponseWriter struct { + *engineTestWriter + header http.Header + headerWrites int + status int +} + +func (w *engineTestResponseWriter) Header() http.Header { + return w.header +} + +func (w *engineTestResponseWriter) WriteHeader(status int) { + w.headerWrites++ + w.status = status +} + +type engineWriteCase struct { + name string + options Options + call func(*Render, io.Writer, int) error + parts []string + contentType string +} + +func engineWriteCases() []engineWriteCase { + data := func(r *Render, w io.Writer, status int) error { + return r.Data(w, status, []byte("payload")) + } + text := func(r *Render, w io.Writer, status int) error { + return r.Text(w, status, "payload") + } + html := func(r *Render, w io.Writer, status int) error { + return r.HTML(w, status, "hello", "gophers") + } + json := func(r *Render, w io.Writer, status int) error { + return r.JSON(w, status, map[string]string{"name": "value"}) + } + jsonp := func(r *Render, w io.Writer, status int) error { + return r.JSONP(w, status, "callback", map[string]string{"name": "value"}) + } + xml := func(r *Render, w io.Writer, status int) error { + return r.XML(w, status, GreetingXML{One: "hello", Two: "world"}) + } + + const ( + jsonBody = `{"name":"value"}` + indentedJSONBody = "{\n \"name\": \"value\"\n}\n" + xmlBody = `` + jsonPrefix = ")]}'\n" + xmlPrefix = "\n" + charset = "; charset=UTF-8" + ) + + return []engineWriteCase{ + { + name: "data", call: data, parts: []string{"payload"}, contentType: ContentBinary, + }, + { + name: "text", call: text, parts: []string{"payload"}, contentType: ContentText + charset, + }, + { + name: "html", call: html, parts: []string{"

Hello gophers

\n"}, contentType: ContentHTML + charset, + }, + { + name: "json", call: json, parts: []string{jsonBody}, contentType: ContentJSON + charset, + }, + { + name: "json-prefix", call: json, parts: []string{jsonPrefix, jsonBody}, contentType: ContentJSON + charset, + options: Options{PrefixJSON: []byte(jsonPrefix)}, + }, + { + name: "json-indented-prefix", call: json, parts: []string{jsonPrefix, indentedJSONBody}, contentType: ContentJSON + charset, + options: Options{PrefixJSON: []byte(jsonPrefix), IndentJSON: true}, + }, + { + name: "streaming-json", call: json, parts: []string{jsonBody + "\n"}, contentType: ContentJSON + charset, + options: Options{StreamingJSON: true}, + }, + { + name: "streaming-json-prefix", call: json, parts: []string{jsonPrefix, jsonBody + "\n"}, contentType: ContentJSON + charset, + options: Options{PrefixJSON: []byte(jsonPrefix), StreamingJSON: true}, + }, + { + name: "jsonp", call: jsonp, parts: []string{"callback(", jsonBody, ");"}, contentType: ContentJSONP + charset, + }, + { + name: "jsonp-indented", call: jsonp, parts: []string{"callback(", strings.TrimSuffix(indentedJSONBody, "\n"), ");", "\n"}, contentType: ContentJSONP + charset, + options: Options{IndentJSON: true}, + }, + { + name: "xml", call: xml, parts: []string{xmlBody}, contentType: ContentXML + charset, + }, + { + name: "xml-prefix", call: xml, parts: []string{xmlPrefix, xmlBody}, contentType: ContentXML + charset, + options: Options{PrefixXML: []byte(xmlPrefix)}, + }, + { + name: "xml-indented-prefix", call: xml, parts: []string{xmlPrefix, xmlBody + "\n"}, contentType: ContentXML + charset, + options: Options{PrefixXML: []byte(xmlPrefix), IndentXML: true}, + }, + } +} + +func TestRenderDestinationWriteErrors(t *testing.T) { + for _, test := range engineWriteCases() { + t.Run(test.name, func(t *testing.T) { + options := test.options + options.Directory = "testdata/basic" + options.DisableHTTPErrorRendering = true + render := New(options) + + for failAt := 1; failAt <= len(test.parts); failAt++ { + for _, partial := range []bool{false, true} { + for _, httpWriter := range []bool{false, true} { + name := fmt.Sprintf("write%d/partial=%t/http=%t", failAt, partial, httpWriter) + t.Run(name, func(t *testing.T) { + writer := &engineTestWriter{failAt: failAt, partial: partial} + var destination io.Writer = writer + var response *engineTestResponseWriter + if httpWriter { + response = &engineTestResponseWriter{engineTestWriter: writer, header: make(http.Header)} + destination = response + } + + err := test.call(render, destination, http.StatusAccepted) + expect(t, err, io.ErrClosedPipe) + expect(t, writer.writes, failAt) + + want := strings.Join(test.parts[:failAt-1], "") + if partial { + failedPart := test.parts[failAt-1] + want += failedPart[:len(failedPart)/2] + } + expect(t, writer.body.String(), want) + if response != nil { + expect(t, response.headerWrites, 1) + expect(t, response.status, http.StatusAccepted) + expect(t, response.Header().Get(ContentType), test.contentType) + } + }) + } + } + } + }) + } +} + +func TestRenderDestinationWriteSuccess(t *testing.T) { + for _, test := range engineWriteCases() { + t.Run(test.name, func(t *testing.T) { + options := test.options + options.Directory = "testdata/basic" + options.DisableHTTPErrorRendering = true + render := New(options) + + for _, httpWriter := range []bool{false, true} { + t.Run(fmt.Sprintf("http=%t", httpWriter), func(t *testing.T) { + writer := &engineTestWriter{} + var destination io.Writer = writer + var response *engineTestResponseWriter + if httpWriter { + response = &engineTestResponseWriter{engineTestWriter: writer, header: make(http.Header)} + destination = response + } + + expectNil(t, test.call(render, destination, http.StatusAccepted)) + expect(t, writer.body.String(), strings.Join(test.parts, "")) + if response != nil { + expect(t, response.headerWrites, 1) + expect(t, response.status, http.StatusAccepted) + expect(t, response.Header().Get(ContentType), test.contentType) + } + }) + } + }) + } +} + +func TestRenderEncodingErrorsBeforeDestinationWrite(t *testing.T) { + cases := []struct { + name string + call func(*Render, io.Writer) error + }{ + { + name: "json", + call: func(r *Render, w io.Writer) error { return r.JSON(w, http.StatusAccepted, math.NaN()) }, + }, + { + name: "jsonp", + call: func(r *Render, w io.Writer) error { return r.JSONP(w, http.StatusAccepted, "callback", math.NaN()) }, + }, + { + name: "xml", + call: func(r *Render, w io.Writer) error { return r.XML(w, http.StatusAccepted, func() {}) }, + }, + { + name: "html", + call: func(r *Render, w io.Writer) error { return r.HTML(w, http.StatusAccepted, "missing", nil) }, + }, + } + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + render := New(Options{ + Directory: "testdata/basic", + DisableHTTPErrorRendering: true, + PrefixJSON: []byte("prefix"), + PrefixXML: []byte("prefix"), + }) + for _, httpWriter := range []bool{false, true} { + t.Run(fmt.Sprintf("http=%t", httpWriter), func(t *testing.T) { + writer := &engineTestWriter{failAt: 1} + var destination io.Writer = writer + var response *engineTestResponseWriter + if httpWriter { + response = &engineTestResponseWriter{engineTestWriter: writer, header: make(http.Header)} + destination = response + } + + expectNotNil(t, test.call(render, destination)) + expect(t, writer.writes, 0) + expect(t, writer.body.Len(), 0) + if response != nil { + expect(t, response.headerWrites, 0) + expect(t, response.Header().Get(ContentType), "") + } + }) + } + }) + } +} + +func TestRenderNoContentWriteError(t *testing.T) { + for _, test := range engineWriteCases() { + t.Run(test.name, func(t *testing.T) { + options := test.options + options.Directory = "testdata/basic" + options.DisableHTTPErrorRendering = true + render := New(options) + writeErrors := make(chan error, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeErrors <- test.call(render, w, http.StatusNoContent) + })) + defer server.Close() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL, nil) + if err != nil { + t.Fatal(err) + } + response, err := server.Client().Do(req) + if err != nil { + t.Fatal(err) + } + defer func() { + expectNil(t, response.Body.Close()) + }() + + body, err := io.ReadAll(response.Body) + expectNil(t, err) + expect(t, response.StatusCode, http.StatusNoContent) + expect(t, string(body), "") + expect(t, <-writeErrors, http.ErrBodyNotAllowed) + }) + } +}