Merge pull request hoisie#199 from hoisie/malformed-scgi

hoisie · web-flow · commit 62eb47b072ea · 2016-08-01T20:55:11.000-07:00
Gracefully handle malformed SCGI requests
diff --git a/scgi.go b/scgi.go
@@ -97,18 +97,20 @@ func (s *Server) readScgiRequest(fd io.ReadWriteCloser) (*http.Request, error) {
 	reader := bufio.NewReader(fd)
 	line, err := reader.ReadString(':')
 	if err != nil {
-		s.Logger.Println("Error during SCGI read: ", err.Error())
+		return nil, err
+	}
+	length, err := strconv.Atoi(line[0 : len(line)-1])
+	if err != nil {
+		return nil, err
 	}
-	length, _ := strconv.Atoi(line[0 : len(line)-1])
 	if length > 16384 {
-		s.Logger.Println("Error: max header size is 16k")
+		return nil, errors.New("Max header size is 16k")
 	}
 	headerData := make([]byte, length)
 	_, err = reader.Read(headerData)
 	if err != nil {
 		return nil, err
 	}
-
 	b, err := reader.ReadByte()
 	if err != nil {
 		return nil, err
@@ -138,14 +140,15 @@ func (s *Server) readScgiRequest(fd io.ReadWriteCloser) (*http.Request, error) {
 }
 
 func (s *Server) handleScgiRequest(fd io.ReadWriteCloser) {
+	defer fd.Close()
 	req, err := s.readScgiRequest(fd)
 	if err != nil {
-		s.Logger.Println("SCGI error: %q", err.Error())
+		s.Logger.Println("Error reading SCGI request: %q", err.Error())
+		return
 	}
 	sc := scgiConn{fd, req, make(map[string][]string), false}
 	s.routeHandler(req, &sc)
 	sc.finishRequest()
-	fd.Close()
 }
 
 func (s *Server) listenAndServeScgi(addr string) error {
diff --git a/web_test.go b/web_test.go
@@ -591,6 +591,29 @@ func TestNoColorOutput(t *testing.T) {
 	}
 }
 
+// a malformed SCGI request should be discarded and not cause a panic
+func TestMaformedScgiRequest(t *testing.T) {
+	var headerBuf bytes.Buffer
+
+	headerBuf.WriteString("CONTENT_LENGTH")
+	headerBuf.WriteByte(0)
+	headerBuf.WriteString("0")
+	headerBuf.WriteByte(0)
+	headerData := headerBuf.Bytes()
+
+	var buf bytes.Buffer
+	fmt.Fprintf(&buf, "%d:", len(headerData))
+	buf.Write(headerData)
+	buf.WriteByte(',')
+
+	var output bytes.Buffer
+	nb := ioBuffer{input: &buf, output: &output}
+	mainServer.handleScgiRequest(&nb)
+	if !nb.closed {
+		t.Fatalf("The connection should have been closed")
+	}
+}
+
 type TestHandler struct{}
 
 func (t *TestHandler) ServeHTTP(c http.ResponseWriter, req *http.Request) {
