http: Status-Line with empty Reason-Phrase not supported

[bradfitz]

51c777dbccb9 release.2010-12-22/release

Go doesn't permit a HTTP response status line like:

HTTP/1.0 303_

(where the "_" above is a trailing space, not an underscore)

Unfortunately, App Engine's dev_appserver.py generates such responses.  However, they
appear to be valid:

HTTP/1.1's definition:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html#sec6.1

      Status-Line = HTTP-Version SP Status-Code SP Reason-Phrase CRLF
...
      Reason-Phrase  = *<TEXT, excluding CR, LF>

Looking at Go's src/pkg/http/request.go and its readLineBytes() func, which is called by
ReadResponse, the problem is that whitespace is stripped from the end before the split:


func readLineBytes(b *bufio.Reader) (p []byte, err os.Error) {
...

        // Chop off trailing white space.                                                                                                                    
        var i int
        for i = len(p); i > 0; i-- {
                if c := p[i-1]; c != ' ' && c != '\r' && c != '\t' && c != '\n' {
                        break
                }
    }
        return p[0:i], nil
}



func ReadResponse(r *bufio.Reader, requestMethod string) (resp *Response, err os.Error) {
...
        // Parse the first line of the response.                                                                                                             
    line, err := readLine(r)
...
        f := strings.Split(line, " ", 3)
        if len(f) < 3 {
                return nil, &badStringError{"malformed HTTP response", line}
        }

[bradfitz]

Comment 1:

Untested minimal fix, perhaps not fixing the root issue.  (I'd prefer to fix readLine,
but that'd require some auditing)
bradfitz@bradfitz-glaptop:~/go/src$ hg diff
diff -r 51c777dbccb9 src/pkg/http/response.go
--- a/src/pkg/http/response.go  Thu Dec 23 13:32:20 2010 +1100
+++ b/src/pkg/http/response.go  Wed Jan 05 07:36:04 2011 -0800
@@ -86,9 +86,12 @@
        return nil, err
    }
    f := strings.Split(line, " ", 3)
-   if len(f) < 3 {
+   if len(f) < 2 {
        return nil, &badStringError{"malformed HTTP response", line}
    }
+   if len(f) == 2 {
+       f = append(f, "")  // empty Reason-Phrase
+   }
    resp.Status = f[1] + " " + f[2]
    resp.StatusCode, err = strconv.Atoi(f[1])
    if err != nil {
And the above patch doesn't properly reject responses like "HTTP/1.0 303" that don't end
in trailing whitespace.

[rsc]

Comment 2:

I don't have a problem with accepting "HTTP/1.0 303\n".
Is it important to reject that?  Liberal in what you accept and all.
I'd avoid the append by doing
text := ""
if len(f) > 2 {
    text = f[2]
}
but otherwise sounds good to me.
Want to send it in?

Owner changed to r...@golang.org.

Status changed to Accepted.

[bradfitz]

Comment 3:

Sure, will do.  One sec.

[bradfitz]

Comment 4:

http://golang.org/cl/3749043

[rsc]

Comment 5:

lgtm; please run hg mail to put it on golang-dev's radar

[rsc]

Comment 6:

This issue was closed by revision d71d08a.

Status changed to Fixed.