Skip to content

net/http: add ResponseController.EnableFullDuplex #57786

Description

@neild

This proposal aims to address #15527.

The net/http HTTP/1 server does not permit reading from an inbound request body after starting to write the response. (See the ResponseWriter.Write documentation).

This limitation is because the server drains any unread portion of the request body before writing the response headers, to avoid deadlocking clients that attempt to write a complete request before reading the response. (See #15527 (comment) for more context.)

I propose that we offer an opt-in mechanism to disable this behavior, permitting a server handler to write some or all of the response interleaved with reads from the request.

// SetBidi indicates whether the request handler will interleave reads from Request.Body with
// writes to the ResponseWriter.
//
// For HTTP/1 requests, the Go HTTP server by default consumes any unread portion of the request
// body before beginning to write the response, preventing handlers from concurrently reading from
// the request and writing the response. Calling SetBidi(true) disables this behavior and permits
// handlers to continue to read from the request while concurrently writing the response.
//
// For HTTP/2 requests, the Go HTTP server always permits concurrent reads and responses.
func (c *ResponseController) SetBidi(bidi bool) error {}

Activity

  1. added this to the Proposal milestone on Jan 13, 2023
  2. moved this to Incoming in Proposalson Jan 13, 2023
  3. rsc commented on Feb 1, 2023

    @rsc
    Contributor

    "bidi" often means bidirectional text like Unicode LTR/RTL.
    It seems like a very short name for a rarely used feature.
    Is there a standard name for this behavior in the HTTP specs?
    It's unclear from the docs whether c.SetBidi(false) has an effect on HTTP/2 (or returns an error?).

  4. rsc commented on Feb 1, 2023

    @rsc
    Contributor

    This proposal has been added to the active column of the proposals project
    and will now be reviewed at the weekly proposal review meetings.
    — rsc for the proposal review group

  5. moved this from Incoming to Active in Proposalson Feb 1, 2023
  6. neild commented on Feb 1, 2023

    @neild
    ContributorAuthor

    I don't believe there is a standard name for this behavior. I'm not committed to the name here; EnableInterleavedReadsAndWrites would be unambiguous if long. EnableResponseInterleaving? EnableInterleave? EnableConcurrentResponse?

    SetBidi(false) will return an error for HTTP/2 requests. We already support interleaving for HTTP/2, and I don't see any value in disabling it. The reason for draining the inbound request before responding is to work better with naive clients, and a naive HTTP/2 client is somewhat of an oxymoron.

    Ideally we wouldn't have a knob here, but I don't see how to avoid it; there are valid reasons to want both possible behaviors here, and changing our current default will break some users.

  7. seankhliao commented on Feb 1, 2023

    @seankhliao
    Member

    informational rfc: bidirectional http
    internet draft: full duplex

  8. neild commented on Feb 1, 2023

    @neild
    ContributorAuthor

    I like EnableFullDuplex.

    And perhaps only permit enabling full duplex, to avoid any questions about what it means to disable it with HTTP/2 or HTTP/3.

    func (c *ResponseController) EnableFullDuplex() error {}
    
  9. changed the title [-]proposal: net/http: ResponseController.SetBidi to support concurrent Request.Body reads and ResponseWriter.Writes[/-] [+]proposal: net/http: ResponseController.EnableFullDuplex to support concurrent Request.Body reads and ResponseWriter.Writes[/+] on Feb 8, 2023
  10. rsc commented on Feb 8, 2023

    @rsc
    Contributor

    With the renaming to EnableFullDuplex, have all the concerns about this proposal been addressed?

  11. changed the title [-]proposal: net/http: ResponseController.EnableFullDuplex to support concurrent Request.Body reads and ResponseWriter.Writes[/-] [+]proposal: net/http: add ResponseController.EnableFullDuplex[/+] on Feb 22, 2023
  12. rsc commented on Feb 22, 2023

    @rsc
    Contributor

    Based on the discussion above, this proposal seems like a likely accept.
    — rsc for the proposal review group

  13. moved this from Active to Likely Accept in Proposalson Feb 22, 2023
  14. 8 remaining items

  15. self-assigned this
    on Mar 1, 2023
  16. gopherbot commented on Mar 1, 2023

    @gopherbot
    Contributor

    Change https://go.dev/cl/472636 mentions this issue: net/http: support full-duplex HTTP/1 responses

  17. gopherbot commented on Mar 1, 2023

    @gopherbot
    Contributor

    Change https://go.dev/cl/472717 mentions this issue: http2: support ResponseController.FullDuplex

  18. gopherbot commented on Jun 6, 2023

    @gopherbot
    Contributor

    Change https://go.dev/cl/501300 mentions this issue: go1.21: document net/http.ResponseController.EnableFullDuplex

  19. francislavoie commented on Aug 1, 2023

    @francislavoie

    Do we have a rough idea of which clients don't support full-duplex? Is there a list of clients known to produce deadlocks?

    I'm asking because we're adding this as an opt-in config option in Caddy (for ref: caddyserver/caddy#5654), and I'd like it if I could document something like "don't enable this if you know you have such and such clients connecting to your server".

    I'd also consider enabling this by default if it's really only super-old clients that don't handle this properly (e.g. only old browsers that nobody should be using anymore anyway, or old versions of curl, etc).

  20. neild commented on Aug 1, 2023

    @neild
    ContributorAuthor

    Sorry, I've got no idea how common this client behavior is. I wouldn't expect this to be an issue for browsers (although I haven't checked any), but browsers are also unlikely to be sending requests that need full duplex handling.

    The simplest implementation of an HTTP client is to open a connection, write a request, and read a response. I suspect there are a fair number of versions of that out in the wild.

  21. oliverpool commented on Aug 12, 2023

    @oliverpool

    This has been implemented, documented and released in go1.21, so I guess this issue can be closed.

  22. modified the milestones: Backlog, Go1.21 on Aug 18, 2023
  23. removed this from Proposalson Aug 14, 2024
  24. locked and limited conversation to collaborators on Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions