Skip to content

Commit

Permalink
caddyhttp: Make use of http.ResponseController (#5654)
Browse files Browse the repository at this point in the history
* caddyhttp: Make use of http.ResponseController

Also syncs the reverseproxy implementation with stdlib's which now uses ResponseController as well golang/go@2449bbb

* Enable full-duplex for HTTP/1.1

* Appease linter

* Add warning for builds with Go 1.20, so it's less surprising to users

* Improved godoc for EnableFullDuplex, copied text from stdlib

* Only wrap in encode if not already wrapped
  • Loading branch information
francislavoie committed Aug 2, 2023
1 parent e198c60 commit cd486c2
Show file tree
Hide file tree
Showing 14 changed files with 166 additions and 94 deletions.
8 changes: 8 additions & 0 deletions caddyconfig/httpcaddyfile/serveroptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type serverOptions struct {
IdleTimeout caddy.Duration
KeepAliveInterval caddy.Duration
MaxHeaderBytes int
EnableFullDuplex bool
Protocols []string
StrictSNIHost *bool
TrustedProxiesRaw json.RawMessage
Expand Down Expand Up @@ -157,6 +158,12 @@ func unmarshalCaddyfileServerOptions(d *caddyfile.Dispenser) (any, error) {
}
serverOpts.MaxHeaderBytes = int(size)

case "enable_full_duplex":
if d.NextArg() {
return nil, d.ArgErr()
}
serverOpts.EnableFullDuplex = true

case "log_credentials":
if d.NextArg() {
return nil, d.ArgErr()
Expand Down Expand Up @@ -327,6 +334,7 @@ func applyServerOptions(
server.IdleTimeout = opts.IdleTimeout
server.KeepAliveInterval = opts.KeepAliveInterval
server.MaxHeaderBytes = opts.MaxHeaderBytes
server.EnableFullDuplex = opts.EnableFullDuplex
server.Protocols = opts.Protocols
server.StrictSNIHost = opts.StrictSNIHost
server.TrustedProxiesRaw = opts.TrustedProxiesRaw
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
idle 30s
}
max_header_size 100MB
enable_full_duplex
log_credentials
protocols h1 h2 h2c h3
strict_sni_host
Expand Down Expand Up @@ -45,6 +46,7 @@ foo.com {
"write_timeout": 30000000000,
"idle_timeout": 30000000000,
"max_header_bytes": 100000000,
"enable_full_duplex": true,
"routes": [
{
"match": [
Expand Down
4 changes: 1 addition & 3 deletions caddytest/integration/stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,7 @@ func testH2ToH2CStreamServeH2C(t *testing.T) *http.Server {

w.Header().Set("Cache-Control", "no-store")
w.WriteHeader(200)
if f, ok := w.(http.Flusher); ok {
f.Flush()
}
http.NewResponseController(w).Flush()

buf := make([]byte, 4*1024)

Expand Down
8 changes: 8 additions & 0 deletions modules/caddyhttp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import (
"fmt"
"net"
"net/http"
"runtime"
"strconv"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -325,9 +327,15 @@ func (app *App) Provision(ctx caddy.Context) error {

// Validate ensures the app's configuration is valid.
func (app *App) Validate() error {
isGo120 := strings.Contains(runtime.Version(), "go1.20")

// each server must use distinct listener addresses
lnAddrs := make(map[string]string)
for srvName, srv := range app.Servers {
if isGo120 && srv.EnableFullDuplex {
app.logger.Warn("enable_full_duplex is not supported in Go 1.20, use a build made with Go 1.21 or later", zap.String("server", srvName))
}

for _, addr := range srv.Listen {
listenAddr, err := caddy.ParseNetworkAddress(addr)
if err != nil {
Expand Down
25 changes: 25 additions & 0 deletions modules/caddyhttp/duplex_go120.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2015 Matthew Holt and The Caddy Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//go:build !go1.21

package caddyhttp

import (
"net/http"
)

func enableFullDuplex(w http.ResponseWriter) {
// Do nothing, Go 1.20 and earlier do not support full duplex
}
25 changes: 25 additions & 0 deletions modules/caddyhttp/duplex_go121.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2015 Matthew Holt and The Caddy Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//go:build go1.21

package caddyhttp

import (
"net/http"
)

func enableFullDuplex(w http.ResponseWriter) {
http.NewResponseController(w).EnableFullDuplex()
}
27 changes: 14 additions & 13 deletions modules/caddyhttp/encode/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,10 @@ func (enc *Encode) openResponseWriter(encodingName string, w http.ResponseWriter
// initResponseWriter initializes the responseWriter instance
// allocated in openResponseWriter, enabling mid-stack inlining.
func (enc *Encode) initResponseWriter(rw *responseWriter, encodingName string, wrappedRW http.ResponseWriter) *responseWriter {
if httpInterfaces, ok := wrappedRW.(caddyhttp.HTTPInterfaces); ok {
rw.HTTPInterfaces = httpInterfaces
if rww, ok := wrappedRW.(*caddyhttp.ResponseWriterWrapper); ok {
rw.ResponseWriter = rww
} else {
rw.HTTPInterfaces = &caddyhttp.ResponseWriterWrapper{ResponseWriter: wrappedRW}
rw.ResponseWriter = &caddyhttp.ResponseWriterWrapper{ResponseWriter: wrappedRW}
}
rw.encodingName = encodingName
rw.config = enc
Expand All @@ -182,7 +182,7 @@ func (enc *Encode) initResponseWriter(rw *responseWriter, encodingName string, w
// using the encoding represented by encodingName and
// configured by config.
type responseWriter struct {
caddyhttp.HTTPInterfaces
http.ResponseWriter
encodingName string
w Encoder
config *Encode
Expand Down Expand Up @@ -211,19 +211,21 @@ func (rw *responseWriter) Flush() {
// to rw.Write (see bug in #4314)
return
}
rw.HTTPInterfaces.Flush()
//nolint:bodyclose
http.NewResponseController(rw).Flush()
}

// Hijack implements http.Hijacker. It will flush status code if set. We don't track actual hijacked
// status assuming http middlewares will track its status.
func (rw *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
if !rw.wroteHeader {
if rw.statusCode != 0 {
rw.HTTPInterfaces.WriteHeader(rw.statusCode)
rw.ResponseWriter.WriteHeader(rw.statusCode)
}
rw.wroteHeader = true
}
return rw.HTTPInterfaces.Hijack()
//nolint:bodyclose
return http.NewResponseController(rw).Hijack()
}

// Write writes to the response. If the response qualifies,
Expand Down Expand Up @@ -260,15 +262,15 @@ func (rw *responseWriter) Write(p []byte) (int, error) {
// by the standard library
if !rw.wroteHeader {
if rw.statusCode != 0 {
rw.HTTPInterfaces.WriteHeader(rw.statusCode)
rw.ResponseWriter.WriteHeader(rw.statusCode)
}
rw.wroteHeader = true
}

if rw.w != nil {
return rw.w.Write(p)
} else {
return rw.HTTPInterfaces.Write(p)
return rw.ResponseWriter.Write(p)
}
}

Expand All @@ -284,7 +286,7 @@ func (rw *responseWriter) Close() error {

// issue #5059, don't write status code if not set explicitly.
if rw.statusCode != 0 {
rw.HTTPInterfaces.WriteHeader(rw.statusCode)
rw.ResponseWriter.WriteHeader(rw.statusCode)
}
rw.wroteHeader = true
}
Expand All @@ -301,7 +303,7 @@ func (rw *responseWriter) Close() error {

// Unwrap returns the underlying ResponseWriter.
func (rw *responseWriter) Unwrap() http.ResponseWriter {
return rw.HTTPInterfaces
return rw.ResponseWriter
}

// init should be called before we write a response, if rw.buf has contents.
Expand All @@ -310,7 +312,7 @@ func (rw *responseWriter) init() {
rw.config.Match(rw) {

rw.w = rw.config.writerPools[rw.encodingName].Get().(Encoder)
rw.w.Reset(rw.HTTPInterfaces)
rw.w.Reset(rw.ResponseWriter)
rw.Header().Del("Content-Length") // https://github.com/golang/go/issues/14975
rw.Header().Set("Content-Encoding", rw.encodingName)
rw.Header().Add("Vary", "Accept-Encoding")
Expand Down Expand Up @@ -429,5 +431,4 @@ var (
_ caddy.Provisioner = (*Encode)(nil)
_ caddy.Validator = (*Encode)(nil)
_ caddyhttp.MiddlewareHandler = (*Encode)(nil)
_ caddyhttp.HTTPInterfaces = (*responseWriter)(nil)
)
2 changes: 1 addition & 1 deletion modules/caddyhttp/headers/headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,5 +371,5 @@ func (rww *responseWriterWrapper) Write(d []byte) (int, error) {
var (
_ caddy.Provisioner = (*Handler)(nil)
_ caddyhttp.MiddlewareHandler = (*Handler)(nil)
_ caddyhttp.HTTPInterfaces = (*responseWriterWrapper)(nil)
_ http.ResponseWriter = (*responseWriterWrapper)(nil)
)
3 changes: 2 additions & 1 deletion modules/caddyhttp/push/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,5 +251,6 @@ const pushedLink = "http.handlers.push.pushed_link"
var (
_ caddy.Provisioner = (*Handler)(nil)
_ caddyhttp.MiddlewareHandler = (*Handler)(nil)
_ caddyhttp.HTTPInterfaces = (*linkPusher)(nil)
_ http.ResponseWriter = (*linkPusher)(nil)
_ http.Pusher = (*linkPusher)(nil)
)
57 changes: 14 additions & 43 deletions modules/caddyhttp/responsewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,34 +24,14 @@ import (
)

// ResponseWriterWrapper wraps an underlying ResponseWriter and
// promotes its Pusher/Flusher/Hijacker methods as well. To use
// this type, embed a pointer to it within your own struct type
// that implements the http.ResponseWriter interface, then call
// methods on the embedded value. You can make sure your type
// wraps correctly by asserting that it implements the
// HTTPInterfaces interface.
// promotes its Pusher method as well. To use this type, embed
// a pointer to it within your own struct type that implements
// the http.ResponseWriter interface, then call methods on the
// embedded value.
type ResponseWriterWrapper struct {
http.ResponseWriter
}

// Hijack implements http.Hijacker. It simply calls the underlying
// ResponseWriter's Hijack method if there is one, or returns
// ErrNotImplemented otherwise.
func (rww *ResponseWriterWrapper) Hijack() (net.Conn, *bufio.ReadWriter, error) {
if hj, ok := rww.ResponseWriter.(http.Hijacker); ok {
return hj.Hijack()
}
return nil, nil, ErrNotImplemented
}

// Flush implements http.Flusher. It simply calls the underlying
// ResponseWriter's Flush method if there is one.
func (rww *ResponseWriterWrapper) Flush() {
if f, ok := rww.ResponseWriter.(http.Flusher); ok {
f.Flush()
}
}

// Push implements http.Pusher. It simply calls the underlying
// ResponseWriter's Push method if there is one, or returns
// ErrNotImplemented otherwise.
Expand All @@ -62,29 +42,18 @@ func (rww *ResponseWriterWrapper) Push(target string, opts *http.PushOptions) er
return ErrNotImplemented
}

// ReadFrom implements io.ReaderFrom. It simply calls the underlying
// ResponseWriter's ReadFrom method if there is one, otherwise it defaults
// to io.Copy.
// ReadFrom implements io.ReaderFrom. It simply calls io.Copy,
// which uses io.ReaderFrom if available.
func (rww *ResponseWriterWrapper) ReadFrom(r io.Reader) (n int64, err error) {
if rf, ok := rww.ResponseWriter.(io.ReaderFrom); ok {
return rf.ReadFrom(r)
}
return io.Copy(rww.ResponseWriter, r)
}

// Unwrap returns the underlying ResponseWriter.
// Unwrap returns the underlying ResponseWriter, necessary for
// http.ResponseController to work correctly.
func (rww *ResponseWriterWrapper) Unwrap() http.ResponseWriter {
return rww.ResponseWriter
}

// HTTPInterfaces mix all the interfaces that middleware ResponseWriters need to support.
type HTTPInterfaces interface {
http.ResponseWriter
http.Pusher
http.Flusher
http.Hijacker
}

// ErrNotImplemented is returned when an underlying
// ResponseWriter does not implement the required method.
var ErrNotImplemented = fmt.Errorf("method not implemented")
Expand Down Expand Up @@ -262,7 +231,8 @@ func (rr *responseRecorder) WriteResponse() error {
}

func (rr *responseRecorder) Hijack() (net.Conn, *bufio.ReadWriter, error) {
conn, brw, err := rr.ResponseWriterWrapper.Hijack()
//nolint:bodyclose
conn, brw, err := http.NewResponseController(rr.ResponseWriterWrapper).Hijack()
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -294,7 +264,7 @@ func (hc *hijackedConn) ReadFrom(r io.Reader) (int64, error) {
// responses instead of writing them to the client. See
// docs for NewResponseRecorder for proper usage.
type ResponseRecorder interface {
HTTPInterfaces
http.ResponseWriter
Status() int
Buffer() *bytes.Buffer
Buffered() bool
Expand All @@ -309,12 +279,13 @@ type ShouldBufferFunc func(status int, header http.Header) bool

// Interface guards
var (
_ HTTPInterfaces = (*ResponseWriterWrapper)(nil)
_ ResponseRecorder = (*responseRecorder)(nil)
_ http.ResponseWriter = (*ResponseWriterWrapper)(nil)
_ ResponseRecorder = (*responseRecorder)(nil)

// Implementing ReaderFrom can be such a significant
// optimization that it should probably be required!
// see PR #5022 (25%-50% speedup)
_ io.ReaderFrom = (*ResponseWriterWrapper)(nil)
_ io.ReaderFrom = (*responseRecorder)(nil)
_ io.ReaderFrom = (*hijackedConn)(nil)
)
5 changes: 2 additions & 3 deletions modules/caddyhttp/reverseproxy/reverseproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -962,9 +962,8 @@ func (h *Handler) finalizeResponse(
// Force chunking if we saw a response trailer.
// This prevents net/http from calculating the length for short
// bodies and adding a Content-Length.
if fl, ok := rw.(http.Flusher); ok {
fl.Flush()
}
//nolint:bodyclose
http.NewResponseController(rw).Flush()
}

// total duration spent proxying, including writing response body
Expand Down

0 comments on commit cd486c2

Please sign in to comment.