Gracefully handle malformed SCGI requests

hoisie · hoisie · commit 2f9512f85248 · 2016-08-01T20:52:30.000-07:00
Previously, malformed SCGI requests would cause a panic when they were processed. Gracefully handle malformed SCGI requests. Also, add some additional error checking when parsing the length of the request, and clean up the code related to logging SCGI errors. Fixes hoisie#166
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) {
