Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(168)

Issue 92590043: code review 92590043: net/http/httputil: ensure DumpRequestOut dump all of Body

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 7 months ago by smh
Modified:
11 years, 3 months ago
Reviewers:
rsc, bradfitz
CC:
golang-codereviews
Visibility:
Public.

Description

net/http/httputil: ensure DumpRequestOut dump all of Body Force a read of Body so requests larger than the buffer are read completely and hence captured correctly. Fixes issue 8089.

Patch Set 1 #

Patch Set 2 : diff -r 0ba5ecac8038 https://code.google.com/p/go #

Total comments: 2

Patch Set 3 : diff -r 0ba5ecac8038 https://code.google.com/p/go #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -1 line) Patch
M src/pkg/net/http/httputil/dump.go View 1 chunk +6 lines, -1 line 0 comments Download
M src/pkg/net/http/httputil/dump_test.go View 2 chunks +28 lines, -0 lines 1 comment Download

Messages

Total messages: 5
bradfitz
https://codereview.appspot.com/92590043/diff/20001/src/pkg/net/http/httputil/dump.go File src/pkg/net/http/httputil/dump.go (right): https://codereview.appspot.com/92590043/diff/20001/src/pkg/net/http/httputil/dump.go#newcode104 src/pkg/net/http/httputil/dump.go:104: ioutil.ReadAll(req.Body) no need to read it to memory. just: ...
11 years, 7 months ago (2014-06-04 20:50:01 UTC) #1
rsc
please run 'hg mail' if you want to continue this review
11 years, 3 months ago (2014-09-16 15:31:38 UTC) #2
smh
Hello bradfitz@golang.org, rsc@golang.org (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
11 years, 3 months ago (2014-09-16 16:04:13 UTC) #3
bradfitz
https://codereview.appspot.com/92590043/diff/40001/src/pkg/net/http/httputil/dump_test.go File src/pkg/net/http/httputil/dump_test.go (right): https://codereview.appspot.com/92590043/diff/40001/src/pkg/net/http/httputil/dump_test.go#newcode189 src/pkg/net/http/httputil/dump_test.go:189: return strings.Repeat(s, l/len(s)+1)[:l] this doesn't even compile. Please fix ...
11 years, 3 months ago (2014-09-24 17:15:35 UTC) #4
bradfitz
11 years, 3 months ago (2014-09-30 18:17:31 UTC) #5
R=close

I've taken over this fix in 144650044. Even with the strings/[]byte fix to make
it compile, the tests didn't pass.

Please test that code compiles & passes tests before sending reviews in the
future.

Thanks for identifying the problem area, though.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b