-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
reverseproxy: Rewrite request buffering for fastcgi #6639
base: master
Are you sure you want to change the base?
Changes from all commits
4715bbf
d395633
02b8640
7d483ff
8c803b5
d26cd24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,8 @@ import ( | |
"net/http" | ||
"strconv" | ||
|
||
"github.com/dustin/go-humanize" | ||
|
||
"github.com/caddyserver/caddy/v2" | ||
"github.com/caddyserver/caddy/v2/caddyconfig" | ||
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" | ||
|
@@ -43,7 +45,10 @@ func init() { | |
// dial_timeout <duration> | ||
// read_timeout <duration> | ||
// write_timeout <duration> | ||
// capture_stderr | ||
// body_buffer_disabled | ||
// body_buffer_memory_limit <size> | ||
// file_buffer_size_limit <size> | ||
// file_buffer_filepath <path> | ||
Comment on lines
+48
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's ambiguous whether these configure the request or response body buffer. |
||
// } | ||
func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { | ||
d.Next() // consume transport name | ||
|
@@ -113,6 +118,35 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { | |
} | ||
t.CaptureStderr = true | ||
|
||
case "body_buffer_disabled": | ||
t.BodyBufferDisabled = true | ||
|
||
case "body_buffer_memory_limit": | ||
if !d.NextArg() { | ||
return d.ArgErr() | ||
} | ||
size, err := humanize.ParseBytes(d.Val()) | ||
if err != nil { | ||
return d.Errf("bad buffer size %s: %v", d.Val(), err) | ||
} | ||
t.BodyBufferMemoryLimit = int64(size) | ||
|
||
case "file_buffer_size_limit": | ||
if !d.NextArg() { | ||
return d.ArgErr() | ||
} | ||
size, err := humanize.ParseBytes(d.Val()) | ||
if err != nil { | ||
return d.Errf("bad buffer size %s: %v", d.Val(), err) | ||
} | ||
t.FileBufferSizeLimit = int64(size) | ||
|
||
case "file_buffer_filepath": | ||
if !d.NextArg() { | ||
return d.ArgErr() | ||
} | ||
t.FileBufferFilepath = d.Val() | ||
|
||
default: | ||
return d.Errf("unrecognized subdirective %s", d.Val()) | ||
} | ||
|
@@ -294,6 +328,40 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error | |
args := dispenser.RemainingArgs() | ||
dispenser.DeleteN(len(args) + 1) | ||
fcgiTransport.CaptureStderr = true | ||
|
||
case "body_buffer_disabled": | ||
args := dispenser.RemainingArgs() | ||
dispenser.DeleteN(len(args) + 1) | ||
fcgiTransport.BodyBufferDisabled = true | ||
|
||
case "body_buffer_memory_limit": | ||
if !dispenser.NextArg() { | ||
return nil, dispenser.ArgErr() | ||
} | ||
size, err := humanize.ParseBytes(dispenser.Val()) | ||
if err != nil { | ||
return nil, dispenser.Errf("bad buffer size %s: %v", dispenser.Val(), err) | ||
} | ||
fcgiTransport.BodyBufferMemoryLimit = int64(size) | ||
dispenser.DeleteN(2) | ||
|
||
case "file_buffer_size_limit": | ||
if !dispenser.NextArg() { | ||
return nil, dispenser.ArgErr() | ||
} | ||
size, err := humanize.ParseBytes(dispenser.Val()) | ||
if err != nil { | ||
return nil, dispenser.Errf("bad buffer size %s: %v", dispenser.Val(), err) | ||
} | ||
fcgiTransport.FileBufferSizeLimit = int64(size) | ||
dispenser.DeleteN(2) | ||
|
||
case "file_buffer_filepath": | ||
if !dispenser.NextArg() { | ||
return nil, dispenser.ArgErr() | ||
} | ||
fcgiTransport.FileBufferFilepath = dispenser.Val() | ||
dispenser.DeleteN(2) | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,9 +128,11 @@ var pad [maxPad]byte | |
type client struct { | ||
rwc net.Conn | ||
// keepAlive bool // TODO: implement | ||
reqID uint16 | ||
stderr bool | ||
logger *zap.Logger | ||
reqID uint16 | ||
stderr bool | ||
logger *zap.Logger | ||
buffer bool | ||
bufferFunc func(io.Reader) (int64, io.ReadCloser, error) | ||
} | ||
|
||
// Do made the request and returns a io.Reader that translates the data read | ||
|
@@ -158,6 +160,10 @@ func (c *client) Do(p map[string]string, req io.Reader) (r io.Reader, err error) | |
if err != nil { | ||
return nil, err | ||
} | ||
// body length mismatch | ||
if lr, ok := req.(*io.LimitedReader); ok && lr.N > 0 { | ||
return nil, io.ErrUnexpectedEOF | ||
} | ||
} | ||
err = writer.FlushStream() | ||
if err != nil { | ||
|
@@ -197,9 +203,68 @@ func (f clientCloser) Close() error { | |
return f.rwc.Close() | ||
} | ||
|
||
// create a response that describes the error message, it's passed to the client. | ||
// caller should close the connection to fastcgi server | ||
func newErrorResponse(status int) *http.Response { | ||
statusText := http.StatusText(status) | ||
resp := &http.Response{ | ||
Status: statusText, | ||
StatusCode: status, | ||
Header: make(http.Header), | ||
Body: io.NopCloser(strings.NewReader(statusText)), | ||
ContentLength: int64(len(statusText)), | ||
} | ||
resp.Header.Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10)) | ||
resp.Header.Set("Content-Type", "text/plain; charset=utf-8") | ||
resp.Header.Set("X-Content-Type-Options", "nosniff") | ||
return resp | ||
} | ||
|
||
// check for invalid content_length to determine if the request should be buffered | ||
func checkContentLength(p map[string]string) (int64, bool) { | ||
clStr, ok := p["CONTENT_LENGTH"] | ||
if !ok { | ||
return 0, false | ||
} | ||
cl, err := strconv.ParseInt(clStr, 10, 64) | ||
if err != nil || cl < 0 { | ||
return 0, false | ||
} | ||
return cl, true | ||
} | ||
|
||
// Request returns a HTTP Response with Header and Body | ||
// from fcgi responder | ||
func (c *client) Request(p map[string]string, req io.Reader) (resp *http.Response, err error) { | ||
// defer closing the request body if it's an io.Closer | ||
if closer, ok := req.(io.Closer); ok { | ||
defer closer.Close() | ||
} | ||
|
||
// check for content_length and buffer the request if needed | ||
cl, ok := checkContentLength(p) | ||
if !ok { | ||
// buffering disabled | ||
if !c.buffer { | ||
c.rwc.Close() | ||
return newErrorResponse(http.StatusLengthRequired), nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, would it be better to throw a caddyhttp.Error instead? That way it could be caught by |
||
} else { | ||
// buffer the request | ||
size, rc, err := c.bufferFunc(req) | ||
if err != nil { | ||
if err == errFileBufferExceeded { | ||
c.rwc.Close() | ||
return newErrorResponse(http.StatusRequestEntityTooLarge), nil | ||
} | ||
return nil, err | ||
} | ||
defer rc.Close() | ||
p["CONTENT_LENGTH"] = strconv.FormatInt(size, 10) | ||
} | ||
} else { | ||
req = io.LimitReader(req, cl) | ||
} | ||
|
||
r, err := c.Do(p, req) | ||
if err != nil { | ||
return | ||
|
@@ -302,7 +367,7 @@ func (c *client) Post(p map[string]string, method string, bodyType string, body | |
// PostForm issues a POST to the fcgi responder, with form | ||
// as a string key to a list values (url.Values) | ||
func (c *client) PostForm(p map[string]string, data url.Values) (resp *http.Response, err error) { | ||
body := bytes.NewReader([]byte(data.Encode())) | ||
body := strings.NewReader(data.Encode()) | ||
return c.Post(p, "POST", "application/x-www-form-urlencoded", body, int64(body.Len())) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you meant to remove this