Empty body for responses that cannot have one

client/client.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"net/http/httputil"
 	"strconv"
 	"strings"
@@ -87,14 +88,20 @@ func NewClient(rw io.ReadWriter) Client {
 type client struct {
 	reader
 	writer
+
+	// method of the last written request, so the response reader knows
+	// whether a body may follow (responses to HEAD never carry one)
+	method string
 }
 
 // SendRequest marshalls a HTTP request to the wire.
 func (c *client) WriteRequest(req *Request) error {
 	if len(req.RawBytes) > 0 {
+		c.method = rawMethod(req.RawBytes)
 		_, err := c.Write(req.RawBytes)
 		return err
 	}
+	c.method = req.Method
 	if err := c.WriteRequestLine(req.Method, req.Path, req.Query, req.Version.String()); err != nil {
 		return err
 	}
@@ -124,6 +131,31 @@ func (c *client) WriteRequest(req *Request) error {
 	return c.WriteBody(req.Body)
 }
 
+// rawMethod returns the method token of a raw request line.
+func rawMethod(raw []byte) string {
+	if i := bytes.IndexByte(raw, ' '); i > 0 {
+		return string(raw[:i])
+	}
+	return ""
+}
+
+// IsBodyless reports whether a response with status code to a request with
+// method cannot carry a body, whatever its headers say (RFC 9112 section 6.3).
+// Reading a body for such a response hangs until the deadline and consumes
+// bytes that belong to the next response on the connection.
+func IsBodyless(method string, code int) bool {
+	switch {
+	case method == http.MethodHead:
+		return true
+	case code >= 100 && code < 200, code == http.StatusNoContent, code == http.StatusNotModified:
+		return true
+	case method == http.MethodConnect && code >= 200 && code < 300:
+		// the connection becomes a tunnel right after the header section
+		return true
+	}
+	return false
+}
+
 // maxDuplicateStatusLines bounds how many repeated status lines we are willing
 // to discard, so a misbehaving (or malicious) server streaming "HTTP/..." lines
 // indefinitely cannot keep us looping.
@@ -176,7 +208,10 @@ func (c *client) ReadResponse(forceReadAll bool) (*Response, error) {
 		Headers: headers,
 		Body:    c.ReadBody(),
 	}
-	if l := resp.ContentLength(); l >= 0 && !forceReadAll {
+	if !forceReadAll && IsBodyless(c.method, code) {
+		// keep the reported Content-Length header, but read no body
+		resp.Body = http.NoBody
+	} else if l := resp.ContentLength(); l >= 0 && !forceReadAll {
 		resp.Body = io.LimitReader(resp.Body, l)
 	} else if resp.TransferEncoding() == "chunked" {
 		resp.Body = httputil.NewChunkedReader(resp.Body)

client/reader_test.go

@@ -93,3 +93,47 @@ func TestReadResponseTripleStatusLines(t *testing.T) {
 	require.Equal(t, 200, resp.Status.Code)
 	require.Equal(t, "test", resp.Headers[0].Value)
 }
+
+// TestReadResponseBodyless reads keep-alive responses back to back from one
+// stream. A response that cannot carry a body must read none, so the next
+// response starts at its status line.
+func TestReadResponseBodyless(t *testing.T) {
+	tests := []struct {
+		name   string
+		req    *Request
+		raw    string
+		code   int
+		length int64
+	}{
+		{name: "204 without length", req: &Request{Method: "GET", Body: strings.NewReader("")}, raw: "HTTP/1.1 204 No Content\r\n\r\n", code: 204, length: -1},
+		{name: "304 with length", req: &Request{Method: "GET", Body: strings.NewReader("")}, raw: "HTTP/1.1 304 Not Modified\r\nContent-Length: 42\r\n\r\n", code: 304, length: 42},
+		{name: "1xx", req: &Request{Method: "GET", Body: strings.NewReader("")}, raw: "HTTP/1.1 102 Processing\r\nContent-Length: 3\r\n\r\n", code: 102, length: 3},
+		{name: "HEAD with length", req: &Request{Method: "HEAD", Body: strings.NewReader("")}, raw: "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n", code: 200, length: 5},
+		{name: "raw HEAD with length", req: &Request{RawBytes: []byte("HEAD / HTTP/1.1\r\n\r\n")}, raw: "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n", code: 200, length: 5},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// requests carry an empty body: the writer only returns to the
+			// request line phase after a body is written
+			next := "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nhello"
+			c := NewClient(readWriterStub{strings.NewReader(tt.raw + next)})
+
+			require.NoError(t, c.WriteRequest(tt.req))
+			resp, err := c.ReadResponse(false)
+			require.NoError(t, err)
+			require.Equal(t, tt.code, resp.Status.Code)
+			require.Equal(t, tt.length, resp.ContentLength(), "the reported Content-Length must be kept")
+			body, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.Empty(t, string(body))
+
+			require.NoError(t, c.WriteRequest(&Request{Method: "GET", Body: strings.NewReader("")}))
+			resp, err = c.ReadResponse(false)
+			require.NoError(t, err)
+			require.Equal(t, 200, resp.Status.Code)
+			body, err = io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.Equal(t, "hello", string(body))
+		})
+	}
+}

client_test.go

@@ -150,3 +150,26 @@ func TestResponseBodyOwnsConnection(t *testing.T) {
 		t.Errorf("Close called %d times, want 1", conn.closeCalls)
 	}
 }
+
+// A response without a body has no gzip stream to decode, even when it
+// reports a gzip Content-Encoding, as a HEAD response does.
+func TestBodylessGzipResponse(t *testing.T) {
+	conn := &cleanupConn{response: &rawclient.Response{
+		Version: rawclient.HTTP_1_1,
+		Status:  rawclient.Status{Code: http.StatusOK, Reason: "OK"},
+		Headers: []rawclient.Header{{Key: "Content-Encoding", Value: "gzip"}, {Key: "Content-Length", Value: "20"}},
+		Body:    http.NoBody,
+	}}
+	c := &Client{dialer: &cleanupDialer{conn: conn}, Options: &Options{}}
+	resp, err := c.DoRaw("HEAD", "http://example.com/", "", nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	body, err := io.ReadAll(resp.Body)
+	if err != nil || len(body) != 0 {
+		t.Fatalf("body = %q, error = %v, want empty", body, err)
+	}
+	if resp.ContentLength != 20 {
+		t.Errorf("ContentLength = %d, want the reported 20", resp.ContentLength)
+	}
+}

clientpipeline/client.go

@@ -545,7 +545,7 @@ func (c *pipelineConnClient) reader(conn net.Conn, stopCh <-chan struct{}) error
 				return err
 			}
 		}
