go icon indicating copy to clipboard operation
go copied to clipboard

net/http: HTTP/2 configuration API

Open neild opened this issue 1 year ago • 10 comments

This issue is part of a project to move x/net/http2 into std: #67810

Configuring HTTP/2-specific protocol options currently requires users to import the golang.org/x/net/http2 package and call http2.ConfigureServer or http2.ConfigureTransports.

Configuring options in this fashion has the side effect of replacing the bundled HTTP/2 implementation in net/http with the one in golang.org/x/net/http2.

I propose adding HTTP/2 configuration options to net/http, permitting HTTP/2 servers and clients to be configured without importing an external package and separating configuration from version selection.

Many of the HTTP/2 settings are identical for server and client connections. For example, the MaxDecoderHeaderTableSize field of http2.Server and http2.Transport sets the SETTINGS_HEADER_TABLE_SIZE setting sent to the peer. We will unify these settings into a single configuration struct, and add this configuration to http.Server and http.Transport.

type HTTP2Config struct {
	// MaxConcurrentStreams optionally specifies the number of
	// concurrent streams that a peer may have open at a
	// time (SETTINGS_MAX_CONCURRENT_STREAMS).
	// If zero, MaxConcurrentStreams defaults to at least 100.
	MaxConcurrentStreams uint32

	// MaxDecoderHeaderTableSize optionally specifies the http2
	// SETTINGS_HEADER_TABLE_SIZE to send in the initial settings frame. It
	// informs the remote endpoint of the maximum size of the header compression
	// table used to decode header blocks, in octets. If zero, the default value
	// of 4096 is used.
	MaxDecoderHeaderTableSize uint32

	// MaxEncoderHeaderTableSize optionally specifies an upper limit for the
	// header compression table used for encoding request headers. Received
	// SETTINGS_HEADER_TABLE_SIZE settings are capped at this limit. If zero,
	// the default value of 4096 is used.
	MaxEncoderHeaderTableSize uint32

	// MaxReadFrameSize optionally specifies the largest frame
	// this endpoint is willing to read (SETTINGS_MAX_FRAME_SIZE).
	// A valid value is between 16k and 16M, inclusive.
	// If zero or otherwise invalid, a default value is used.
	MaxReadFrameSize uint32

	// MaxUploadBufferPerConnection is the size of the initial flow
	// control window for each connection. The HTTP/2 spec does not
	// allow this to be smaller than 65535 or larger than 2^32-1.
	// If the value is outside this range, a default value will be
	// used instead.
	MaxUploadBufferPerConnection int32

	// MaxUploadBufferPerStream is the size of the initial flow control
	// window for each stream. The HTTP/2 spec does not allow this to
	// be larger than 2^32-1. If the value is zero or larger than the
	// maximum, a default value will be used instead.
	MaxUploadBufferPerStream int32

	// SendPingTimeout is the timeout after which a health check using a ping
	// frame will be carried out if no frame is received on the connection.
	// If zero, no health check is performed.
	SendPingTimeout time.Duration

	// PingTimeout is the timeout after which the connection will be closed
	// if a response to a ping is not received.
	// If zero, a default of 15 seconds is used.
	PingTimeout time.Duration

	// WriteByteTimeout is the timeout after which a connection will be
	// closed if no data can be written to it. The timeout begins when data is
	// available to write, and is extended whenever any bytes are written.
	WriteByteTimeout time.Duration

	// PermitProhibitedCipherSuites, if true, permits the use of
	// cipher suites prohibited by the HTTP/2 spec.
	PermitProhibitedCipherSuites bool

	// CountError, if non-nil, is called on HTTP/2 errors.
	// It's intended to increment a metric for monitoring, such
	// as an expvar or Prometheus metric.
	// The errType consists of only ASCII word characters.
	CountError func(errType string)
}

type Server struct { // contains unchanged fields
	HTTP2 HTTP2Config
}

type Transport struct { // contains unchanged fields
	HTTP2 HTTP2Config
}

The SendPingTImeout and PingTimeout fields assume #67812 is accepted, and the WriteByteTimeout field assumes #67811 is accepted.

The http2.Transport.StrictMaxConcurrentStreams field controls whether a new connection should be opened to a server if an existing connection has exceeded its stream limit. For example, if an HTTP/2 server advertises a stream limit of 100 concurrent streams, then the 101st concurrent stream opened by the client will block waiting for an existing stream to complete when StrictMaxConcurrentStreams is true, or create a new connection when it is false. There is no equivalent to this setting for HTTP/1 connections, which only support a single concurrent request per connection. We will add this setting to http.Transport, since it could be used to configure HTTP/3 connections (if and when we support HTTP/3):

type Transport struct { // contains unchanged fields
	// StrictMaxConcurrentRequests controls whether an HTTP/2 server's
	// concurrency limit should be respected globally.
	// If true, new requests sent when an connection's concurrency limit has been exceeded
	// will block until an existing request completes.
	// If false, an additional connection will be opened if all existing connections are at their limit.
	StrictMaxConcurrentRequests bool
}

neild avatar Jun 04 '24 16:06 neild

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

rsc avatar Jun 12 '24 20:06 rsc

Why are the types uncommon (for Go APIs) sizes like uint32? Can they all be int? Otherwise this seems fine.

rsc avatar Jun 20 '24 18:06 rsc

The proposal reuses the types from the x/net/http2 Server and Transport.

HTTP/2 settings are defined as 32-bit unsigned integers; I don't know for sure, but I'd guess that's the reason for the uint32s. I don't see a problem with defining these as ints, although the extra range isn't going to be used. 24 more bytes in the Server and Transport structs isn't going to be noticeable, given that you're not expected to have a lot of either.

neild avatar Jun 20 '24 20:06 neild

It seems like now that this has outgrown being an HTTP/2 guts package, we should probably use Go types (int or else int64 if it really matters) instead of wire types. Otherwise this seems fine.

rsc avatar Jun 26 '24 17:06 rsc

Teşekkür

Fun32secg32 avatar Jun 26 '24 17:06 Fun32secg32

Have all remaining concerns about this proposal been addressed?

The proposal is https://github.com/golang/go/issues/67813#issue-2333927077, except using ints instead of uint32 etc.

rsc avatar Jun 27 '24 13:06 rsc

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

The proposal is https://github.com/golang/go/issues/67813#issue-2333927077, except using ints instead of uint32 etc.

rsc avatar Jul 25 '24 09:07 rsc

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

The proposal is https://github.com/golang/go/issues/67813#issue-2333927077, except using ints instead of uint32 etc.

rsc avatar Jul 31 '24 19:07 rsc

Change https://go.dev/cl/602175 mentions this issue: net/http: add HTTP2Config

gopherbot avatar Jul 31 '24 20:07 gopherbot

Change https://go.dev/cl/607255 mentions this issue: http2: add support for net/http HTTP2 config field

gopherbot avatar Aug 21 '24 00:08 gopherbot

A minor change to rename two fields:

MaxUploadBufferPerConnection is now MaxReceiveBufferPerConnection. MaxUploadBufferPerStream is now MaxReceiveBufferPerStream.

These fields originate as http2.Server fields, where "upload" unambiguously refers to the flow control window used to limit the rate at which the peer sends data. As part of a common struct configuring client and server, I think it's clearer for this field to consistently control the peer's flow control window, in which case "receive buffer" is more accurate than "upload buffer".

neild avatar Aug 26 '24 21:08 neild

Change https://go.dev/cl/615875 mentions this issue: net/http: add Transport.StrictMaxConcurrentRequests

gopherbot avatar Sep 25 '24 18:09 gopherbot

Change https://go.dev/cl/615895 mentions this issue: http2: add support for net/http Transport.StrictMaxConcurrentRequests

gopherbot avatar Sep 25 '24 18:09 gopherbot

Just to make sure I understand, I see that most of this API is in 1.24, but the issue is still open. Is it just Transport.StrictMaxConcurrentRequests that's missing from 1.24?

aclements avatar Dec 07 '24 01:12 aclements

Yes, we have everything in 1.24 except for Transport.StrictMaxConcurrentRequests. Some questions about StrictMaxConcurrentRequests came up in review, and I was still thinking through whether we want to promote it to the net/http API or not in its current form when the freeze window closed.

If you'd like, I can close this issue and open a new one for StrictMaxConcurrentRequests.

neild avatar Dec 09 '24 21:12 neild

If you'd like, I can close this issue and open a new one for StrictMaxConcurrentRequests.

No need. Thanks for the update!

aclements avatar Dec 10 '24 15:12 aclements

Just a note that I think that CountError could use a little more documentation. Are the strings passed to it entirely arbitrary other than being lowercase, digits, and underscore? Is there any canonical list we can point to? Or, what do we expect people to do with the counts once they have them? Thanks.

ianlancetaylor avatar Dec 11 '24 23:12 ianlancetaylor

Change https://go.dev/cl/706195 mentions this issue: net/http: drop "does not have any effect" from Server/Transport.HTTP2

gopherbot avatar Sep 23 '25 16:09 gopherbot

@neild I see some progress here from 3 days ago based on message above. Could you possibly fix also https://github.com/golang/go/issues/53208 as a part of this work 🙏? (adding configuration property to enable extended CONNECT for HTTP/2)

janvavro avatar Sep 26 '25 12:09 janvavro

Change https://go.dev/cl/707315 mentions this issue: http2: support HTTP2Config.StrictMaxConcurrentRequests

gopherbot avatar Sep 27 '25 00:09 gopherbot

Change https://go.dev/cl/707775 mentions this issue: all: update x/net to f2e909b98269

gopherbot avatar Sep 29 '25 16:09 gopherbot