Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(401)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

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.
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
Created: 11 years, 3 months ago
Download [raw] [tar.bz2]
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
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
https://codereview.appspot.com/92590043/diff/20001/src/pkg/net/http/httputil/...
File src/pkg/net/http/httputil/dump.go (right):
https://codereview.appspot.com/92590043/diff/20001/src/pkg/net/http/httputil/...
src/pkg/net/http/httputil/dump.go:104: ioutil.ReadAll(req.Body)
no need to read it to memory. just:
io.Copy(ioutil.Discard, req.Body)
https://codereview.appspot.com/92590043/diff/20001/src/pkg/net/http/httputil/...
File src/pkg/net/http/httputil/dump_test.go (right):
https://codereview.appspot.com/92590043/diff/20001/src/pkg/net/http/httputil/...
src/pkg/net/http/httputil/dump_test.go:188: func sizedBody(s string, l int)
[]byte {
This could just be:
// sizedBody returns a string of length l containing s repeatedly.
func sizedBody(s string, l int) string{
 return strings.Repeat(s, l/len(s)+1)[:l]
}
Sign in to reply to this message.
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
please run 'hg mail' if you want to continue this review
Sign in to reply to this message.
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
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 
Sign in to reply to this message.
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
https://codereview.appspot.com/92590043/diff/40001/src/pkg/net/http/httputil/...
File src/pkg/net/http/httputil/dump_test.go (right):
https://codereview.appspot.com/92590043/diff/40001/src/pkg/net/http/httputil/...
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 and verify your test actually passes. Please note you'll need to
update the CL to change the files to be at src/net/http/httputil/* now. The
"pkg" directory in has been removed. (src/pkg is now just src)
Sign in to reply to this message.
bradfitz
R=close I've taken over this fix in 144650044. Even with the strings/[]byte fix to make ...
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

AltStyle によって変換されたページ (->オリジナル) /