code review 4930047: http: delete error kludge

http: delete error kludge

The kludge is targeted at broken web browsers
like Chrome and IE, but it gets in the way of
sending 400 or 500-series error results with
formatted bodies in response to AJAX requests
made by pages executing in those browsers.

Now the AJAX cases will work and Chrome
and IE will be as broken with Go servers as
they are with all the other servers.

Fixes issue 2169.

[rsc, 2011-08-23 20:55:23]

Hello bradfitz (cc: golang-dev@googlegroups.com),

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

[dsymonds, 2011-08-23 21:53:28]

LGTM

[bradfitz, 2011-08-24 00:51:30]

LGTM++

On Wed, Aug 24, 2011 at 12:55 AM, <rsc@golang.org> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> http: delete error kludge
>
> The kludge is targeted at broken web browsers
> like Chrome and IE, but it gets in the way of
> sending 400 or 500-series error results with
> formatted bodies in response to AJAX requests
> made by pages executing in those browsers.
>
> Now the AJAX cases will work and Chrome
> and IE will be as broken with Go servers as
> they are with all the other servers.
>
> Fixes issue 2169.
>
> Please review this at
http://codereview.appspot.com/**4930047/<http://codereview.appspot.com/4930047/>
>
> Affected files:
>  M src/pkg/http/server.go
>
>
> Index: src/pkg/http/server.go
> ==============================**==============================**=======
> --- a/src/pkg/http/server.go
> +++ b/src/pkg/http/server.go
> @@ -491,55 +491,6 @@
>        return m + n, err
>  }
>
> -// If this is an error reply (4xx or 5xx)
> -// and the handler wrote some data explaining the error,
> -// some browsers (i.e., Chrome, Internet Explorer)
> -// will show their own error instead unless the error is
> -// long enough.  The minimum lengths used in those
> -// browsers are in the 256-512 range.
> -// Pad to 1024 bytes.
> -func errorKludge(w *response) {
> -       const min = 1024
> -
> -       // Is this an error?
> -       if kind := w.status / 100; kind != 4 && kind != 5 {
> -               return
> -       }
> -
> -       // Did the handler supply any info?  Enough?
> -       if w.written == 0 || w.written >= min {
> -               return
> -       }
> -
> -       // Is it a broken browser?
> -       var msg string
> -       switch agent := w.req.UserAgent(); {
> -       case strings.Contains(agent, "MSIE"):
> -               msg = "Internet Explorer"
> -       case strings.Contains(agent, "Chrome/"):
> -               msg = "Chrome"
> -       default:
> -               return
> -       }
> -       msg += " would ignore this error page if this text weren't here.\n"
> -
> -       // Is it text?  ("Content-Type" is always in the map)
> -       baseType := strings.SplitN(w.header.Get("**Content-Type"), ";",
> 2)[0]
> -       switch baseType {
> -       case "text/html":
> -               io.WriteString(w, "<!-- ")
> -               for w.written < min {
> -                       io.WriteString(w, msg)
> -               }
> -               io.WriteString(w, " -->")
> -       case "text/plain":
> -               io.WriteString(w, "\n")
> -               for w.written < min {
> -                       io.WriteString(w, msg)
> -               }
> -       }
> -}
> -
>  func (w *response) finishRequest() {
>        // If this was an HTTP/1.0 request with keep-alive and we sent a
> Content-Length
>        // back, we can make this a keep-alive response ...
> @@ -555,7 +506,6 @@
>        if w.needSniff {
>                w.sniff()
>        }
> -       errorKludge(w)
>        if w.chunking {
>                io.WriteString(w.conn.buf, "0\r\n")
>                // trailer key/value pairs, followed by blank line
>
>
>

[rsc, 2011-08-24 02:39:33]

*** Submitted as http://code.google.com/p/go/source/detail?r=0f4ae6ba2d8f ***

http: delete error kludge

The kludge is targeted at broken web browsers
like Chrome and IE, but it gets in the way of
sending 400 or 500-series error results with
formatted bodies in response to AJAX requests
made by pages executing in those browsers.

Now the AJAX cases will work and Chrome
and IE will be as broken with Go servers as
they are with all the other servers.

Fixes issue 2169.

R=bradfitz, dsymonds
CC=golang-dev
http://codereview.appspot.com/4930047