-		if err = w.resp.Read(br); err != nil {
+		if err = w.resp.ReadWithMethod(br, w.req.Method); err != nil {
 			w.err = err
 			w.done <- struct{}{}
 			return err

clientpipeline/response.go

@@ -6,9 +6,12 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"net/http/httputil"
 	"strconv"
 	"strings"
+
+	"github.com/projectdiscovery/rawhttp/client"
 )
 
 // Response represents an RFC2616 response.
@@ -65,7 +68,16 @@ type Status struct {
 	Reason string
 }
 
+// Read reads a response from r. It cannot tell a response to HEAD from
+// others, so prefer ReadWithMethod when the request method is known.
 func (resp *Response) Read(r *bufio.Reader) error {
+	return resp.ReadWithMethod(r, "")
+}
+
+// ReadWithMethod reads the response to a request with method from r. A
+// response that cannot carry a body (RFC 9112 section 6.3) gets an empty
+// Body and leaves r at the start of the next response.
+func (resp *Response) ReadWithMethod(r *bufio.Reader, method string) error {
 	version, code, msg, err := resp.ReadStatusLine(r)
 	var headers []Header
 	if err != nil {
@@ -89,6 +101,12 @@ func (resp *Response) Read(r *bufio.Reader) error {
 	resp.Version = version
 	resp.Status = Status{Code: code, Reason: msg}
 	resp.Headers = headers
+	if client.IsBodyless(method, code) {
+		// keep the reported Content-Length header, but read no body
+		resp.body = nil
+		resp.Body = http.NoBody
+		return nil
+	}
 	resp.Body = resp.ReadBody(r)
 
 	if l := resp.ContentLength(); l >= 0 {
@@ -219,6 +237,10 @@ func (resp *Response) ReadHeader(r *bufio.Reader) (string, string, bool, error)
 }
 
 func (resp *Response) ReadBody(r *bufio.Reader) io.Reader {
+	if client.IsBodyless("", resp.Status.Code) {
+		resp.body = nil
+		return http.NoBody
+	}
 	l := resp.ContentLength()
 	if l > 0 {
 		resp.body = make([]byte, l)

clientpipeline/response_test.go

@@ -0,0 +1,136 @@
+package clientpipeline
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+)
+
+type pipelinedExchange struct {
+	method string
+	path   string
+	raw    string // the response the server writes for this request
+	code   int
+	body   string
+	length int64 // the Content-Length the response reports, -1 if none
+}
+
+// servePipelined accepts one connection, reads one request per exchange and
+// only then writes all responses in a single write, in the order the requests
+// arrived, so the responses sit back to back in the client read buffer.
+func servePipelined(t *testing.T, exchanges []pipelinedExchange) string {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = ln.Close() })
+
+	byPath := make(map[string]string, len(exchanges))
+	for _, e := range exchanges {
+		byPath[e.path] = e.raw
+	}
+	go func() {
+		conn, err := ln.Accept()
+		if err != nil {
+			return
+		}
+		defer func() { _ = conn.Close() }()
+		br := bufio.NewReader(conn)
+		var out strings.Builder
+		for range exchanges {
+			req, err := http.ReadRequest(br)
+			if err != nil {
+				return
+			}
+			out.WriteString(byPath[req.URL.Path])
+		}
+		if _, err := io.WriteString(conn, out.String()); err != nil {
+			return
+		}
+		// keep the connection open: a client that reads past a response
+		// waits here instead of seeing EOF
+		_, _ = io.Copy(io.Discard, conn)
+	}()
+	return ln.Addr().String()
+}
+
+func runPipelined(t *testing.T, exchanges []pipelinedExchange) {
+	t.Helper()
+	addr := servePipelined(t, exchanges)
+	c := &PipelineClient{
+		Addr:        addr,
+		MaxConns:    1,
+		ReadTimeout: 5 * time.Second,
+	}
+
+	type result struct {
+		resp Response
+		body string
+		err  error
+	}
+	results := make([]result, len(exchanges))
+	var wg sync.WaitGroup
+	for i, e := range exchanges {
+		wg.Add(1)
+		go func(i int, e pipelinedExchange) {
+			defer wg.Done()
+			req := ToRequest(e.method, e.path, nil, map[string][]string{"Host": {addr}}, nil)
+			res := &results[i]
+			if res.err = c.Do(req, &res.resp); res.err != nil {
+				return
+			}
+			b, err := io.ReadAll(res.resp.Body)
+			res.body, res.err = string(b), err
+		}(i, e)
+	}
+
+	done := make(chan struct{})
+	go func() {
+		wg.Wait()
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		t.Fatal("pipelined responses did not complete: a bodyless response is read until the deadline")
+	}
+
+	for i, e := range exchanges {
+		res := results[i]
+		name := fmt.Sprintf("%s %s", e.method, e.path)
+		if res.err != nil {
+			t.Errorf("%s: %v", name, res.err)
+			continue
+		}
+		if res.resp.Status.Code != e.code || res.body != e.body {
+			t.Errorf("%s = %d %q, want %d %q", name, res.resp.Status.Code, res.body, e.code, e.body)
+		}
+		if got := res.resp.ContentLength(); got != e.length {
+			t.Errorf("%s: ContentLength() = %d, want the reported %d", name, got, e.length)
+		}
+	}
+}
+
+func TestPipelineBodylessStatus(t *testing.T) {
+	runPipelined(t, []pipelinedExchange{
+		{method: "GET", path: "/no-content", raw: "HTTP/1.1 204 No Content\r\n\r\n", code: 204, length: -1},
+		{method: "GET", path: "/one", raw: "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nfirst", code: 200, body: "first", length: 5},
+		// a 304 reports the length of the representation it validates
+		{method: "GET", path: "/not-modified", raw: "HTTP/1.1 304 Not Modified\r\nContent-Length: 42\r\n\r\n", code: 304, length: 42},
+		{method: "GET", path: "/two", raw: "HTTP/1.1 200 OK\r\nContent-Length: 6\r\n\r\nsecond", code: 200, body: "second", length: 6},
+	})
+}
+
+func TestPipelineHeadResponse(t *testing.T) {
+	runPipelined(t, []pipelinedExchange{
+		{method: "HEAD", path: "/head", raw: "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n", code: 200, length: 5},
+		{method: "GET", path: "/get", raw: "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nhello", code: 200, body: "hello", length: 5},
+	})
+}

util.go

@@ -60,7 +60,8 @@ func toHTTPResponse(conn Conn, resp *client.Response) (*http.Response, error) {
 
 	var err error
 	rbody := resp.Body
-	if headerValue(rheaders, "Content-Encoding") == "gzip" {
+	// an empty body is not a gzip stream, so do not wrap one
+	if rbody != http.NoBody && headerValue(rheaders, "Content-Encoding") == "gzip" {
 		rbody, err = gzip.NewReader(rbody)
 		if err != nil {
 			return nil, err