Skip to content
Merged
22 changes: 17 additions & 5 deletions agent/kvs_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,19 +234,31 @@ func (s *HTTPHandlers) KVSPut(resp http.ResponseWriter, req *http.Request, args
}

// Check the content-length
if req.ContentLength > int64(s.agent.config.KVMaxValueSize) {
maxSize := int64(s.agent.config.KVMaxValueSize)
if req.ContentLength > maxSize {
return nil, HTTPError{
StatusCode: http.StatusRequestEntityTooLarge,
Reason: fmt.Sprintf("Request body(%d bytes) too large, max size: %d bytes. See %s.",
req.ContentLength, s.agent.config.KVMaxValueSize, "https://developer.hashicorp.com/docs/agent/config/config-files#kv_max_value_size"),
req.ContentLength, maxSize, "https://developer.hashicorp.com/docs/agent/config/config-files#kv_max_value_size"),
}
}

// Copy the value
buf := bytes.NewBuffer(nil)
if _, err := io.Copy(buf, req.Body); err != nil {
// LimitReader to limit copy of large requests with no Content-Length
limitedReader := io.LimitReader(req.Body, maxSize+1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is maxSize+1 needed?

buf := new(bytes.Buffer)
copiedBytes, err := io.Copy(buf, limitedReader)
if err != nil {
return nil, err
}

// Reject request if actual read size exceeds allowed limit
if copiedBytes > maxSize {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are already restricting copiedBytes then why do we need this condition?

return nil, HTTPError{
StatusCode: http.StatusRequestEntityTooLarge,
Reason: fmt.Sprintf("Request body too large, max allowed is %d bytes.", maxSize),
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add jira tickets to oss repo prs.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit tests for this.

applyReq.DirEnt.Value = buf.Bytes()

// Make the RPC
Expand Down
Loading