Fixed bug in scgi parsing bodies with nil string

Michael Hoisie · Michael Hoisie · commit e590b3384f28 · 2010-01-13T23:00:33.000-08:00
diff --git a/scgi.go b/scgi.go
@@ -59,24 +59,51 @@ func (conn *scgiConn) Write(data []byte) (n int, err os.Error) {
 
 func (conn *scgiConn) Close() { conn.fd.Close() }
 
-func readScgiRequest(buf *bytes.Buffer) *Request {
+func readScgiRequest(buf *bytes.Buffer) (*Request, os.Error) {
     headers := make(map[string]string)
 
-    content := buf.Bytes()
-    colon := bytes.IndexByte(content, ':')
-    content = content[colon+1:]
-    fields := bytes.Split(content, []byte{0}, 0)
+    data := buf.Bytes()
+    var clen int
+
+    colon := bytes.IndexByte(data, ':')
+    data = data[colon+1:]
+    var err os.Error
+    //find the CONTENT_LENGTH
+
+    clfields := bytes.Split(data, []byte{0}, 3)
+    if len(clfields) != 3 {
+        return nil,os.NewError ("Invalid SCGI Request -- no fields")
+    }
+
+    clfields = clfields[0:2]
+    if string(clfields[0]) != "CONTENT_LENGTH"{
+        return nil,os.NewError ("Invalid SCGI Request -- expecing CONTENT_LENGTH")
+    }
+
+    if clen,err = strconv.Atoi ( string(clfields[1]) ); err != nil {
+        return nil,os.NewError ("Invalid SCGI Request -- invalid CONTENT_LENGTH field")
+    }
+    
+
+    content := data[ len(data) - clen : ]
+
+    println("clen", clen, len(content))
+    
+    fields := bytes.Split ( data [ 0: len(data) - clen], []byte{0}, 0 )
+
     for i := 0; i < len(fields)-1; i += 2 {
         key := string(fields[i])
         value := string(fields[i+1])
         headers[key] = value
     }
-    var body bytes.Buffer
-    body.Write(fields[len(fields)-1][1:])
 
-    req := newRequestCgi(headers, &body)
+    body := bytes.NewBuffer(content)
+    //var body bytes.Buffer
+    //body.Write(content)
+
+    req := newRequestCgi(headers, body)
 
-    return req
+    return req, nil
 }
 
 func handleScgiRequest(fd io.ReadWriteCloser) {
@@ -92,17 +119,23 @@ func handleScgiRequest(fd io.ReadWriteCloser) {
     read := n
     length, _ := strconv.Atoi(string(tmp[0:colonPos]))
     buf.Write(tmp[0:n])
-
+    println("read", n)
     for read < length {
         n, err := fd.Read(&tmp)
         if err != nil || n == 0 {
             break
         }
+	
         buf.Write(tmp[0:n])
         read += n
     }
 
-    req := readScgiRequest(&buf)
+    req,err := readScgiRequest(&buf)
+    
+    if err != nil {
+        log.Stderrf("SCGI read error", err.String())
+	return;
+    }
 
     sc := scgiConn{fd, make(map[string][]string), false}
     routeHandler(req, &sc)
diff --git a/web_test.go b/web_test.go
@@ -138,9 +138,12 @@ func init() {
 var tests = []Test{
     Test{"GET", "/", "", 200, "index"},
     Test{"GET", "/echo/hello", "", 200, "hello"},
+    Test{"GET", "/echo/hello", "", 200, "hello"},
     Test{"GET", "/multiecho/a/b/c/d", "", 200, "abcd"},
     Test{"POST", "/post/echo/hello", "", 200, "hello"},
+    Test{"POST", "/post/echo/hello", "", 200, "hello"},
     Test{"POST", "/post/echoparam/a", "a=hello", 200, "hello"},
+    Test{"POST", "/post/echoparam/a", "a=hello\x00", 200, "hello\x00"},
     //long url
     Test{"GET", "/echo/" + strings.Repeat("0123456789", 100), "", 200, strings.Repeat("0123456789", 100)},
 
@@ -195,8 +198,8 @@ func TestRouting(t *testing.T) {
     }
 }
 
-func buildScgiFields(fields map[string]string) []byte {
-    var buf bytes.Buffer
+func buildScgiFields(fields map[string]string, buf *bytes.Buffer) []byte {
+    
     for k, v := range (fields) {
         buf.WriteString(k)
         buf.Write([]byte{0})
@@ -208,34 +211,40 @@ func buildScgiFields(fields map[string]string) []byte {
 }
 
 func buildTestScgiRequest(method string, path string, body string, headers map[string]string) *bytes.Buffer {
+
+    var hbuf bytes.Buffer
     scgiHeaders := make(map[string]string)
 
+    hbuf.WriteString("CONTENT_LENGTH")
+    hbuf.Write([]byte{0})
+    hbuf.WriteString(fmt.Sprintf("%d", len(body)))
+    hbuf.Write([]byte{0})
+
     scgiHeaders["REQUEST_METHOD"] = method
     scgiHeaders["HTTP_HOST"] = "127.0.0.1"
     scgiHeaders["REQUEST_URI"] = path
     scgiHeaders["SERVER_PORT"] = "80"
     scgiHeaders["SERVER_PROTOCOL"] = "HTTP/1.1"
     scgiHeaders["USER_AGENT"] = "web.go test framework"
 
+    buildScgiFields(scgiHeaders, &hbuf)
+
     if method == "POST" {
         headers["Content-Length"] = fmt.Sprintf("%d", len(body))
         headers["Content-Type"] = "text/plain"
     }
 
-    f1 := buildScgiFields(scgiHeaders)
-    fields := f1
-
     if len(headers) > 0 {
-        f2 := buildScgiFields(headers)
-        fields = bytes.Join([][]byte{f1, f2}, []byte{})
+        buildScgiFields(headers, &hbuf)
     }
-
-    var buf bytes.Buffer
-
-    //comma at the end
-    clen := len(fields) + len(body) + 1
-    fmt.Fprintf(&buf, "%d:", clen)
-    buf.Write(fields)
+    
+    fielddata := hbuf.Bytes()
+    var buf bytes.Buffer;
+
+    //extra 1 is for the comma at the end
+    dlen := len(fielddata) + len(body) + 1
+    fmt.Fprintf(&buf, "%d:", dlen)
+    buf.Write(fielddata)
     buf.WriteString(",")
     buf.WriteString(body)
 
@@ -289,17 +298,20 @@ func buildTestFcgiRequest(method string, path string, bodychunks []string, heade
     var req bytes.Buffer
     fcgiHeaders := make(map[string]string)
 
+    bodylength := 0
+    for _, s := range (bodychunks) {
+        bodylength += len(s)
+    }
+
+    fcgiHeaders["CONTENT_LENGTH"] = fmt.Sprintf("%d", bodylength)
     fcgiHeaders["REQUEST_METHOD"] = method
     fcgiHeaders["HTTP_HOST"] = "127.0.0.1"
     fcgiHeaders["REQUEST_URI"] = path
     fcgiHeaders["SERVER_PORT"] = "80"
     fcgiHeaders["SERVER_PROTOCOL"] = "HTTP/1.1"
     fcgiHeaders["USER_AGENT"] = "web.go test framework"
 
-    bodylength := 0
-    for _, s := range (bodychunks) {
-        bodylength += len(s)
-    }
+
 
     if method == "POST" {
         fcgiHeaders["Content-Length"] = fmt.Sprintf("%d", bodylength)
