code review 84480045: net/http: don't reject 0-lengthed bodies with Expect 10...

net/http: don't reject 0-lengthed bodies with Expect 100-continue

I was implementing rules from RFC 2616. The rules are apparently useless,
ambiguous, and too strict for common software on the Internet. (e.g. curl)

Add more tests, including a test of a chunked request.

Fixes Issue 7625

[bradfitz, 2014-04-11 04:40:35]

Hello golang-codereviews@googlegroups.com (cc: adg@golang.org,
dsymonds@golang.org, rsc@golang.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[dsymonds, 2014-04-11 04:46:27]

LGTM

https://codereview.appspot.com/84480045/diff/40001/src/pkg/net/http/serve_tes...
File src/pkg/net/http/serve_test.go (right):

https://codereview.appspot.com/84480045/diff/40001/src/pkg/net/http/serve_tes...
src/pkg/net/http/serve_test.go:1025: var targ io.WriteCloser = struct {
neat trick. I wonder if it's a little too clever though. There's exactly one
place that this would need closing (and don't you want to close it even if
there's a write error?)

var w io.Writer = conn
if test.chunked {
  w = httputil.NewChunkedWriter(conn)
}
body := ...
_, err = fmt.Fprint(w, body)
if test.chunked {
  if cerr := w.Close(); err == nil {
    err = cerr
  }
}

[bradfitz, 2014-04-11 05:19:38]

I will submit this as-is since you don't seem to care too much, and it's easier
to not touch.

https://codereview.appspot.com/84480045/diff/40001/src/pkg/net/http/serve_tes...
File src/pkg/net/http/serve_test.go (right):

https://codereview.appspot.com/84480045/diff/40001/src/pkg/net/http/serve_tes...
src/pkg/net/http/serve_test.go:1025: var targ io.WriteCloser = struct {
On 2014/04/11 04:46:27, dsymonds wrote:
> neat trick. I wonder if it's a little too clever though. 

Writing fun test code is the only way I stay sane fixing boring bugs, though. 
:)

> There's exactly one
> place that this would need closing (and don't you want to close it even if
> there's a write error?)

The conn is already defer conn.Close() above.  This is just for writing.  Once
there's an error, there's no need to "Close" the chunk writing... that just
writes the final chunk header.  So it actually makes more sense to stop writing
at the first error.

> 
> var w io.Writer = conn
> if test.chunked {
>   w = httputil.NewChunkedWriter(conn)
> }
> body := ...
> _, err = fmt.Fprint(w, body)
> if test.chunked {
>   if cerr := w.Close(); err == nil {

You mean w.(io.Closer).Close(), which I think's a little gross.

>     err = cerr
>   }
> }

[dsymonds, 2014-04-11 05:20:37]

LGTM

yes, I got confused. it's fine.

[bradfitz, 2014-04-11 05:25:38]

*** Submitted as https://code.google.com/p/go/source/detail?r=872cad382ed5 ***

net/http: don't reject 0-lengthed bodies with Expect 100-continue

I was implementing rules from RFC 2616. The rules are apparently useless,
ambiguous, and too strict for common software on the Internet. (e.g. curl)

Add more tests, including a test of a chunked request.

Fixes Issue 7625

LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://codereview.appspot.com/84480045

[gobot, 2014-04-11 06:56:43]

This CL appears to have broken the netbsd-amd64-bsiegert builder.
See http://build.golang.org/log/940378e642a02cb59f2bb00c73aadbde3c888f26