Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Commit

Permalink
Fixed a bug that breaks large upload requests.
Browse files Browse the repository at this point in the history
When a client issue an upload request of sufficient size to WebPageReplay (10MB +), WPR resets the connection, sends a TCP RST error, and causes the client to abort the upload.

The root cause for this issue is that WPR does not clear the request body buffer. Uploading a large file to WPR is an uncommon scenario. Since WPR does not use the request body, it never reads the request body.

Here is how I speculate this error plays out:

When a client issues an upload request, the request body buffer fills up and blocks further upload bytes from being sent. When WPR responds to the request, the underlying Go code closes the input channel of the connection to prevent the client from sending more data. This results in the operating system issuing a TCP "connection reset by peer" message back to the client.

The fix is to read every request body to ensure that the request body
buffer is always clear, before responding to requests.

Bug: catapult:#1215668
Change-Id: I2b2880b2487c2412eb9a8f70558e8f9700ae11af
Reviewed-on: https://chromium-review.googlesource.com/c/catapult/+/2934719
Reviewed-by: Ian Struiksma <ianstruiksma@google.com>
Commit-Queue: Yiming Zhou <uwyiming@google.com>
  • Loading branch information
Yiming Zhou authored and Catapult LUCI CQ committed Jun 7, 2021
1 parent c8ae7ec commit 282b92a
Showing 1 changed file with 25 additions and 1 deletion.
26 changes: 25 additions & 1 deletion web_page_replay_go/src/webpagereplay/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,33 @@ func assertCompleteURL(url *url.URL) {
}

// FindRequest searches for the given request in the archive.
// Returns ErrNotFound if the request could not be found. Does not consume req.Body.
// Returns ErrNotFound if the request could not be found.
//
// Does not use the request body, but reads the request body to
// prevent WPR from issuing a Connection Reset error when
// handling large upload requests.
// (https://bugs.chromium.org/p/chromium/issues/detail?id=1215668)
//
// TODO: conditional requests
func (a *Archive) FindRequest(req *http.Request) (*http.Request, *http.Response, error) {
// Clear the input channel on large uploads to prevent WPR
// from resetting the connection, and causing the upload
// to fail.
// Large upload is an uncommon scenario for WPR users. To
// avoid exacting an overhead on every request, restrict
// the operation to large uploads only (size > 1MB).
if req.Body != nil &&
(strings.EqualFold("POST", req.Method) || strings.EqualFold("PUT", req.Method)) &&
req.ContentLength > 2<<20 {
buf := make([]byte, 1024)
for {
_, read_err := req.Body.Read(buf)
if read_err == io.EOF {
break
}
}
}

hostMap := a.Requests[req.Host]
if len(hostMap) == 0 {
return nil, nil, ErrNotFound
Expand Down

0 comments on commit 282b92a

Please sign in to comment.