Skip to content

net/http: ResponseWriter.ReadFrom writes to HEAD response body #68609

Closed
@uhthomas

Description

@uhthomas

Go version

go version go1.22.5 linux/amd64

Output of go env in your module/workspace:

N/A

What did you do?

I have published a small reproducer to this repository: https://github.com/uhthomas/go-unsolicited-http

The instructions are in the README, but for brevity:

Run the server:

code
main.go
package main

import (
	"context"
	"flag"
	"fmt"
	"io"
	"log"
	"net/http"
	"sync/atomic"
)

type Server struct {
	requestCount uint64
}

func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	requestID := atomic.AddUint64(&s.requestCount, 1)

	if f, ok := w.(http.Flusher); ok {
		f.Flush()
	}

	res, err := http.Get("https://go.dev")
	if err != nil {
		panic(err)
	}

	n, err := io.Copy(w, res.Body)

	fmt.Printf("%d: %d bytes written, err=%v\n", requestID, n, err)
}

func Main(ctx context.Context) error {
	addr := flag.String("addr", ":8080", "address to listen on")
	flag.Parse()

	log.Println("listening on", *addr)

	return http.ListenAndServe(*addr, &Server{})
}

func main() {
	if err := Main(context.Background()); err != nil {
		log.Fatal(err)
	}
}
❯ go run github.com/uhthomas/go-unsolicited-http/cmd/server@main

Run the client:

code
main.go
package main

import (
	"context"
	"fmt"
	"log"
	"net/http"
	"time"
)

func Main(ctx context.Context) error {
	for i := 0; ; i++ {
		fmt.Println("attempt", i)

		if _, err := http.Head("http://localhost:8080"); err != nil {
			return fmt.Errorf("head: %w", err)
		}

		time.Sleep(200 * time.Millisecond)
	}
}

func main() {
	if err := Main(context.Background()); err != nil {
		log.Fatal(err)
	}
}
❯ go run github.com/uhthomas/go-unsolicited-http/cmd/client@main

What did you see happen?

Server:

1: 54 bytes written, err=<nil>
2: 0 bytes written, err=readfrom tcp [::1]:8080->[::1]:48336: write tcp [::1]:8080->[::1]:48336: write: broken pipe
3: 54 bytes written, err=<nil>
4: 54 bytes written, err=<nil>
5: 54 bytes written, err=<nil>
6: 54 bytes written, err=<nil>
7: 54 bytes written, err=<nil>
8: 54 bytes written, err=<nil>
9: 54 bytes written, err=<nil>
10: 0 bytes written, err=readfrom tcp [::1]:8080->[::1]:33742: write tcp [::1]:8080->[::1]:33742: write: broken pipe

Client:

attempt 0
2024/07/26 16:17:17 Unsolicited response received on idle HTTP channel starting with "<!DOCTYPE html>\n<html>\n  <head>\n    <title>Thomas</tit"; err=<nil>
attempt 1
2024/07/26 16:17:17 Unsolicited response received on idle HTTP channel starting with "<!DOCTYPE html>\n<html>\n  <head>\n    <title>Thomas</tit"; err=<nil>
attempt 2
2024/07/26 16:17:17 Unsolicited response received on idle HTTP channel starting with "<!DOCTYPE html>\n<html>\n  <head>\n    <title>Thomas</tit"; err=<nil>
attempt 3
2024/07/26 16:17:17 Unsolicited response received on idle HTTP channel starting with "<!DOCTYPE html>\n<html>\n  <head>\n    <title>Thomas</tit"; err=<nil>
attempt 4
2024/07/26 16:17:17 Unsolicited response received on idle HTTP channel starting with "<!DOCTYPE html>\n<html>\n  <head>\n    <title>Thomas</tit"; err=<nil>
attempt 5
2024/07/26 16:17:17 Unsolicited response received on idle HTTP channel starting with "<!DOCTYPE html>\n<html>\n  <head>\n    <title>Thomas</tit"; err=<nil>
attempt 6
attempt 7
2024/07/26 16:17:18 do: Head "http://localhost:8080": net/http: HTTP/1.x transport connection broken: malformed HTTP status code "html>"
exit status 1

What did you expect to see?

There should be no Unsolicited response received on idle HTTP channel starting with messages, and the request should not fail with net/http: HTTP/1.x transport connection broken: malformed HTTP status code "html>".

Activity

uhthomas

uhthomas commented on Jul 26, 2024

@uhthomas
Author

#19895 is one of the only matches for "Unsolicited response received on idle HTTP channel starting with" and claims the HTTP server to be misbehaving.

seankhliao

seankhliao commented on Jul 26, 2024

@seankhliao
Member

Your server is indeed misbehaving by writing data when it receives a HEAD request.

uhthomas

uhthomas commented on Jul 26, 2024

@uhthomas
Author

Sure, but it should not cause this sort of behavior at all. It's quite problematic, and inconsistent. Under many circumstances, there are no issues with writing a response to a HEAD request, even if it's wrong. It should not affect other connections to the server, which it does, and it should not leak data between connections, which it does. Please can this be reopened and considered more fairly?

mvdan

mvdan commented on Jul 26, 2024

@mvdan
Member

@seankhliao where is that documented in the net/http docs? A quick skim doesn't show me anything, even if HEAD responses not containing a body could be common knowledge.

Assuming you're right about that assumption, I would imagine that calling Write on https://pkg.go.dev/net/http#ResponseWriter for a HEAD request should result in either a clear error or panic, and not anything like corruption or data races or confusing errors.

terinjokes

terinjokes commented on Jul 26, 2024

@terinjokes
Contributor

To me the issue here looks like a Go HTTP client receiving a body in response to an HEAD request, and then the established HTTP/1.1 connection was reused for another request, where it tried to read the previously sent body as the start of the new response. This could happen regardless of the server implementation.

RFC 9110 section 9.3.2. recommends clients close and error when a HEAD response is received with a body to avoid request/response smuggling. Is that something we can consider here?

changed the title [-]net/http: data is written to idle connections[/-] [+]net/http: Transport should discard connections from HEAD requests with a body[/+] on Jul 26, 2024
added
NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
on Jul 26, 2024
seankhliao

seankhliao commented on Jul 26, 2024

@seankhliao
Member

looks like the client (transport) assumes a HEAD request never has a body https://go.googlesource.com/go/+/3959d54c0bd5c92fe0a5e33fedb0595723efc23b/src/net/http/transport.go#2245

cc @neild

seankhliao

seankhliao commented on Jul 26, 2024

@seankhliao
Member

I suppose #62015 would be the corresponding issue for discarding the body from the server side

uhthomas

uhthomas commented on Jul 26, 2024

@uhthomas
Author

Thanks for the additional input, and thank you for reopening.

Something I'd really like to understand is why this doesn't happen when writing data in-memory? I could only ever reproduce this when making a HTTP request on the server and writing it back to the client.

neild

neild commented on Jul 26, 2024

@neild
Contributor

Some of the above is talking about a body in a HEAD request, and some is talking about a body in the response to a HEAD request. These are completely different things.

HEAD requests may contain a body, but the body has no defined semantics. (RFC 9110, section 9.3.2)

The response to a HEAD request does not contain a body. RFC 9112, section 6.3:

Any response to a HEAD request and any response with a 1xx (Informational), 204 (No Content), or 304 (Not Modified) status code is always terminated by the first empty line after the header fields, regardless of the header fields present in the message, and thus cannot contain a message body or trailer section.

Improperly writing a body leaves a connection in an invalid state, since the client is expecting the next bytes read to be the status line for the next request on the connection.


Thanks for the report, @uhthomas. This is a bug in net/http. The problem is that while an http.ResponseWriter normally discards the body for HEAD requests, ResponseWriter.ReadFrom fails to do so. Triggering this bug requires either calling ReadFrom directly, or using io.Copy to copy to a ResponseWriter from a source with no WriteTo method.

uhthomas

uhthomas commented on Jul 26, 2024

@uhthomas
Author

@neild Ohh, that answers my previous question, thank you so much for the insight.

22 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    NeedsFixThe path to resolution is known, but the work has not been done.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @neild@terinjokes@dmitshur@mvdan@gopherbot

        Issue actions

          net/http: ResponseWriter.ReadFrom writes to HEAD response body · Issue #68609 · golang/go